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

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1




------- Comment #4 from darin at apple.com  2009-06-03 12:27 PDT -------
(In reply to comment #3)
> (In reply to comment #2)
> > > +        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.

OK. That's not how it is supposed to work. The only thing a committer should
have to do is add the name of the reviewer and update the date.

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

Join the debate!

http://en.wikipedia.org/wiki/French_spacing
http://www.chicagomanualofstyle.org/CMS_FAQ/OneSpaceorTwo/OneSpaceorTwo02.html
http://www.webword.com/reports/period.html
http://en.wikipedia.org/wiki/Wikipedia_talk:Manual_of_Style_archive_(spaces_after_a_full_stop/period)

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

There's no reason for you to settle for the status quo. Everything about our
testing infrastructure was created by someone at one point to solve a problem
like the one you just mentioned. You could be the person who made this
possible. Exposing some particular function to JavaScript is all it takes to
make a particular unit test in our existing framework; that has been done many
times. There's an experimental feature in the Mac DumpRenderTree that lets
JavaScript call any Objective-C method, too. You could invent a new approach.
Maybe you could bring unit tests as you know them from other projects to this
project.

Please don't assume that someone else is going to fix this!

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

It'd be kind for you to do that. Really depends on the committer how they feel
about doing such minor chores.


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