[Webkit-unassigned] [Bug 26154] SecurityOrigin::createFromDatabaseIdentifier should handle _'s in the hostname

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 3 10:04:28 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=26154





------- Comment #3 from jorlow at chromium.org  2009-06-03 10:04 PDT -------
(In reply to comment #2)
> (From update of attachment 30891 [review])
> > +        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!

I left that in as a warning to my reviewer.  I assumed it'd be taken out by
whomever checked it in.

> > +    // 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.

Ugh.  That's going to take some getting used to.  :-)

> 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.

It does seem like a pain to test.  Now that I think about it, even if I wrote a
LayoutTest, there's really no good way to verify the output beyond it not
crashing.  It's really too bad there's no way to write unittests in WebKit
since this is basically a perfect example of what they're really useful for.  

Anyway, would you like me to fix the ChangeLog + 2 spaces issue and send
another patch, or did you + it since a committer can easily do that?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list