|Summary:||Do not allow access to existing HTML5 databases in private browsing mode|
|Product:||WebKit||Reporter:||Anton D'Auria <adauria>|
|Component:||WebCore Misc.||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||adauria, andersca, beidson, commit-queue, darin, jberlin, joepeck, kdecker, webkit.review.bot|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Anton D'Auria 2010-11-10 11:12:53 PST
Comment 1 Anton D'Auria 2010-11-10 11:14:28 PST
Created attachment 73515 [details] [PATCH] Denies read access to databases in private browsing mode
Comment 2 Darin Adler 2010-11-10 17:05:02 PST
Comment on attachment 73515 [details] [PATCH] Denies read access to databases in private browsing mode Patch looks good, but it’s missing a change log entry. review- because of lack of ChangeLog
Comment 3 Anton D'Auria 2010-11-10 17:29:01 PST
Created attachment 73560 [details] [PATCH] Denies read access to databases in private browsing mode (with ChangeLog) Patch now includes ChangeLog.
Comment 4 WebKit Commit Bot 2010-11-10 18:04:19 PST
Comment on attachment 73560 [details] [PATCH] Denies read access to databases in private browsing mode (with ChangeLog) Rejecting patch 73560 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2 Last 500 characters of output: ... fast/tokenizer ........................................ fast/transforms ....................... fast/url .................... fast/workers ............................................. fast/workers/storage .................... fast/workers/storage/read-and-write-transactions-dont-run-together.html -> failed Exiting early after 1 failures. 16196 tests run. 274.85s total testing time 16195 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 4 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/5610009
Comment 5 Anton D'Auria 2010-12-07 11:18:32 PST
Created attachment 75831 [details] [PATCH] separate read-only transaction state from private browsing no read-write state Previously, read-only transactions and private browsing mode were represented by the same SQLStatement and DatabaseAuthorizer states. This patch removes the m_readOnly member variable from SQLStatement and DatabaseAuthorizer, and replaces it with m_permissions whose bit fields are initialized by a DatabaseAuthorizer enum Permissions (ReadWrite, ReadOnly, NoAccess). A read-only transaction sets permissions to ReadOnly, and if !m_database->scriptExecutionContext()->allowDatabaseAccess(), then permissions also set to NoAccess.
Comment 6 WebKit Review Bot 2010-12-07 21:44:19 PST
Attachment 75831 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2 Updating OpenSource Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061 Died at WebKitTools/Scripts/update-webkit line 132. If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Anton D'Auria 2010-12-09 16:17:09 PST
Created attachment 76132 [details] [PATCH] separate read-only transaction state from private browsing no read-write state (renamed mask variables)
Comment 8 Anton D'Auria 2010-12-09 16:23:25 PST
Created attachment 76135 [details] [PATCH] separate read-only transaction state from private browsing no read-write state (fixed ChangeLog conflict)
Comment 9 Eric Seidel (no email) 2010-12-14 01:24:28 PST
Comment on attachment 73560 [details] [PATCH] Denies read access to databases in private browsing mode (with ChangeLog) Cleared Darin Adler's review+ from obsolete attachment 73560 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 10 Darin Adler 2010-12-14 18:04:26 PST
Comment on attachment 76135 [details] [PATCH] separate read-only transaction state from private browsing no read-write state (fixed ChangeLog conflict) View in context: https://bugs.webkit.org/attachment.cgi?id=76135&action=review r=me, but the enum/bit combo is a bit ugly, the bitifield use here is sloppy and there is some incorrectly formatted code > WebCore/storage/DatabaseAuthorizer.h:97 > + void setPermissions(int permissions); It doesn’t seem right to use int for permissions, but use an enum for the values. I think we normally have a better idiom for this sort of thing. > WebCore/storage/DatabaseAuthorizer.h:114 > + int m_permissions; > bool m_securityEnabled : 1; All these bools are set to be a single bit to save memory, I suppose, and then the three bit permissions is taking up an entire 32-bit int! I think the use of bitfields on the bools is excessive. You could use a bitfield for permissions, but because we are using a signed type, int, we’d need to use four bits. It would be better to use unsigned and three bits. Or not use bitfields at all in this class. > WebCore/storage/SQLTransactionSync.cpp:94 > + if (!m_database->scriptExecutionContext()->allowDatabaseAccess()) > + permissions |= DatabaseAuthorizer::NoAccessMask; > + else if (m_readOnly) > + permissions |= DatabaseAuthorizer::ReadOnlyMask; This is not indented correctly.
Comment 11 WebKit Commit Bot 2010-12-14 18:59:45 PST
The commit-queue encountered the following flaky tests while processing attachment 76135 [details]: inspector/styles-new-API.html bug 50889 (authors: email@example.com and firstname.lastname@example.org) The commit-queue is continuing to process your patch.
Comment 12 WebKit Commit Bot 2010-12-14 19:02:14 PST
Comment on attachment 76135 [details] [PATCH] separate read-only transaction state from private browsing no read-write state (fixed ChangeLog conflict) Clearing flags on attachment: 76135 Committed r74093: <http://trac.webkit.org/changeset/74093>
Comment 13 WebKit Commit Bot 2010-12-14 19:02:21 PST
All reviewed patches have been landed. Closing bug.