Bug 220776 - Fix nullptr dereference introduced in r268228
Summary: Fix nullptr dereference introduced in r268228
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-20 11:34 PST by Alex Christensen
Modified: 2021-04-27 14:38 PDT (History)
6 users (show)

See Also:


Attachments
Patch (5.82 KB, patch)
2021-01-20 11:36 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (6.19 KB, patch)
2021-01-20 11:44 PST, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-01-20 11:34:39 PST
Fix nullptr dereference introduced in r268228
Comment 1 Alex Christensen 2021-01-20 11:36:50 PST
Created attachment 417983 [details]
Patch
Comment 2 Chris Dumez 2021-01-20 11:40:49 PST
Comment on attachment 417983 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417983&action=review

> Source/WebCore/fileapi/Blob.cpp:274
> +            if (bytesLoaded == m_loader->totalBytes()&& !m_bytesRead)

nit: missing space before &&
Comment 3 Chris Dumez 2021-01-20 11:42:11 PST
Comment on attachment 417983 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417983&action=review

>> Source/WebCore/fileapi/Blob.cpp:274
>> +            if (bytesLoaded == m_loader->totalBytes()&& !m_bytesRead)
> 
> nit: missing space before &&

Also may want to use FileReaderLoader::isCompleted() for clarity.
Comment 4 Alex Christensen 2021-01-20 11:44:33 PST
Created attachment 417984 [details]
Patch
Comment 5 EWS 2021-01-20 13:10:00 PST
Committed r271669: <https://trac.webkit.org/changeset/271669>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417984 [details].
Comment 6 Radar WebKit Bug Importer 2021-01-20 13:10:15 PST
<rdar://problem/73417187>
Comment 7 Alex Christensen 2021-01-20 17:26:48 PST
This was rdar://problem/71627170
Comment 8 John Hiesey 2021-04-26 19:08:32 PDT
It is unfortunate that Safari 14.1 shipped with Blob.stream() but without this fix.

Our web app, wormhole.app, uses feature detection to determine if Blob.stream() is available, so it broke when Safari 14.1 was released. We had to quickly add a patch to disable using Blob.stream() in Safari, since it would immediately crash the renderer process.

If the feature was known to be buggy, I wish it hadn't shipped, or that the fix had been applied before shipping it.

We did test in Safari Technology Preview, but it already had this fix applied so we didn't encounter the crash.
Comment 9 Chris Dumez 2021-04-27 11:55:29 PDT
(In reply to John Hiesey from comment #8)
> It is unfortunate that Safari 14.1 shipped with Blob.stream() but without
> this fix.
> 
> Our web app, wormhole.app, uses feature detection to determine if
> Blob.stream() is available, so it broke when Safari 14.1 was released. We
> had to quickly add a patch to disable using Blob.stream() in Safari, since
> it would immediately crash the renderer process.
> 
> If the feature was known to be buggy, I wish it hadn't shipped, or that the
> fix had been applied before shipping it.
> 
> We did test in Safari Technology Preview, but it already had this fix
> applied so we didn't encounter the crash.

Thanks for bringing this to our attention. We'll look into what happened.