Bug 1433958 - If setting the host succeeds, SetHostPort should not return an error code
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
--- a/netwerk/base/nsIURI.idl
+++ b/netwerk/base/nsIURI.idl
@@ -149,16 +149,18 @@ interface nsIURI : nsISupports
attribute AUTF8String username;
attribute AUTF8String password;
/**
* The host:port (or simply the host, if port == -1).
*
* 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;
/**
* This function will always set a host and a port. If the port part is
* empty, the value of the port will be set to the default value.
*/
void setHostAndPort(in AUTF8String hostport);
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -1973,35 +1973,35 @@ nsStandardURL::SetHostPort(const nsACStr
// The format should be [2001::1]:80 where the port is optional
return NS_ERROR_MALFORMED_URI;
}
}
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;
}
// This function is different than SetHostPort in that the port number will be
// reset as well if aValue parameter does not contain a port port number.
NS_IMETHODIMP
nsStandardURL::SetHostAndPort(const nsACString &aValue)
{
--- a/netwerk/test/unit/test_standardurl.js
+++ b/netwerk/test/unit/test_standardurl.js
@@ -203,18 +203,23 @@ 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]10").finalize(); }, "bad IPv6 address");
Assert.throws(() => { url = url.mutate().setHostPort("[2001:1]10:20").finalize(); }, "bad IPv6 address");
Assert.throws(() => { url = url.mutate().setHostPort("[2001:1]:10:20").finalize(); }, "bad IPv6 address");
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("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();
});
add_test(function test_clearedSpec()
{
var url = stringToURL("http://example.com/path");
Assert.throws(() => { url = url.mutate().setSpec("http: example").finalize(); }, "set bad spec");
Assert.throws(() => { url = url.mutate().setSpec("").finalize(); }, "set empty spec");