[webkit-reviews] review granted: [Bug 44461] Assertion failure at WebCore/platform/network/CredentialStorage.cpp:85. : [Attachment 65189] remove redundant forward slashes. (add new line at end of test file).

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 23 18:30:09 PDT 2010


Alexey Proskuryakov <ap at webkit.org> has granted Yongjun Zhang
<yongjun_zhang at apple.com>'s request for review:
Bug 44461: Assertion failure at
WebCore/platform/network/CredentialStorage.cpp:85.
https://bugs.webkit.org/show_bug.cgi?id=44461

Attachment 65189: remove redundant forward slashes. (add new line at end of
test file).
https://bugs.webkit.org/attachment.cgi?id=65189&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> even though multiple slashes are valid in URL

I'm not really sure on the status of this validity. In file paths, multiple
slashes always mean the same as a single one, but I think that in URLs, it's up
to the server to make a decision. This thought is what has been preventing from
fixing this myself.

I guess it's fine for us to do whatever Firefox is doing, and if it normalizes
slashes before looking up a directory for stored credentials, so could we. But
this patch doesn't quite implement that, as it only removes trailing slashes.

 +	  while (index > 0 && directoryURL[index-1] == '/')

There should be spaces around "-".

> +	       index--;

There is no practical difference for integer values, but for consistency with
iterators, it's best to always use prefix increment and decrement. When the
compiler cannot optimize it, postfix variants create temporary copies.

> +    xhr.open("GET", "resources/remember-bad-password//count-failures.php",
false);

Ideally, the test would check the response to make sure that mapping doesn't
fail (and it should pass in Firefox).

r=me.


More information about the webkit-reviews mailing list