Fix authorizer issue with fts3 in WebDatabase.

Fix authorizer issue with fts3 in WebDatabase.

fts3 itself was authorized, but the most recent fts3 implementation
calls PRAGMA page_size, and PRAGMA was not authorized.

BUG=85522
TEST=See bug.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90163

[Scott Hess - ex-Googler, 2011-06-22 20:16:12]

Any one reviewer should be fine, let me know if there's someone more
appropriate.

Also, I would like to write a test that fts3 works, but find myself unable to
describe where the test should live.  Since DatabaseAuthorizer::createVTable()
references fts3, does that imply that it should be a layout test, rather than
something Chromium-specific?

[michaeln, 2011-06-22 22:12:51]

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/README.chromium
File third_party/sqlite/README.chromium (right):

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/README.chrom...
third_party/sqlite/README.chromium:80: patch -p0 < ../sqlite/fts3_85522.patch
where is this .patch file?

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/src/ext/fts3...
File third_party/sqlite/src/ext/fts3/fts3.c (right):

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/src/ext/fts3...
third_party/sqlite/src/ext/fts3/fts3.c:634: p->nPgsz = 1024;
1024... what if this hardcoded value does not match the actual page size in use?

Wow, this entire file has changed dramatically since the previous version, how
did the previous version figure out the page size?

Is there a more direct means of looking up the page size given an sqlite3 ptr?
Also an alternative may be to temporarily unhook the authorizer function while
performing this query and restoring it prior to return.

sqlite3* db;
db->xAuth = NULL;
db->pAuthArg = NULL;

[Scott Hess - ex-Googler, 2011-06-22 23:25:34]

I also see that I didn't add the patch file.  Adding.

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/src/ext/fts3...
File third_party/sqlite/src/ext/fts3/fts3.c (right):

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/src/ext/fts3...
third_party/sqlite/src/ext/fts3/fts3.c:634: p->nPgsz = 1024;
On 2011/06/22 22:12:51, michaeln wrote:
> 1024... what if this hardcoded value does not match the actual page size in
use?

AFAICT, the goal of pulling the page size is to improve the estimate for the
cost of pulling in certain data.  Having the wrong page size may cause a less
efficient code path, but it shouldn't be wildly less efficient, and correctness
should be preserved.

1024 is the default page size for SQLite.  I'll work on upstreaming the patch,
at which point I'm sure better suggestions will be made.  I also wanted to keep
the patch reasonable to merge to M-13.

> Wow, this entire file has changed dramatically since the previous version, how
> did the previous version figure out the page size?

This query optimization wasn't previously present.

> Is there a more direct means of looking up the page size given an sqlite3 ptr?
> Also an alternative may be to temporarily unhook the authorizer function while
> performing this query and restoring it prior to return.
> 
> sqlite3* db;
> db->xAuth = NULL;
> db->pAuthArg = NULL;

I considered that, but it seemed a little dangerous to me - what if you got it
wrong and left the authorizer wide open, or allowed some sort of information
leakage?

http://codereview.chromium.org/7230021/diff/1/third_party/sqlite/src/ext/fts3...
third_party/sqlite/src/ext/fts3/fts3.c:635: rc= SQLITE_OK;
Oops, I'll get that whitespace.

[michaeln, 2011-06-23 00:08:12]

LGTM

> does that imply that it should be a layout test

A layout test (or a layout test like "browser test") would probably be the best
option.