Don't use responseType = blob | safari fails

[jimmywarting]

Ideally i think this should be reported to Safari but you could fix it much faster

I found out about this bug on a page that is running https

• the page has an iframe with a sandbox attribute
• the sandbox attribute only has "allow-script"
• the iframe is executing fetch's polyfills that boils down to this code:

xhr = new XMLHttpRequest()
xhr.open('GET', 'https://httpbin.org/get')
xhr.responseType = 'blob'
xhr.send()
xhr.onload = () => {
  var fr = new FileReader()
  fr.onerror = () => console.error
  fr.onload = () => console.log(fr.result)
  fr.readAsText(xhr.response)
}

The result i'm getting is

[blocked] The page at https://localhost/ was not allowed to display insecure content from blob:null/bd7df00e-cc00-41d7-a81d-64e9962417bf.

[mislav]

Thanks for tracking this down. Ideally, we shouldn't be setting xhr.responseType = 'blob' for every request, mostly because this is sort of wasteful in scenarios where we're only interested in response.text() (the most common ajax scenario in today's applications), which causes the responseText -> Blob -> string conversion.

However, since we legitimately need to support non-text response types, such as when you use response.blob() or response.arrayBuffer(), our only option is to unconditionally set xhr.responseType = 'blob' in advance, because at request time we can't know which type will the user need to consume later.

So for now, I don't see a way how we can avoid opting into blob mode when just reading text. If anyone has ideas, I would like to hear them.

[jimmywarting]

I'm running a forked version of your fetch in order to support streams but i don't think you would want it, adds to much dependencies.

I'm building it in es6 and always making sure ReadableStream, URLSearchParam, blob, typed arrays exist as polyfills, i'm also including core.js to support everything web-streams-polyfill needs

But i'm not here to talk about that. The way i'm able to stream is by not setting any responseType so it's just plain text. That way I can use the progress event and get part of the response and then be able to stream it. if i would have used any other kind (arraybuffer, blob) then i wouldn't be able to slice it and stream it.

So you maybe want to consider using just plain text if you ever want to support streams
you can always transform the text to an arraybuffer, json or blob later.
one thing you should be worrying about then is encoding if you want to turn a binary text into a blob.
The solution is simple really, you just have to add this:

xhr.overrideMimeType('text/plain; charset=x-user-defined')

if you are any interested in my fork then i can share it with you

[mislav]

So you maybe want to consider using just plain text if you ever want to support streams
you can always transform the text to an arraybuffer, json or blob later.

Interesting. I'd like to see a PR that makes this change, if you have the time. I'd prefer to see it separate from the streaming implementation, for clarity.

[jimmywarting]

My version is quite different cuz everything is transformed into stream no mather what you put in as argument into the Response constructor (the initbody fn). Even FormData gets transformed into a buffer/stream since i'm able to read it with the newest spec. I don't have _bodyText, _bodyBlob, _bodyFormData, _bodyArrayBuffer any longer and treating everything as a stream since you need to be able to stream it anyway and truly knowing the state of bodyUsed and locked to a consumer,

So you could say that mine is built all around buffers/streams, where as you still keep the reference to string, Blob, FormData, typedArray etc. So it's not some easy cherry picking from my code exactly your response methods are very conditional... Mine is closer to the spec but ways a lot more (due to dependencies)

But it maybe dosen't have to be so complicated as it may seems

how about just switching

      if ('responseType' in xhr && support.blob) {
        xhr.responseType = 'blob'
      }

to

      xhr.overrideMimeType('text/plain; charset=x-user-defined')

I think it will mostly work out of the box. you are already handeling transformation of a string to a blob already in your response method when you do new Response("text").blob()

This is just exactly the reason why i went with my fork instead of your polyfill

Only gotcha might be that some text response might not be the same when you call .text() since you no longer do the auto encoding stuff that browser dose

The magic happens in line 5, which overrides the MIME type, forcing the browser to treat it as plain text, using a user-defined character set. This tells the browser not to parse it, and to let the bytes pass through unprocessed.
mdn: Sending and Receiving Binary Data

[jimmywarting]

if you use overrideMimeType then you don't get the original content-type :(
setting it to responseType = arraybuffer worked much better 👍

[lukechinworth]

I am also having this issue in opera 25 running on a tv: xhr.onerror is called right after xhr.send. If I remove xhr.responseType = 'blob', the request succeeds.

One option may be to conditionally set responseType based on the response headers in the onreadystatechange callback.

      xhr.onreadystatechange = function() {
        if (
          xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED &&
          xhr.status === 200 &&
          'responseType' in xhr &&
          support.blob &&
          xhr.getResponseHeader('Content-Type') !== 'application/json'
        ) {
            xhr.responseType = 'blob'
        }
      };

This let's the tests pass, but I'm not sure about any other content types we would need to skip blob.

[jimmywarting]

We don't have to conditionally set the response type to blob. We need to change the response type to arraybuffer

[fab1o]

If anyone has ideas, I would like to hear them.

@mislav

We are experiencing a performance hit with a lower powered device with one of our users - Apparently it runs out of memory.
We did this change and it fixed it https://github.com/github/fetch/pull/752/files

Now, I would like to discuss the best solution to see if can get this fixed here too.

[JakeChampion]

I believe this is fixed by #752 -- If I'm wrong and this is still an issue, please comment and I will reopen the issue and investigate some more :-)