Server-provided Content-Type:application/xml is altered to application/xhtml+xml |
|||||
Issue descriptionChrome Version: 59 Canary OS: All but iOS What steps will reproduce the problem? (1) Open http://w3c-test.org/dom/nodes/Document-createElement-namespace.html What is the expected result? No FAILs What happens instead? There are three fails about contentType. Please use labels and text to provide additional information. See https://bugs.chromium.org/p/chromium/issues/detail?id=700237#c10 and https://bugs.chromium.org/p/chromium/issues/detail?id=700237#c11 . None of Edge, Firefox and Safari doesn't have such weird behavior.
,
Mar 16 2017
"application/xml" is in the "sniffable mime types list", meaning we sniff for it despite the type. And if we find the xhtml+xml magic "number" (<html xmlns="http://www.w3.org/1999/xhtml"), we then change the mime type (Presumably since it's really XHTML, not just XML, and we want to render it as HTML). Anyhow, given the lack of comments or motivation behind this, and the fact that the result feeds into yet another black box that I know nothing about, I'd be reluctant to change this without understanding the motivation for the current behavior, and why it is wrong.
,
Mar 17 2017
Yeah, I think we'd need to factor in the context when deciding whether to sniff the mime type. There are contexts in which we shouldn't change an XML mime type, and there are context in which the browser is still expected to sniff. It might be worthwhile tossing in a few MIME sniffing test cases in to the w3c test suite so that we can make sure that the browsers are on the same page WRT sniffing. It's not clear from the report whether the observed differences in behavior are due to other browsers factoring in the resource fetch context. Suggested next steps: * Look into whether resource context is a factor in the observed differences in behavior. We definitely shouldn't be sniffing XML MIME types indiscriminately. At the same time, we shouldn't turn it off indiscriminately either. * Contribute good test cases to w3c test suite for https://mimesniff.spec.whatwg.org/ I'm hosed, but will put this on the backburner.
,
May 15 2017
I looked at the code, and found we had this behavior since the Chromium initial commit: https://chromium.googlesource.com/chromium/src/+blame/ff308e070f61164178200816832dc00a974f8430/net/base/mime_sniffer.cc#544 static const MagicNumber kMagicXML[] = { // We want to be very conservative in interpreting text/xml content as // XHTML -- we just want to sniff enough to make unit tests pass. // So we match explicitly on this, and don't match other ways of writing // it in semantically-equivalent ways. MAGIC_STRING("application/xhtml+xml", "<html xmlns=\"http://www.w3.org/1999/xhtml\""), According to the comment, XHTML sniff is just for testing, and rewriting application/xml to application/xhtml+xml is done by an accident. Actually, if a resource starts with <html xmlns='http://www.w3.org/1999/xhtml'> (single-quotes instead of double-quotes) or <html xmlns="http://www.w3.org/1999/xhtml"> (two spaces after <html), we don't alter the MIME type. Maybe we should pass type_hint to SniffXML(), and skip XHTML sniff for application/xml.
,
May 15 2017
Presumably those tests we wanted to pass existed for a reason? I'm not really sure of what, if any, fallout we could see here, so am reluctant to just sign off on the change.
,
May 16 2017
I searched the pre-initial-commit history. https://chrome-internal.googlesource.com/chrome/chrome-history/+/6371a0ddfcde7436bf01f641f5c7c30d7abd1d92%5E%21/#F1 added XML sniff code for text/xml, and https://chrome-internal.googlesource.com/chrome/chrome-history/+/f1d0548c457edc2716771fbbaf248d94a49f232c expanded for application/xml. The reason is explained by the following comment: +// XHTML payload, Content-Type: "text/xml": +// * IE 7: Render as XML +// * Firefox 2: Render as HTML +// * Safari 3: Render as HTML +// * Opera 9: Render as HTML +// The layout tests rely on us rendering this as HTML. +// But we're conservative in XHTML detection, as this runs afoul of the +// "don't detect dangerous mime types" rule. So this was just to pass layout tests, and nowadays we don't need it. I'm convinced that we can remove this behavior, and we should remove it for better security.
,
May 16 2017
But this is type application/xml, not text/xml.... Weird. Do we have a layout test that makes sure we render application/xml content as HTML (If we should...)?
,
May 16 2017
> Do we have a layout test that makes sure we render application/xml content as HTML (If we should...)? Yes. Layout tests ensures xhtml content with both of text/xml and application/xml rendered as XHTML. All of Chrome, Edge, Firefox, and Safari agree on this behavior.
,
May 16 2017
> Layout tests ensures xhtml content with both of text/xml and application/xml rendered as XHTML. I'm not sure that was true 10 years ago. I'm afraid httpd configuration for Chromium was wrong when the XHTML sniff was added.
,
May 22 2017
,
May 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cc9427a4b67b394747dde2f1b8279a0d3384892 commit 9cc9427a4b67b394747dde2f1b8279a0d3384892 Author: tkent <tkent@chromium.org> Date: Tue May 23 04:08:00 2017 net: Do not sniff XHTML content This CL removes XHTML content sniff for text/xml and application/xml. We altered the content-type to application/xhtml+xml if the root tag starts with the following: <html xmlns="http://www.w3.org/1999/xhtml" We stop it. This behavior has never been applied to <html xmlns='http://www.w3.org/1999/xhtml' or <html xmlns="http://www.w3.org/1999/xhtml" because this behavior was introduced to pass some layout tests. The behavior difference between application/xml and application/xhtml+xml is document.createElement(). This CL updates some tests. New behavior is interoperable with Edge, Firefox, and Safari. * LayoutTests changes ** external/wpt/dom/nodes/Document-createElement-namespace.html This CL fixes failing sub-tests. ** fast/dom/Document/xml-document-focus.xml Page rendering is changed because of document.createElement() in js-test.js. ** fast/dom/ping-attribute-no-crash.xml This assumes document.createElement() has XHTML behavior. This CL renames it to html/text_level_semantics/a-ping-no-crash.xhtml. ** http/tests/misc/createElementNamespace1.xml This CL fixes a failing sub-test about document.createElement(). BUG= 231927 , 702084 Review-Url: https://codereview.chromium.org/2883833002 Cr-Commit-Position: refs/heads/master@{#473805} [modify] https://crrev.com/9cc9427a4b67b394747dde2f1b8279a0d3384892/net/base/mime_sniffer.cc [modify] https://crrev.com/9cc9427a4b67b394747dde2f1b8279a0d3384892/net/base/mime_sniffer_unittest.cc [delete] https://crrev.com/6df0b5c128f7ebfe6c6cc3b8bd496ea3e13fa8cc/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Document-createElement-namespace-expected.txt [modify] https://crrev.com/9cc9427a4b67b394747dde2f1b8279a0d3384892/third_party/WebKit/LayoutTests/fast/dom/Document/xml-document-focus-expected.txt [rename] https://crrev.com/9cc9427a4b67b394747dde2f1b8279a0d3384892/third_party/WebKit/LayoutTests/html/text_level_semantics/a-ping-no-crash-expected.txt [rename] https://crrev.com/9cc9427a4b67b394747dde2f1b8279a0d3384892/third_party/WebKit/LayoutTests/html/text_level_semantics/a-ping-no-crash.xhtml [modify] https://crrev.com/9cc9427a4b67b394747dde2f1b8279a0d3384892/third_party/WebKit/LayoutTests/http/tests/misc/createElementNamespace1-expected.txt
,
May 23 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tkent@chromium.org
, Mar 16 2017