1300205 - Names of DB/ObjectStore/Index specified with "\uDC00\uD800" will be converted to "\uD800\uDC00" unexpectedly in SQLite DB

Status: NEW

(Core :: Storage: IndexedDB, defect, P3)

Description

[Victor Costan]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.89 Safari/537.36

Steps to reproduce:

1) Go to any http/https site
2) Open the dev console
3) Execute the following JS snippet:
indexedDB.open("\uDC00\uD800")


Actual results:

UnknownError was raised


Expected results:

The IndexedDB [1] spec says that all JS strings are acceptable database names, so open should not throw an exception.

[1] https://w3c.github.io/IndexedDB/

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Bevis might have some ideas.

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

[Child 23262] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 623
[Child 23262] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 593
[Child 23262] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 537
[Child 23262] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 494
[Parent 23207] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 623
[Parent 23207] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 593
[Parent 23207] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 537
[Parent 23207] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 494
[Parent 23207] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 623
[Parent 23207] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 593
[Parent 23207] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 537
[Parent 23207] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 494
[Parent 23207] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 623
[Parent 23207] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 593
[Parent 23207] WARNING: got a low Surrogate but no high surrogate: file gecko/src/xpcom/string/nsUTF8Utils.h, line 537
[Parent 23207] WARNING: String ending in half a surrogate pair!: file gecko/src/xpcom/string/nsUTF8Utils.h, line 494
[Parent 23207] WARNING: 'mCommonParams.metadata().name() != databaseName', file gecko/src/dom/indexedDB/ActorsParent.cpp, line 21010
[Parent 23207] WARNING: 'NS_FAILED(rv)', file gecko/src/dom/indexedDB/ActorsParent.cpp, line 20915
[Parent 23207] WARNING: 'NS_FAILED(rv)', file gecko/src/dom/indexedDB/ActorsParent.cpp, line 20672
[Parent 23207] WARNING: Converting non-IndexedDB error code (0x8052000B) to NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR: file gecko/src/dom/indexedDB/ActorsParent.cpp, line 580

Comment 3

[Jan Varga [:janv]]

Doesn't seem to be IndexedDB specific issue. Either string code is buggy or the input string in JS is not valid.

Comment 4

[Victor Costan]

The string is a valid JS string. It is not valid Unicode, but that's a different story.

Also, if it helps Firefox's indexedDB accepts this string as a name for object stores and indexes (which is correct, according to the spec).

A bit more context: I'm implementing object store and index renaming in Chromium and I wrote a test that covers edge cases, such as this one. We're going to upstream the test to WPT, so I wanted it to pass on Firefox. In order to accomplish that, I've had to add a little workaround so that we don't call indexedDB.open with this string.

If it helps, the workaround is here: https://codereview.chromium.org/2276593002/diff/460001/third_party/WebKit/LayoutTests/storage/indexeddb/rename-index.html#newcode384

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

The behavior is inconsistent when "\uDC00\uD800" cannot be a db name but can be a name of an objectstore or an index.

Take this bug to follow up.

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Victor Costan from comment #4)
> going to upstream the test to WPT, so I wanted it to pass on Firefox. In
> order to accomplish that, I've had to add a little workaround so that we
> don't call indexedDB.open with this string.
> 
> If it helps, the workaround is here:
> https://codereview.chromium.org/2276593002/diff/460001/third_party/WebKit/
> LayoutTests/storage/indexeddb/rename-index.html#newcode384

I'd like to take a note here:
To be more precise, the workaround here is to have '\\uDC00\\uD800' in db-name but have '\uDC00\uD800'  as index name converted from JSON.parse('"'+'\\uDC00\\uD800' +'"');

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

Update my investigation:
1. Comment 5 is wrong. the db-name, index-name, and store-name with '\uDC00\uD800' are all converted unexpectedly to '\uD800\uDC00' in SQLite database.
2. Precisely speaking,
   - this improper unicode string(Low Surrogate + High Surrogate) is bound correctly to MozStorageStatement for the creation of database/objectstore/index in [1][2][3] respectively.
   - However, in next IDBOpenRequest, when retrieving this information again from SQLite tables in OpenDatabaseOp::LoadDatabaseInformation()[4][5][6] to setup the metadata, the retrieved values are converted to High Surrogate + Low Surrogate unexpectedly instead.
3. This causes
   - the error reported in WARNING: 'mCommonParams.metadata().name() != databaseName', file gecko/src/dom/indexedDB/ActorsParent.cpp, line 21010 when opening new database.
   - impossibility to retrieve the objectstore and the index with name correct name "\uDC00\uD800" specified, i.e., db.transaction("\uDC00\uD800") and objectStore.index("\uDC00\uD800").

[1] http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/indexedDB/ActorsParent.cpp#4607
[2] http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/indexedDB/ActorsParent.cpp#23526
[3] http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/indexedDB/ActorsParent.cpp#24079
[4] http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/indexedDB/ActorsParent.cpp#20998
[5] http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/indexedDB/ActorsParent.cpp#21062
[6] http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/indexedDB/ActorsParent.cpp#21179

Comment 8

[Tom Tung [:tt, :ttung]]

Maybe, it's the same issue as Bug 1681300

Comment 9

[Kagami Rosylight [:saschanaz] (they/them)]

Hi Jari, per comment #8, could you also take look here?

Comment 10

[Jari Jalkanen]

The question is, should we be supporting only well-formed Unicode or all byte strings as keys of string type for indexedDB?

According to https://www.w3.org/TR/IndexedDB/#name and https://webidl.spec.whatwg.org/#idl-DOMString, we should support "the set of all possible sequences of code units" so this is valid.

Comment 11

[Jari Jalkanen]

Attachment: WIP: Bug 1300205 - Support all DOMStrings as database names.