[webkit-reviews] review granted: [Bug 26154] SecurityOrigin::createFromDatabaseIdentifier should handle _'s in the hostname : [Attachment 30891] A potential fix.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jeremy Orlow <jorlow at chromium.org>'s
request for review:
Bug 26154: SecurityOrigin::createFromDatabaseIdentifier should handle _'s in
the hostname
https://bugs.webkit.org/show_bug.cgi?id=26154

Attachment 30891: A potential fix.
https://bugs.webkit.org/attachment.cgi?id=30891&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   WARNING: NO TEST CASES ADDED OR CHANGED

This is something you're supposed to delete if you either add a test case or
decide you can't add one. You shouldn't submit patches with this in them. We
don't want to check this warning into the change log!

> +    // Ensure there were at least 2 seperator characters.  Some hostnames on
intranets have
> +    // underscores in them, so we'll assume that any additional underscores
are part of the host.

Typo in this comment ("seperator"). Also, we use only one space after periods
in our comments.

Seems OK to land this without a test. To make a test we'd have to change
run-webkit-tests to set up the Apache server to run with a hostname that had an
underscore in it. Not impossible, but perhaps difficult.

It would also be good to have tests for the existing behavior, for example,
illegal database identifiers that fall into the failure case.

r=me