This is a leftover from bug 479485 to be fixed. We should add an is-positive (or even better is > 0) check for the port number.
Needs to be fixed before final, don't think it needs to be in a beta, though.
Created attachment 472477 [details] [diff] [review] wip 1 This is a work in progress patch. I updated the test and went further and find out, that URL "http://foo.com:1234:80/bar" is correctly parsed and internally represented as "http://foo.com:1234/bar" - the ":80" from the port portion of the URL is read but then ignored. nsCAutoString::ToInteger is responsible for that. This is another example where we are completely failing to safely parse a string. I would like to use something better for parsing here, like lexical analyzer. See .  http://groups.google.ca/group/mozilla.dev.platform/browse_thread/thread/47bf5b4cb8203053
We could definitely use some better XPCOM parsing functions. Note that the DOM no longer uses the ns(C)String::ToInteger gunk, since it just didn't do what the DOM needed.
(In reply to comment #3) > We could definitely use some better XPCOM parsing functions. Do you know about any?
The point was we should create some. We don't have them now.
I'll simply update the patch to check the string is whole just numbers and then use ToInteger on it. Simple and quick, but also very dirty.
Created attachment 474044 [details] [diff] [review] v1 - checking for only digits in the port string - checking for positive value
Comment on attachment 474044 [details] [diff] [review] v1 >+++ b/netwerk/base/src/nsURLParsers.cpp >+ const char* nondigit = NS_strspnp("0123456789", buf.BeginReading()); .get() please, since you actually want a null-terminated string. r=me with that
Created attachment 474362 [details] [diff] [review] v1.1 [Check in comment 14] Updated, thanks for review.
Comment on attachment 474362 [details] [diff] [review] v1.1 [Check in comment 14] http://hg.mozilla.org/mozilla-central/rev/8f65173dacb0
Backed out for: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284217060.1284218020.18295.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284216191.1284217040.14302.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215868.1284216788.13151.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215765.1284216810.13218.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215991.1284216655.12771.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215991.1284216924.13798.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215605.1284217545.16230.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215491.1284216422.11484.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215490.1284216321.11209.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215491.1284216157.10391.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215491.1284216444.11697.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284215521.1284217737.16918.gz
So, it looks like fixing bug 595525 will take some time, so I'll disable the failing test for now. The test is nothing critical.
(In reply to comment #12) > So, it looks like fixing bug 595525 will take some time, so I'll disable the > failing test for now. The test is nothing critical. Taking back... it is not my test that is failing!
Comment on attachment 474362 [details] [diff] [review] v1.1 [Check in comment 14] http://hg.mozilla.org/mozilla-central/rev/491c6d05108f