46982 2010-10-01 07:04:26 -0700 Setting href protocol attribute on malformed URL is incorrect when page is served over HTTP 2022-08-05 09:15:31 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED WONTFIX P2 Normal --- 47348 1 steveblock webkit-unassigned abarth ahmad.saleem792 ap bfulgham brettw fishd rniwa steveblock yael oldest_to_newest 288233 0 steveblock 2010-10-01 07:04:26 -0700 LayoutTest fast/dom/HTMLAnchorElement/set-href-attribute-protocol.html tests setting the protocol attribute of the href of an anchor element. A particular part of this test is as follows ... debug("Set protocol to http on malformed URL"); a.href = "foo:??bar"; a.protocol = "http"; shouldBe("a.href", "'http:??bar'"); I noticed that when the test is served over HTTP, rather than as a file:// URL as is currently the case, this test fails as follows ... Chromium (including ToT WebKit): FAIL - http://host/??bar Safari (including ToT WebKit) : FAIL - http://host/path??bar Android (including ToT WebKit) : FAIL - http://host/path??bar The test passes on recent versions of FF and IE. A test case, which duplicates the existing test as an HTTP test, is attached. The spec - http://dev.w3.org/html5/spec/urls.html - suggests that the expected result in the test is correct irrespective of the scheme over which the page is served, so it looks to me like this should be fixed. 288235 1 69464 steveblock 2010-10-01 07:08:59 -0700 Created attachment 69464 Test case 288600 2 yael 2010-10-01 17:29:17 -0700 I can make the test case even smaller to show the error: var a = document.createElement('a'); a.href="http:??bar"; alert(a.href); The reason for the failure is in KURL.cpp, line 401. if (p[1] != '/' && equalIgnoringCase(base.protocol(), String(str, p - str)) && base.isHierarchical()) We compare the protocol of the base url to the protocol of the new utl, and if they match, and the base url is hierarchical, then we assume that either the new url is hierarchical or relative. I don't think that assumption is valid. 288884 3 steveblock 2010-10-04 05:28:10 -0700 Considering your simpler test case ... Served from http://host/path document.createElement('a').href = 'http:?query'; 'http://host/path?query' is the correct result provided we're following the non-strict parsing rules defined at http://tools.ietf.org/html/rfc3986#section-5.2.2. I think that the logic you mention at line 401 of KURL.cpp is providing exactly this non-strict parsing - we ignore the scheme if it's the same as that of the base URI. If we were to use the strict parsing rules, then the result should be 'http:?query'. This matches the case when the scheme doesn't match that of the base URI, as is the case when the layout tests are served from file://. As for the test case I originally mentioned, this still looks like a bug to me, as the protocol is being set on the resolved URL, which should be 'foo:?query' ... Served from http://host/path a.href = "foo:?query"; a.protocol = "http"; shouldBe("a.href", "'http:?query'"); Chromium (including ToT WebKit): FAIL - http://host/?query Safari (including ToT WebKit) : FAIL - http://host/path?query Android (including ToT WebKit) : FAIL - http://host/path?query Note that the reverse case works as expected ... Served from http://host/path a.href = "http:?query"; a.protocol = "foo"; shouldBe("a.href", "'foo://host/path?query'"); 290986 4 steveblock 2010-10-07 07:18:15 -0700 > As for the test case I originally mentioned, this still looks like a bug to me, as the protocol is being set on the > resolved URL, which should be 'foo:?query' ... > Served from http://host/path > a.href = "foo:?query"; > a.protocol = "http"; > shouldBe("a.href", "'http:?query'"); > > Chromium (including ToT WebKit): FAIL - http://host/?query > Safari (including ToT WebKit) : FAIL - http://host/path?query > Android (including ToT WebKit) : FAIL - http://host/path?query Having looked into this more carefully, I'm starting to think the Safari/Android behaviour is correct. The spec says that href attribute is subject to changes in base URL and should be re-resolved when the base URL changes. So the href attribute should 'store' the unresolved URL, and it is resolved relative to the base URL only when the getter is called. So it makes sense that .. a.href = "foo:?query"; a.protocol = "http"; shouldBe("a.href", "'http://host/path?query'"); ... should be identical in behaviour to ... a.href = "http:?query"; shouldBe("a.href", "'http://host/path?query'"); Can anybody confirm that this desired behaviour is correct? If so, then only Chromium is buggy. In any case, I think I've found a related, more fundamental bug, which should probably be fixed first - Bug 47348 291085 5 abarth 2010-10-07 10:10:13 -0700 Sadly, this stuff is a mess. I'm interested to hear what Brett thinks. 291461 6 brettw 2010-10-07 23:10:10 -0700 To answer the question, if you have href="http://google.com" and you ask for the path component, should it be empty or should it be "/"? Another question would he "HTTP://google.com/" should the protocol be "http" or "HTTP". Currently Chromium will definitely give you a slash and "http" because it resolves the full URL all the time. We have three different path canonicalization routines currently. One for hierarchical (http and the like), one for file URLs, and one for data/javascript URLs and the like. Say the page says a.path = "/foo?bar#baz" I think we have to escape the ? and the # or else this won't just be setting the path. Yet if the algorithm is supposed to be "parse this URL and do a string substitution without canonicalization" then we would need yet a fourth mode where we just escape characters that change the parsing of the URL. I think our behavior makes sense and I haven't heard of any real world problems with it. Changing this, even if it would make it more correct, would introduce a lot of complexity for almost no benefit. 291464 7 ap 2010-10-07 23:23:07 -0700 But isn't it even less desirable to change KURL to match? 291466 8 abarth 2010-10-07 23:24:37 -0700 (In reply to comment #7) > But isn't it even less desirable to change KURL to match? The most desirable thing, IMHO, is to unfork our URL code. As long as we have two pieces of code doing the same job, we're going to have two different behaviors. 291471 9 brettw 2010-10-07 23:35:40 -0700 I wasn't arguing that KURL should be changed in this respect. 291516 10 steveblock 2010-10-08 02:25:35 -0700 OK, so it sounds like this should be WONTFIX. I'd appreciate your feedback on Bug 47348 though, to see it that should be fixed. 294060 11 steveblock 2010-10-14 05:21:51 -0700 The thinking is that this isn't worth fixing due to the complexity it would introduce. However, this assumption may change once Bug 47348 is fixed, as that may involve changing much of this code. Leaving this bug open until Bug 47348 is fixed. 1889117 12 ahmad.saleem792 2022-08-05 07:52:02 -0700 I took the test case from attached and changed it into JSFiddle: Link - https://jsfiddle.net/9drs1ofw/show Following are results across browsers: *** Safari 15.6 on macOS 12.5 *** FAIL a.href should be https://www.mydomain.com/path/. Was null://www.mydomain.com/path/. ^ Safari fail only in this. *** Firefox Nightly 105 *** FAIL a.href should be https://www.mydomain.com/path/. Was null://www.mydomain.com/path/. *** Chrome Canary 106 *** FAIL a.href should be https://www.mydomain.com/path/. Was null://www.mydomain.com/path/. FAIL a.href should be http:??bar. Was http:/??bar. _______ Do we need to fix the failing test case? Just wanted to share updated test results. Thanks! 1889129 13 bfulgham 2022-08-05 09:15:31 -0700 It seems like the standard has moved in this space, and all browsers agree on our current behavior. Seems like WONTFIX to me. 69464 2010-10-01 07:08:59 -0700 2010-10-01 07:08:59 -0700 Test case hrefSetProtocolTestCase.txt text/plain 5346 steveblock SW5kZXg6IExheW91dFRlc3RzL2h0dHAvdGVzdHMvdXJpL3NldC1ocmVmLWF0dHJpYnV0ZS1wcm90 b2NvbC1odHRwLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9odHRwL3Rl c3RzL3VyaS9zZXQtaHJlZi1hdHRyaWJ1dGUtcHJvdG9jb2wtaHR0cC1leHBlY3RlZC50eHQJKHJl dmlzaW9uIDApCisrKyBMYXlvdXRUZXN0cy9odHRwL3Rlc3RzL3VyaS9zZXQtaHJlZi1hdHRyaWJ1 dGUtcHJvdG9jb2wtaHR0cC1leHBlY3RlZC50eHQJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMjcg QEAKK1Rlc3Qgc2V0dGluZyB0aGUgcHJvdG9jb2wgYXR0cmlidXRlIG9mIHRoZSBVUkwgaW4gSFRN TEFuY2hvckVsZW1lbnQgd2hlbiB0aGUgcGFnZSBpcyBzZXJ2ZWQgb3ZlciBIVFRQLgorCitPbiBz dWNjZXNzLCB5b3Ugd2lsbCBzZWUgYSBzZXJpZXMgb2YgIlBBU1MiIG1lc3NhZ2VzLCBmb2xsb3dl ZCBieSAiVEVTVCBDT01QTEVURSIuCisKK0Jhc2ljIHRlc3QKK1BBU1MgYS5ocmVmIGlzICdodHRw LWZvbzovL3d3dy5teWRvbWFpbi5jb20vcGF0aC8nCitTZXQgYSBwcm90b2NvbCB0aGF0IGNvbnRh aW5zICc6JworUEFTUyBhLmhyZWYgaXMgJ2h0dHA6Ly93d3cubXlkb21haW4uY29tL3BhdGgvJwor U2V0IGEgcHJvdG9jb2wgdGhhdCBjb250YWlucyBpbnZhbGlkIGNoYXJhY3RlcnMKK1BBU1MgYS5o cmVmIGlzICdodHRwczovL3d3dy5teWRvbWFpbi5jb20vcGF0aC8nCitTZXQgYSBwcm90b2NvbCB0 byBhIFVSTCB3aXRoIGludmFsaWQgaG9zdCBuYW1lCitQQVNTIGEuaHJlZiBpcyAnZm9vOl5eJwor U2V0IGEgcHJvdG9jb2wgdGhhdCBzdGFydHMgd2l0aCAnOicKK1BBU1MgYS5ocmVmIGlzICdodHRw czovL3d3dy5teWRvbWFpbi5jb20vcGF0aC8nCitTZXQgcHJvdG9jb2wgdG8gbnVsbAorUEFTUyBh LmhyZWYgaXMgJ2h0dHBzOi8vd3d3Lm15ZG9tYWluLmNvbS9wYXRoLycKK1NldCBwcm90b2NvbCB0 byBlbXB0eSBzdHJpbmcKK1BBU1MgYS5ocmVmIGlzICdodHRwczovL3d3dy5teWRvbWFpbi5jb20v cGF0aC8nCitTZXQgcHJvdG9jb2wgdG8gaHR0cCBvbiBtYWxmb3JtZWQgVVJMCitQQVNTIGEuaHJl ZiBpcyAnaHR0cDo/P2JhcicKK1NldCBwcm90b2NvbCB0byBhIFVSTCB3aGljaCBwb2ludHMgdG8g YSBsb2NhbCBmaWxlCitQQVNTIGEuaHJlZiBpcyAnZi1vbzpwYXRoJworU2V0IHByb3RvY29sIHRv IHVuZGVmaW5lZAorUEFTUyBhLmhyZWYgaXMgJ3VuZGVmaW5lZDovL3d3dy5teWRvbWFpbi5jb20v cGF0aC8nCisKK1RFU1QgQ09NUExFVEUKKwpJbmRleDogTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy91 cmkvc2V0LWhyZWYtYXR0cmlidXRlLXByb3RvY29sLWh0dHAuaHRtbAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBM YXlvdXRUZXN0cy9odHRwL3Rlc3RzL3VyaS9zZXQtaHJlZi1hdHRyaWJ1dGUtcHJvdG9jb2wtaHR0 cC5odG1sCShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy91cmkvc2V0LWhy ZWYtYXR0cmlidXRlLXByb3RvY29sLWh0dHAuaHRtbAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSwx MjEgQEAKKzxodG1sPgorICA8aGVhZD4KKyAgICA8c2NyaXB0PgorICAgICAgZnVuY3Rpb24gZGVi dWcobXNnKQorICAgICAgeworICAgICAgICAgIHZhciBzcGFuID0gZG9jdW1lbnQuY3JlYXRlRWxl bWVudCgic3BhbiIpOworICAgICAgICAgIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJjb25zb2xl IikuYXBwZW5kQ2hpbGQoc3Bhbik7IC8vIGluc2VydCBpdCBmaXJzdCBzbyBYSFRNTCBrbm93cyB0 aGUgbmFtZXNwYWNlCisgICAgICAgICAgc3Bhbi5pbm5lckhUTUwgPSBtc2cgKyAnPGJyIC8+JzsK KyAgICAgIH0KKworICAgICAgZnVuY3Rpb24gdGVzdFBhc3NlZChtc2cpCisgICAgICB7CisgICAg ICAgICAgZGVidWcoJzxzcGFuPjxzcGFuIGNsYXNzPSJwYXNzIj5QQVNTPC9zcGFuPiAnICsgbXNn ICsgJzwvc3Bhbj4nKTsKKyAgICAgIH0KKworICAgICAgZnVuY3Rpb24gdGVzdEZhaWxlZChtc2cp CisgICAgICB7CisgICAgICAgICAgZGVidWcoJzxzcGFuPjxzcGFuIGNsYXNzPSJmYWlsIj5GQUlM PC9zcGFuPiAnICsgbXNnICsgJzwvc3Bhbj4nKTsKKyAgICAgIH0KKworICAgICAgZnVuY3Rpb24g c2hvdWxkQmUoX2EsIF9iKSB7CisgICAgICAgICAgdmFyIF9hdiA9IGV2YWwoX2EpOworICAgICAg ICAgIHZhciBfYnYgPSBldmFsKF9iKTsKKyAgICAgICAgICBpZiAoX2F2ID09IF9idikgeworICAg ICAgICAgICAgICB0ZXN0UGFzc2VkKF9hICsgIiBpcyAiICsgX2IpOworICAgICAgICAgIH0gZWxz ZSB7CisgICAgICAgICAgICAgIHRlc3RGYWlsZWQoX2EgKyAiIHNob3VsZCBiZSAiICsgX2J2ICsg Ii4gV2FzICIgKyBfYXYgKyAiLiIpOworICAgICAgICAgIH0KKyAgICAgIH0KKworICAgICAgaWYg KHdpbmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikKKyAgICAgICAgICBsYXlvdXRUZXN0Q29udHJv bGxlci5kdW1wQXNUZXh0KCk7CisgICAgPC9zY3JpcHQ+CisgIDwvaGVhZD4KKyAgPGJvZHk+Cisg ICAgPHA+VGVzdCBzZXR0aW5nIHRoZSBwcm90b2NvbCBhdHRyaWJ1dGUgb2YgdGhlIFVSTCBpbiBI VE1MQW5jaG9yRWxlbWVudCB3aGVuIHRoZSBwYWdlIGlzIHNlcnZlZCBvdmVyIEhUVFAuPC9wPgor ICAgIDxwPk9uIHN1Y2Nlc3MsIHlvdSB3aWxsIHNlZSBhIHNlcmllcyBvZiAiPHNwYW4gY2xhc3M9 InBhc3MiPlBBU1M8L3NwYW4+IiBtZXNzYWdlcywgZm9sbG93ZWQgYnkgIjxzcGFuIGNsYXNzPSJw YXNzIj5URVNUIENPTVBMRVRFPC9zcGFuPiIuPC9wPgorICAgIDxkaXYgaWQ9ImNvbnNvbGUiPjwv ZGl2PgorICAgIDxzY3JpcHQ+CisKK3ZhciBhID0gZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgnYScp OworCitkZWJ1ZygiQmFzaWMgdGVzdCIpOworYS5ocmVmID0gImh0dHBzOi8vd3d3Lm15ZG9tYWlu LmNvbS9wYXRoLyI7CithLnByb3RvY29sID0gImh0dHAtZm9vIjsKK3Nob3VsZEJlKCJhLmhyZWYi LCAiJ2h0dHAtZm9vOi8vd3d3Lm15ZG9tYWluLmNvbS9wYXRoLyciKTsKKworLy8gSUU4IHRocm93 cyAiSW52YWxpZCBhcmd1bWVudCIgZXhjZXB0aW9uLgorZGVidWcoIlNldCBhIHByb3RvY29sIHRo YXQgY29udGFpbnMgJzonIik7Cit0cnkgeworYS5ocmVmID0gImh0dHBzOi8vd3d3Lm15ZG9tYWlu LmNvbS9wYXRoLyI7CithLnByb3RvY29sID0gImh0dHA6Zm9vIjsKK3Nob3VsZEJlKCJhLmhyZWYi LCAiJ2h0dHA6Ly93d3cubXlkb21haW4uY29tL3BhdGgvJyIpOworfSBjYXRjaChlKSB7CitkZWJ1 ZygiRXhjZXB0aW9uOiAiICsgZS5kZXNjcmlwdGlvbik7Cit9CisKKy8vIElFOCB0aHJvd3MgIklu dmFsaWQgYXJndW1lbnQiIGV4Y2VwdGlvbi4KK2RlYnVnKCJTZXQgYSBwcm90b2NvbCB0aGF0IGNv bnRhaW5zIGludmFsaWQgY2hhcmFjdGVycyIpOwordHJ5IHsKK2EuaHJlZiA9ICJodHRwczovL3d3 dy5teWRvbWFpbi5jb20vcGF0aC8iOworYS5wcm90b2NvbCA9ICJodHRwXmZvbyI7CitzaG91bGRC ZSgiYS5ocmVmIiwgIidodHRwczovL3d3dy5teWRvbWFpbi5jb20vcGF0aC8nIik7Cit9IGNhdGNo KGUpIHsKK2RlYnVnKCJFeGNlcHRpb246ICIgKyBlLmRlc2NyaXB0aW9uKTsKK30KKworLy8gVGhl IGV4cGVjdGVkIGJlaGF2aW9yIHNob3VsZCBjaGFuZ2Ugd2hlbiB0aGUgY2hhcmFjdGVyIHRhYmxl IGlzIHVwZGF0ZWQuCisvLyBJRTggZW5jb2RlcyAnXicgaW4gdGhlIGhvc3QuCitkZWJ1ZygiU2V0 IGEgcHJvdG9jb2wgdG8gYSBVUkwgd2l0aCBpbnZhbGlkIGhvc3QgbmFtZSIpOworYS5ocmVmID0g Img6Xl4iOworYS5wcm90b2NvbCA9ICJmb28iOworc2hvdWxkQmUoImEuaHJlZiIsICInZm9vOl5e JyIpOworCisvLyBJRTggdGhyb3dzICJJbnZhbGlkIGFyZ3VtZW50IiBleGNlcHRpb24uCit0cnkg eworZGVidWcoIlNldCBhIHByb3RvY29sIHRoYXQgc3RhcnRzIHdpdGggJzonIik7CithLmhyZWYg PSAiaHR0cHM6Ly93d3cubXlkb21haW4uY29tL3BhdGgvIjsKK2EucHJvdG9jb2wgPSAiOmh0dHAi Oworc2hvdWxkQmUoImEuaHJlZiIsICInaHR0cHM6Ly93d3cubXlkb21haW4uY29tL3BhdGgvJyIp OworfSBjYXRjaChlKSB7CitkZWJ1ZygiRXhjZXB0aW9uOiAiICsgZS5kZXNjcmlwdGlvbik7Cit9 CisKKy8vIElFOCBjb252ZXJ0cyBudWxsIHRvICJudWxsIiwgd2hpY2ggaXMgbm90IHRoZSByaWdo dCB0aGluZyB0byBkby4KK2RlYnVnKCJTZXQgcHJvdG9jb2wgdG8gbnVsbCIpOworYS5ocmVmID0g Imh0dHBzOi8vd3d3Lm15ZG9tYWluLmNvbS9wYXRoLyI7CithLnByb3RvY29sID0gbnVsbDsKK3No b3VsZEJlKCJhLmhyZWYiLCAiJ2h0dHBzOi8vd3d3Lm15ZG9tYWluLmNvbS9wYXRoLyciKTsKKwor Ly8gSUU4IHRocm93cyAiSW52YWxpZCBhcmd1bWVudCIgZXhjZXB0aW9uLgordHJ5IHsKK2RlYnVn KCJTZXQgcHJvdG9jb2wgdG8gZW1wdHkgc3RyaW5nIik7CithLmhyZWYgPSAiaHR0cHM6Ly93d3cu bXlkb21haW4uY29tL3BhdGgvIjsKK2EucHJvdG9jb2wgPSAiIjsKK3Nob3VsZEJlKCJhLmhyZWYi LCAiJ2h0dHBzOi8vd3d3Lm15ZG9tYWluLmNvbS9wYXRoLyciKTsKK30gY2F0Y2goZSkgeworZGVi dWcoIkV4Y2VwdGlvbjogIiArIGUuZGVzY3JpcHRpb24pOworfQorCisvLyBGaXJlZm94IDMuNS4y IHRyaWVzIHRvIGJ1aWxkIGEgaGllcmFyY2hpY2FsIFVSTC4KK2RlYnVnKCJTZXQgcHJvdG9jb2wg dG8gaHR0cCBvbiBtYWxmb3JtZWQgVVJMIik7CithLmhyZWYgPSAiZm9vOj8/YmFyIjsKK2EucHJv dG9jb2wgPSAiaHR0cCI7CitzaG91bGRCZSgiYS5ocmVmIiwgIidodHRwOj8/YmFyJyIpOworCisv LyBJRTgga2VlcHMgdGhlIHByb3RvY29sIGlmIGl0IGlzICdjOicuCitkZWJ1ZygiU2V0IHByb3Rv Y29sIHRvIGEgVVJMIHdoaWNoIHBvaW50cyB0byBhIGxvY2FsIGZpbGUiKTsKK2EuaHJlZiA9ICJj OlxwYXRoIjsKK2EucHJvdG9jb2wgPSAiZi1vbyI7CitzaG91bGRCZSgiYS5ocmVmIiwgIidmLW9v OnBhdGgnIik7CisKK2RlYnVnKCJTZXQgcHJvdG9jb2wgdG8gdW5kZWZpbmVkIik7CithLmhyZWYg PSAiaHR0cHM6Ly93d3cubXlkb21haW4uY29tL3BhdGgvIjsKK2EucHJvdG9jb2wgPSB1bmRlZmlu ZWQ7CitzaG91bGRCZSgiYS5ocmVmIiwgIid1bmRlZmluZWQ6Ly93d3cubXlkb21haW4uY29tL3Bh dGgvJyIpOworCitkZWJ1ZygnPGJyIC8+PHNwYW4gY2xhc3M9InBhc3MiPlRFU1QgQ09NUExFVEU8 L3NwYW4+Jyk7CisgICAgPC9zY3JpcHQ+CisgIDwvYm9keT4KKzwvaHRtbD4K