WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 168724
XMLHttpRequest: do not sniff text/html, and do not sniff XML when responseType is set to "text"
https://bugs.webkit.org/show_bug.cgi?id=168724
Summary
XMLHttpRequest: do not sniff text/html, and do not sniff XML when responseTyp...
Anne van Kesteren
Reported
2017-02-22 06:53:58 PST
See
http://w3c-test.org/XMLHttpRequest/responsetext-decoding.htm
. Context:
https://github.com/w3c/web-platform-tests/pull/4935
.
Attachments
WIP Patch
(72.36 KB, patch)
2017-10-11 10:14 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(12.62 KB, patch)
2017-10-11 13:26 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(12.63 KB, patch)
2017-10-11 13:35 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
(1.33 MB, application/zip)
2017-10-11 14:24 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews117 for mac-elcapitan
(2.02 MB, application/zip)
2017-10-11 14:41 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews100 for mac-elcapitan
(995.80 KB, application/zip)
2017-10-11 14:41 PDT
,
Build Bot
no flags
Details
Patch
(25.37 KB, patch)
2017-10-11 14:49 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2017-10-11 09:10:05 PDT
Blink patch:
https://bugs.chromium.org/p/chromium/issues/detail?id=695045
Chris Dumez
Comment 2
2017-10-11 10:14:11 PDT
Created
attachment 323424
[details]
WIP Patch
Chris Dumez
Comment 3
2017-10-11 13:26:44 PDT
Created
attachment 323453
[details]
Patch
Chris Dumez
Comment 4
2017-10-11 13:35:43 PDT
Created
attachment 323455
[details]
Patch
Build Bot
Comment 5
2017-10-11 14:24:32 PDT
Comment on
attachment 323455
[details]
Patch
Attachment 323455
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/4827931
New failing tests: http/tests/xmlhttprequest/response-encoding.html
Build Bot
Comment 6
2017-10-11 14:24:33 PDT
Created
attachment 323461
[details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 7
2017-10-11 14:41:11 PDT
Comment on
attachment 323455
[details]
Patch
Attachment 323455
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/4827990
New failing tests: http/tests/xmlhttprequest/response-encoding.html
Build Bot
Comment 8
2017-10-11 14:41:12 PDT
Created
attachment 323462
[details]
Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 9
2017-10-11 14:41:52 PDT
Comment on
attachment 323455
[details]
Patch
Attachment 323455
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/4828066
New failing tests: http/tests/xmlhttprequest/response-encoding.html
Build Bot
Comment 10
2017-10-11 14:41:53 PDT
Created
attachment 323463
[details]
Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Chris Dumez
Comment 11
2017-10-11 14:49:53 PDT
Created
attachment 323465
[details]
Patch
youenn fablet
Comment 12
2017-10-11 15:03:44 PDT
Patch seems fine. I guess the compat issue is really low since Chrome probably shipped this a while ago.
Chris Dumez
Comment 13
2017-10-11 15:09:31 PDT
(In reply to youenn fablet from
comment #12
)
> Patch seems fine. > I guess the compat issue is really low since Chrome probably shipped this a > while ago.
Full disclosure, the Blink patch landed yesterday so they did not ship this yet. They will definitely ship it before us though and we can always revert if they run into trouble.
youenn fablet
Comment 14
2017-10-11 15:17:42 PDT
Do you know what IE/MS are doing here?
youenn fablet
Comment 15
2017-10-11 15:17:56 PDT
IE/Firefox...
Chris Dumez
Comment 16
2017-10-11 15:20:44 PDT
(In reply to youenn fablet from
comment #15
)
> IE/Firefox...
I do not know about IE but Firefox aligned with the spec. Info available at: -
https://github.com/w3c/web-platform-tests/pull/4935
-
https://bugzilla.mozilla.org/show_bug.cgi?id=1341260
So basically, both Firefox and Chrome aligned. Anne may know about IE / Edge.
Ryosuke Niwa
Comment 17
2017-10-11 15:21:11 PDT
Comment on
attachment 323465
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=323465&action=review
> Source/WebCore/xml/XMLHttpRequest.cpp:1062 > + }
Don't we need to check if the response is HTML? We used to do that in the old code.
Chris Dumez
Comment 18
2017-10-11 16:58:47 PDT
Comment on
attachment 323465
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=323465&action=review
>> Source/WebCore/xml/XMLHttpRequest.cpp:1062 >> + } > > Don't we need to check if the response is HTML? We used to do that in the old code.
No, if I add this check then a WPT subtest would no longer start passing. Also note that Chrome is not doing the HTML check either in this case.
> LayoutTests/imported/w3c/web-platform-tests/XMLHttpRequest/responsetext-decoding-expected.txt:3 > +PASS XMLHttpRequest: responseText decoding (text/html %3C!doctype%20html%3E%3Cmeta%20charset%3Dwindows-1252%3E%3Cx%3E%e6%a9%9f%3C%2Fx%3E empty)
This subtest would fail again.
WebKit Commit Bot
Comment 19
2017-10-11 17:27:27 PDT
Comment on
attachment 323465
[details]
Patch Clearing flags on attachment: 323465 Committed
r223217
: <
https://trac.webkit.org/changeset/223217
>
WebKit Commit Bot
Comment 20
2017-10-11 17:27:29 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 21
2017-10-11 17:28:48 PDT
<
rdar://problem/34946063
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug