corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1433958 - If setting the host succeeds, SetHostPort should not return an error code r=mayhemer

Browse Source 
WebPlatformTests expect that when calling
url.host = "host:" // port missing
url.host = "host:65536" // port too big
url.host = "host:bla" // invalid port
that the hostname will be set, but the port will be left unchanged.
Since DOM APIs are the only consumers for SetHostPort it means we can change this behaviour to match the WPT expectations.
As such, SetHostPort will return NS_OK if setting the host succeded, and will ignore if setting the port failed.

MozReview-Commit-ID: LoMw8hCWlCv
This commit is contained in:
Valentin Gosu
2018-02-26 20:43:47 +01:00
parent 4c423bbfee
commit 75f40b01e3
3 changed files with 27 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
netwerk/base/nsIURI.idl
View File

@@ -154,6 +154,8 @@ interface nsIURI : nsISupports
*
* If this attribute is set to a value that only has a host part, the port
* will not be reset. To reset the port as well use setHostAndPort.
* If setting the host succeeds, this method will return NS_OK, even if
* setting the port fails (error in parsing the port, or value out of range)
*/
attribute AUTF8String hostPort;

36
netwerk/base/nsStandardURL.cpp
View File

@@ -1978,25 +1978,25 @@ nsStandardURL::SetHostPort(const nsACString &aValue)
nsresult rv = SetHost(Substring(start, iter));
NS_ENSURE_SUCCESS(rv, rv);
// Also set the port if needed.
if (iter != end) {
iter++;
if (iter != end) {
nsCString portStr(Substring(iter, end));
nsresult rv;
int32_t port = portStr.ToInteger(&rv);
if (NS_SUCCEEDED(rv)) {
rv = SetPort(port);
NS_ENSURE_SUCCESS(rv, rv);
} else {
// Failure parsing port number
return NS_ERROR_MALFORMED_URI;
}
} else {
// port number is missing
return NS_ERROR_MALFORMED_URI;
}
if (iter == end) {
// does not end in colon
return NS_OK;
}
iter++; // advance over the colon
if (iter == end) {
// port number is missing
return NS_OK;
}
nsCString portStr(Substring(iter, end));
int32_t port = portStr.ToInteger(&rv);
if (NS_FAILED(rv)) {
// Failure parsing the port number
return NS_OK;
}
Unused << SetPort(port);
return NS_OK;
}

9
netwerk/test/unit/test_standardurl.js
View File

@@ -208,8 +208,13 @@ add_test(function test_ipv6_fail()
Assert.throws(() => { url = url.mutate().setHostPort("2001]:1").finalize(); }, "bad IPv6 address");
Assert.throws(() => { url = url.mutate().setHostPort("2001:1]").finalize(); }, "bad IPv6 address");
Assert.throws(() => { url = url.mutate().setHostPort("").finalize(); }, "Empty hostPort should fail");
Assert.throws(() => { url = url.mutate().setHostPort("[2001::1]:").finalize(); }, "missing port number");
Assert.throws(() => { url = url.mutate().setHostPort("[2001::1]:bad").finalize(); }, "bad port number");
// These checks used to fail, but now don't (see bug 1433958 comment 57)
url = url.mutate().setHostPort("[2001::1]:").finalize();
Assert.equal(url.spec, "http://[2001::1]/");
url = url.mutate().setHostPort("[2002::1]:bad").finalize();
Assert.equal(url.spec, "http://[2002::1]/");
run_next_test();
});