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> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | adauria, andersca, beidson, commit-queue, darin, jberlin, joepeck, kdecker, webkit.review.bot |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | OS X 10.5 | ||
Attachments: |
Description
Anton D'Auria
2010-11-10 11:12:53 PST
Created attachment 73515 [details]
[PATCH] Denies read access to databases in private browsing mode
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
Created attachment 73560 [details]
[PATCH] Denies read access to databases in private browsing mode (with ChangeLog)
Patch now includes ChangeLog.
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 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.
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.
Created attachment 76132 [details]
[PATCH] separate read-only transaction state from private browsing no read-write state (renamed mask variables)
Created attachment 76135 [details]
[PATCH] separate read-only transaction state from private browsing no read-write state (fixed ChangeLog conflict)
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 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. The commit-queue encountered the following flaky tests while processing attachment 76135 [details]: inspector/styles-new-API.html bug 50889 (authors: apavlov@chromium.org and pfeldman@chromium.org) The commit-queue is continuing to process your patch. 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> All reviewed patches have been landed. Closing bug. |