Open Bug 1431241 Opened 6 years ago Updated 2 years ago

URLSearchParams can be stored in IndexedDB in violation of spec

Categories

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

enhancement

Tracking

()

People

(Reporter: asuth, Unassigned)

References

(Blocks 1 open bug)

Details

IndexedDB ends up storing anything that StructuredCloneHolder::ReadFullySerializableObjects[1] handles.  There are some static asserts in CommonStructuredCloneReadCallback[2] that make sure the things IDB explicitly supports don't change, but there are no checks to ensure that new DOM things don't accidentally start getting persisted in IDB without intent.

For URLSearchParams, it seems bug 1207233 intentionally made URLSearchParams clonable for reasons that weren't immediately obvious from a quick pre-dinner scan of the bug.

I made a glitch at https://idb-structured-clone-check.glitch.me/ that shows Firefox trunk stores URLSearchParams because of this and Google Chrome does not.

Going forward, I think we ideally want bug 1350254, but having IDB's CommonStructuredCloneReadCallback explode if it sees a tag that's greater than  SCTAG_BASE but not in a whitelist might be a good stopgap.  We may need to whitelist URLSearchParams initially, depending on what our stance is on existing DB's out there.

1: https://searchfox.org/mozilla-central/rev/1f9b4f0f0653b129b4db1b9fc5e9068054afaac0/dom/base/StructuredCloneHolder.cpp#352
2: https://searchfox.org/mozilla-central/rev/1f9b4f0f0653b129b4db1b9fc5e9068054afaac0/dom/indexedDB/IDBObjectStore.cpp#1174
The reason for making them clonable was an artifact of how the worker XHR implementation currently works (it sends objects to the main thread from the worker by cloning them). This can be seen here: https://dxr.mozilla.org/mozilla-central/source/dom/xhr/XMLHttpRequestWorker.cpp#2132

I'm not sure if there's an easy way around this.
Priority: -- → P3
:Baku, thoughts on this?
Flags: needinfo?(amarchesini)
By spec, we should not support CryptoKey nor URLSearchParams. We should probably file a couple of bugs in these specs asking for adding [Serializable] to these 2 interfaces. NI :annevk
Flags: needinfo?(amarchesini) → needinfo?(annevk)
CryptoKey is to be supported, but the specification is not maintained (https://www.w3.org/TR/2017/REC-WebCryptoAPI-20170126/#cryptokey-interface-clone). As for URLSearchParams, it would be best for someone to file an issue at https://github.com/whatwg/url/issues/new if we want to pursue that. However, should we then also make Headers and FormData serializable as they are fairly similar?
Flags: needinfo?(annevk)
> However, should we then also make Headers and FormData serializable as they
> are fairly similar?

FormData is already supported. We don't have the support for Headers, but that can be added easily.

I'm filing a spec issue for having URLSearchParams and URL interfaces serializable.
URL/URLSearchParams: https://github.com/whatwg/url/issues/370
FormData (also not standardized, but there is support from Chrome to do so): https://github.com/whatwg/xhr/issues/55
Blocks: IDBv3
See Also: → 1636761
Depends on: 1636761
See Also: 1636761
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.