403480 - setting nsIURI::port to its default value shouldn't be possible

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[:Gavin Sharp [email: [e-mail]]]

The existing code seems to make it impossible to set port to it's default value except in one specific case: set it to a different port and then set it back to the default. This seems inconsistent.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 2

[:Gavin Sharp [email: [e-mail]]]

The reason I looked into this was the patch for bug 396316, which has:

+  // If the URI explicitly specified a port, only include it when
+  // it's not the default. (We never want "http://foo.com:80")
+  port = uri.port;
+  if (port != -1) {
+      var handler = this._ioService.getProtocolHandler(scheme);
+      if (port != handler.defaultPort)
+          realm += ":" + port;
+  }

I'm assuming this change means that the getProtocolHandler/defaultPort code wouldn't be necessary, and it could just check for -1. Is that a safe assumption?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 288311 [details] [diff] [review]
patch

(I suppose the comment isn't quite right in the case of mDefaultPort == -1, suggestions for a better one are welcome.)

Comment 4

[Justin Dolske [:Dolske]]

Hmm, while we're here perhaps an input value of "-1" (or any negative number, really) should also be illegal? "http://google.com:-1" doesn't seem like it should work, but it does.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 288311 [details] [diff] [review]
patch

Simple correctness fix that'll help simplify the patch for blocker bug 396316.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

mozilla/netwerk/base/src/nsStandardURL.cpp 	1.102