SecurityOrigin::createFromDatabaseIdentifier contains the following two bugs. Line 286 When testing that the identifier contains at least two separators, the test is reversed. As a result, identifiers with at least two separators are erroneously rejected. This was introduced in http://trac.webkit.org/changeset/44423. Test case ... INPUT: "http_webkit.org_80" ACTUAL: SecurityOrigin(KURL()) (input rejected) EXPECTED: SecurityOrigin(KURL("http://webkit.org:80")) Line 292 When testing that the port is either a valid integer or absent, the logic is incorrect. Instead, the logic tests that the port is a valid integer or present. Hence identifiers where the port is absent are erroneously rejected and those where the port is present but invalid are erroneously accepted. This was introduced in http://trac.webkit.org/changeset/29386. Test cases ... INPUT: "http_webkit.org_" ACTUAL: SecurityOrigin(KURL()) (input rejected) EXPECTED: SecurityOrigin(KURL("http://webkit.org:0")) INPUT: "http_webkit.org_string" ACTUAL: SecurityOrigin(KURL("http://webkit.org:0")) EXPECTED: SecurityOrigin(KURL()) (input rejected)
I'll take a look.
Created attachment 34753 [details] Patch 1 for bug 28262 Is there a way to add low-level tests for this kind of functionality?
(In reply to comment #2) > Created an attachment (id=34753) [details] > Patch 1 for bug 28262 > > Is there a way to add low-level tests for this kind of functionality? Not really. And, in fact, I created a bug suggesting that we do find a way. This class seems trivial to unittests, but ther'es no framework at the moment. (Only LayoutTests.)
Comment on attachment 34753 [details] Patch 1 for bug 28262 The best way to make tests for this is to figure out what actual effect the bugs would have. It's OK to have them not tested if there's no obvious way to do so. Building a lower level unit test machinery would allow testing things like this, but would have other costs that may not be acceptable. Find to just land this as-is. r=me
Assigning to benm for landing.
Comment on attachment 34753 [details] Patch 1 for bug 28262 Clearing flags on attachment: 34753 Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/page/SecurityOrigin.cpp Committed r47221 M WebCore/ChangeLog M WebCore/page/SecurityOrigin.cpp r47221 = ce243898041701dab28e73a78b800546022b62dc (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/47221
All reviewed patches have been landed. Closing bug.