New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 618381 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Writing to websqldatabases accessed with file:// URLs causes a crash

Reported by dbast...@gdssecurity.com, Jun 8 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0

Steps to reproduce the problem:
1. Create a local HTML file with the following contents:
<script type="text/javascript">
   var db = openDatabase('test', '1.0', 'test', 262144)
   db.transaction(function (tx) {
      tx.executeSql('create table if not exists test (test)');
   }, function(t, e) {console.log(e)}, function(t, r) { alert('success'); });
</script>

2. Access this page using a file:// URL (i.e. file:///Users/user/test.html) 

3. The tab will crash with an 'aw snap' error

What is the expected behavior?
The page should display a javascript "success" alert dialog.  The test case works when accessing the page with an http:// URL.

What went wrong?
The following error message is printed prior to the crash when debug logging is enabled:

ERROR:bad_message.cc(18)] Terminating renderer for bad IPC message, reason 24

reason 24 is defined as DBMF_DB_NOT_OPEN_ON_MODIFY in  https://chromium.googlesource.com/chromium/src/+/master/content/browser/bad_message.h

The commit causing the problem is https://chromium.googlesource.com/chromium/src/+/1d0a8a44b6c8b671a45cf8d4fe7da52a74d42930 - builds prior to this commit work, those after do not.  The code review link https://codereview.chromium.org/1779413002 mentions a "Hacky file:/// fix" which appears to be the source of the problem.

Crashed report ID: no

How much crashed? Just one tab

Is it a problem with a plugin? No 

Did this work before? Yes Prior to commit 1d0a8a44

Chrome version: 51.0.2704.79 (Official Build) (64-bit)  Channel: stable
OS Version: OS X 10.11
Flash Version: Shockwave Flash 21.0 r0
 
Components: Blink>Storage>WebSQL
Owner: jsb...@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: -OS-Mac OS-All
Status: Started (was: Assigned)
Grrr, this probably needs a fix similar to https://codereview.chromium.org/1914763002 - s/toString/toRawString/ in some cases.

 
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1667e9b86e2ede1e6267db73445478b447cca2e8

commit 1667e9b86e2ede1e6267db73445478b447cca2e8
Author: jsbell <jsbell@chromium.org>
Date: Tue Jun 14 17:54:45 2016

DatabaseTracker: Use 'raw' string as key for origin

Prior to r382660 the WebSQL implementation used DatabaseIdentifiers, and
a serialization was used as the key in maps. This was changed to take
SecurityOrigins instead and the serialization via toString(). This
introduced additional special cases e.g. "null" for file:// origins in
some cases, leading to mismatches between setting and lookup in the
map depending on when the SecurityOrigin was minted. To fix, use
toRawString() within the DatabaseTracker which matches the old behavior.

Similarly, use toRawString() when serializing an origin for posting
across threads.

BUG= 618381 
R=michaeln@chromium.org

Review-Url: https://codereview.chromium.org/2050803005
Cr-Commit-Position: refs/heads/master@{#399743}

[modify] https://crrev.com/1667e9b86e2ede1e6267db73445478b447cca2e8/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
[modify] https://crrev.com/1667e9b86e2ede1e6267db73445478b447cca2e8/third_party/WebKit/Source/modules/webdatabase/SQLTransactionClient.cpp

Labels: Merge-Request-52
Requesting to merge to 52, it's a small change with resolves a crashing bug when using websql from file:// urls.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1667e9b86e2ede1e6267db73445478b447cca2e8

commit 1667e9b86e2ede1e6267db73445478b447cca2e8
Author: jsbell <jsbell@chromium.org>
Date: Tue Jun 14 17:54:45 2016

DatabaseTracker: Use 'raw' string as key for origin

Prior to r382660 the WebSQL implementation used DatabaseIdentifiers, and
a serialization was used as the key in maps. This was changed to take
SecurityOrigins instead and the serialization via toString(). This
introduced additional special cases e.g. "null" for file:// origins in
some cases, leading to mismatches between setting and lookup in the
map depending on when the SecurityOrigin was minted. To fix, use
toRawString() within the DatabaseTracker which matches the old behavior.

Similarly, use toRawString() when serializing an origin for posting
across threads.

BUG= 618381 
R=michaeln@chromium.org

Review-Url: https://codereview.chromium.org/2050803005
Cr-Commit-Position: refs/heads/master@{#399743}

[modify] https://crrev.com/1667e9b86e2ede1e6267db73445478b447cca2e8/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
[modify] https://crrev.com/1667e9b86e2ede1e6267db73445478b447cca2e8/third_party/WebKit/Source/modules/webdatabase/SQLTransactionClient.cpp

Cc: nyerramilli@chromium.org
Labels: TE-Verified-M53 TE-Verified-53.0.2768.0
Tested the issue on Win7, Mac OS X 10.11.5, Ubuntu 14.04 using Chrome Canary #53.0.2768.0 using attached file - did not observe any crash and page displayed "success" alert dialog.

adding TE-Verified labels.
attaching sample html file & screenshot for reference.
618381_win7.jpg
87.0 KB View Download
618381.html
311 bytes View Download

Comment 7 by tin...@google.com, Jun 15 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Please merge your CL in to M52 branch by Friday [6/17] so that it can be picked up for beta promotion scheduled in next week.
Cc: michaeln@chromium.org
Looping in michaeln@ as jsbell@ is OOO to get the CL merged in to M52 branch.
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 17 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09a5bbb16c83ffad3ec63a6e272367428c190a2a

commit 09a5bbb16c83ffad3ec63a6e272367428c190a2a
Author: Michael Nordman <michaeln@google.com>
Date: Fri Jun 17 22:07:33 2016

DatabaseTracker: Use 'raw' string as key for origin

Prior to r382660 the WebSQL implementation used DatabaseIdentifiers, and
a serialization was used as the key in maps. This was changed to take
SecurityOrigins instead and the serialization via toString(). This
introduced additional special cases e.g. "null" for file:// origins in
some cases, leading to mismatches between setting and lookup in the
map depending on when the SecurityOrigin was minted. To fix, use
toRawString() within the DatabaseTracker which matches the old behavior.

Similarly, use toRawString() when serializing an origin for posting
across threads.

BUG= 618381 
R=michaeln@chromium.org

Review-Url: https://codereview.chromium.org/2050803005
Cr-Commit-Position: refs/heads/master@{#399743}
(cherry picked from commit 1667e9b86e2ede1e6267db73445478b447cca2e8)

Review URL: https://codereview.chromium.org/2080463002 .

Cr-Commit-Position: refs/branch-heads/2743@{#383}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/09a5bbb16c83ffad3ec63a6e272367428c190a2a/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
[modify] https://crrev.com/09a5bbb16c83ffad3ec63a6e272367428c190a2a/third_party/WebKit/Source/modules/webdatabase/SQLTransactionClient.cpp

Labels: TE-Verified-M52 TE-Verified-52.0.2743.49
Tested the issue on Win7, Mac OS X 10.11.5, Ubuntu 14.04 using Chrome Beta #52.0.2743.49 using test file in c#6 - did not observe any crash and page displayed "success" alert dialog.

adding TE-Verified labels and attached screenshot for reference.
618381_Beta.jpg
127 KB View Download
Cc: cmumford@chromium.org
 Issue 618975  has been merged into this issue.
 Issue 617596  has been merged into this issue.
Status: Fixed (was: Started)

Sign in to add a comment