417218 - nsStandardURL::SetPath does the wrong thing when the path is '/' and is being set to ''

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Jeff Walden [:Waldo]]

random-three-o-eight:~/moz/builds/clean/dist/bin jwalden$ ./run-mozilla.sh ./xpcshell 
js> const URL = Components.Constructor("@mozilla.org/network/standard-url;1",
                                       "nsIStandardURL", "init");
js> const nsIStandardURL = Components.interfaces.nsIStandardURL;
js> var provided = new URL(nsIStandardURL.URLTYPE_AUTHORITY, 80,
                           "http://example.com", "UTF-8", null);
js> var target = new URL(nsIStandardURL.URLTYPE_AUTHORITY, 80,
"http://example.com/tests/dom/tests/mochitest/whatwg/postMessage_origin_helper.xhtml",
"UTF-8", null);
js> provided.QueryInterface(Components.interfaces.nsIURI)
[xpconnect wrapped (nsISupports, nsIStandardURL, nsIURI) @ 0x61f7a0 (native @ 0x61f504)]
js> target.QueryInterface(Components.interfaces.nsIURI)
[xpconnect wrapped (nsISupports, nsIStandardURL, nsIURI) @ 0x61fa20 (native @ 0x61f974)]
js> provided.spec
http://example.com/
js> target.spec
http://example.com/tests/dom/tests/mochitest/whatwg/postMessage_origin_helper.xhtml
js> provided.userPass = "", provided.path = ""

js> target.userPass = "", target.path = ""

js> target.spec
http://example.com/
js> provided.spec
http://example.com/
js> target.equals(provided)
false


Debugging says |SegmentIs(mBasename, other->mSpec.get(), other->mBasename)| (only mBasename, nothing else in Equals is broken) is false because mBasename differs between the two (where |this == target| and |other == provided|):

(gdb) p mBasename
$36 = {
  mPos = 52, 
  mLen = -1
}
(gdb) p other->mBasename
$37 = {
  mPos = 19, 
  mLen = 0
}


The lengths compare unequal, and SegmentIs returns PR_FALSE.

This breaks at least two of the tests I've written for my current implementation of the optional origin argument to HTML5's postMessage.  Yay for tests; boo for our URL classes.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: One-character (!) patch and a test

It should be fairly obvious what's happening; we're passing an empty string, and the logic that runs for a new, empty path only runs if the current path is more than one character long -- which is an easy enough optimization for the Cut, but doing so avoids the other, necessary side effects in that case.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: With a few more tests of other edge cases

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 303198 [details] [diff] [review]
With a few more tests of other edge cases

Necessary to fix bug 417075...

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 303198 [details] [diff] [review]
With a few more tests of other edge cases

a=beltzner for 1.9

Comment 5

[Jeff Walden [:Waldo]]

Checked in.