Bug 231138 – REGRESSION (r277818): XHR with requestType document broken for larger HTML files

• Component: DOM
• Status: RESOLVED FIXED

When requesting HTML documents via XHRHttpRequests the response object is missing even when the request itself was successful.

Ways to reproduce:

1. Go to https://www.decathlon.de
2. In the console execute this code
```
var xhr = new XMLHttpRequest();
xhr.withCredentials = true;
xhr.open('GET', 'https://www.decathlon.de/browse/c0-alle-sportarten-a-z/c1-fahrrad-welt/c3-mountainbike/_/N-obq78x', true); 
xhr.responseType = 'document';

xhr.addEventListener('readystatechange', () => {
  if (xhr.readyState === 4) {
    console.log('Success')
    console.log('Res: ' + xhr.response)
  }
});

// Handle errors of XHR connection
xhr.addEventListener('error', (e) => {
  
    console.log('Error', e)
});
xhr.send();
```

3. Observe that the xhr.response is null even though the request was successful.


The same issue is reproducable on a lot of domains but does not happen for all HTML files. It seems to be related to the size of the HTML document though. For example, if you swap the URL in the example above for this one https://www.decathlon.de/p/mountainbike-e-st-520-27-5-zoll-grau-gelb/_/R-p-311400 everything works fine and the response is available.

This issue is highly critical for a lot of our customers. Is there a workaround to still use XHR for those files until there is a fix for the issue?

[Alexey Proskuryakov]

Ugh. Has this been the case before Safari 15 too?

From looking at the code, there isn't a lot that can go wrong. Perhaps some confusion about MIME types that would be easy to see in a debugger.

As a workaround, it looks like fetching as string and parsing the response using DOMParser works.

It works perfectly fine in Safari 14 (tested on Mac with Version 14.1.2 (16611.3.10.1.6)). It only breaks in Safari 15 (tested on Mac with Release 133 (Safari 15.4, WebKit 16613.1.2.2)).

As I said, it breaks a lot of customer sites, so we see the error in a lot of production websites in Safari 15 both on Mac and iPhones.

Are you able to reproduce with the code snippet I supplied?

[Alexey Proskuryakov]

Yes, I can reproduce. Thank you for the easy to follow steps.

Please let us know if the suggested workaround doesn't help.

[Radar WebKit Bug Importer]

<rdar://problem/83823282>

(In reply to Alexey Proskuryakov from comment #3)
> Yes, I can reproduce. Thank you for the easy to follow steps.
> 
> Please let us know if the suggested workaround doesn't help.

I think the workaround works functionally. Since we use that code in a very performance critical phase, I am not sure it will work well for us though.

Not sure how it works in Safari in particular but in Chrome for example the native DOM parsing of the XHRHttpRequest is more efficient compared to using DOMParser with the document as string. The parsing of the XHR request is for example interlaced with JS execution of the page while DOMParser will block the main thread until the document is parsed (probably as it is not streamed into the parser)

[Chris Dumez]

r279000: BAD
r277000: GOOD

Will bisect.

[Chris Dumez]

Regression range:
https://trac.webkit.org/log/webkit/trunk?mode=follow_copy&rev=277818&stop_rev=277809

Almost certainly Antti's https://trac.webkit.org/r277818.

[Chris Dumez]

Created attachment 440066 [details]
Failing test

[Chris Dumez]

Created attachment 440068 [details]
Failing Test

[Chris Dumez]

Note that the test case I uploaded relies on the actual page from decathlon which may not be suitable to commit to our repo. I tried generating a large lorem Ipsum HTML file instead but it didn't reproduce.

[Jonathan Bedard]

Comment on attachment 440068 [details]
Failing Test

Changing this so it's not marked as a patch, EWS is having a rough time with it.

[Aakash Jain]

Marked as patch again to test EWS. EWS is able to process it efficiently now (after the fix in Bug 231190).

Hi folks,

I think things are more severely broken than just XHR with response type document.

Trying out the workaround via DOMParser I discovered that one is broken in iOS 15 as well. See bug report https://bugs.webkit.org/show_bug.cgi?id=231375

[Chris Dumez]

(In reply to erik.witt from comment #13)
> Hi folks,
> 
> I think things are more severely broken than just XHR with response type
> document.
> 
> Trying out the workaround via DOMParser I discovered that one is broken in
> iOS 15 as well. See bug report https://bugs.webkit.org/show_bug.cgi?id=231375

I guess that isn't too surprising. XHR response type document and DOMParser are probably using the same logic for parsing.

@antti: Should we consider reverting your change until we have a fix? Seems like a bad regression.

You are right, it is not too surprising but it eliminates the workaround Alexey suggested.

Is there another way to get to the parsed document that currently works in Safari 15? We urgently need one

[Antti Koivisto]

Created attachment 440596 [details]
Patch

[Antti Koivisto]

Created attachment 440597 [details]
Patch

[Antti Koivisto]

There are several possible workarounds, at least

1) Since the bug only happens when the parser yields before a script tag you can avoid it by removing or mangling any <script> in the input source before giving it to HTMLParser (scripts don't execute anyway).

2) Use <template> element

3) Use <iframe sandbox="">

[Antti Koivisto]

> giving it to HTMLParser (scripts don't execute anyway).

DOMParser that is.

[Darin Adler]

Comment on attachment 440597 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440597&action=review

> Source/WebCore/dom/DocumentParser.h:54
> +    virtual void appendSynchronously(RefPtr<StringImpl>&& inputSource) { append(WTFMove(inputSource)); }

Totally not about this patch: I do not think we should ever use the type RefPtr<StringImpl> since that’s exactly how String is implemented. I could understand StringImpl* or Ref<StringImpl>, but RefPtr<StringImpl> in particular is just an alternate way to say String, and I think it’s annoying that we mix the two.

[EWS]

Committed r283803 (242696@main): <https://commits.webkit.org/242696@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440597 [details].

Hi all,

sorry for the stupid question. But since the status is set to resolved, when does the fix land in the Safari release und which release will it be?

[Antti Koivisto]

Can't comment on release but I'll try to update this bug when the fix is available in a public beta.

[Alexey Proskuryakov]

*** Bug 231375 has been marked as a duplicate of this bug. ***

Hey folks!

Since Safari 15.1 has landed and the issue seems to still be there, is there already an open version somewhere where we can test the fix?

Or is there any way of seeing in which release this fix is included?

[Chris Dumez]

(In reply to erik.witt from comment #25)
> Hey folks!
> 
> Since Safari 15.1 has landed and the issue seems to still be there, is there
> already an open version somewhere where we can test the fix?
> 
> Or is there any way of seeing in which release this fix is included?

Safari Technology Preview 134 seems to have the fix:
https://developer.apple.com/safari/download/

I can confirm this has not shipped yet in mainline Safari.

[Antti Koivisto]

This should be fixed now in Safari 15.2