HTMLEditor::InsertObject incorrectly handles reading input streams from blobs
Categories
(Core :: DOM: Editor, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox108 | --- | fixed |
People
(Reporter: nika, Assigned: evilpie)
References
(Depends on 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
The code in HTMLEditor::InsertObject
will attempt to read the input stream from the associated type-erased object if it is being used to insert an image into the document. Unfortunately, this behaviour is performed with some incorrect assumptions.
As noted in the documentation for NS_ConsumeStream
, this method will only consume the data from a stream up until it returns an error, including the NS_BASE_STREAM_WOULD_BLOCK
error which is returned from non-blocking async streams. This means that the method is only an appropriate way to consume an entire stream synchronously if the passed-in stream is fully available and synchronous.
However, the stream which is passed in is frequently not fully-available and synchronous when the blob path is taken. A blob received from the parent process, such as a file blob, will generally use a RemoteLazyInputStream
as the returned stream type. This stream is always async, and will stream in the response data asynchronously when it is requested, meaning that the call to NS_ConsumeStream
will always return NS_BASE_STREAM_WILL_BLOCK
and return nothing when used with a remote blob in this way.
This may also be the case with the streams which are passed in directly, however it is harder to follow the origins of those input streams, so there may be some other invariant somehow ensuring that they're always synchronous.
Given that this code is running on the main thread, and may need to do some I/O to read in the stream, it probably should be refactored to be able to read the data asynchronously. This would also allow removing the call to SlurpFileToString
which when called would do blocking file I/O on the main thread to read in a file from disk.
It seems likely that it would be possible to handle this situation asynchronously given that the code earlier operating on BlobImpl
will consume the blob's data asynchronously through SlurpBlob
.
Comment 1•2 years ago
|
||
Thank you for explaining the detail! It sounds like the pasting file/image is completely broken, however, we don't get such bug reports as far as I know. Perhaps I misunderstand somethimg or the path is a dead path actually, e.g., JS libraries for creating editor or web apps themselves handle it instead for better and consistent behavior between browsers.
I need to keep investigating more for considering the severity.
Comment 2•2 years ago
|
||
The severity field is not set for this bug.
:kmag, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•2 years ago
|
Comment hidden (obsolete) |
Updated•2 years ago
|
Assignee | ||
Comment 4•2 years ago
|
||
Updated•2 years ago
|
Assignee | ||
Comment 5•2 years ago
|
||
Depends on D155753
Updated•2 years ago
|
Updated•2 years ago
|
Comment 7•2 years ago
|
||
Backed out for causing mochitest plain failures in editor/libeditor/tests/test_bug490879.html
Backout link: https://hg.mozilla.org/integration/autoland/rev/472635603967535a4fabdb805f16955e638c7525
INFO - TEST-INFO | screenshot: exit 0
[task 2022-10-18T19:03:12.663Z] 19:03:12 INFO - TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug490879.html | uncaught exception - TypeError: can't access property "src", doc.getElementsByTagName(...)[0] is undefined at verifyContent@http://mochi.test:8888/tests/editor/libeditor/tests/test_bug490879.html:15:12
[task 2022-10-18T19:03:12.664Z] 19:03:12 INFO - runTest@http://mochi.test:8888/tests/editor/libeditor/tests/test_bug490879.html:38:3
Comment 9•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d6c4fdcf6a69
https://hg.mozilla.org/mozilla-central/rev/47dfb29abf33
Comment 10•2 years ago
|
||
Unfortunately this busted Thunderbird as we are using SlurpFileToString
here: https://searchfox.org/comm-central/rev/809c0b1305c77e3d9b4b90eefed6c1f27c47439c/mailnews/compose/src/nsMsgCompose.cpp#3743
Could I kindly ask for some help in porting this changes?
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Description
•