Bug 220776

Summary:

Fix nullptr dereference introduced in r268228

Product:

WebKit

Reporter:

Alex Christensen <achristensen>

Component:

New Bugs

Assignee:

Alex Christensen <achristensen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bugmail, cdumez, feross, jhiesey, lwarlow, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	ews-feeder: commit-queue-

Alex Christensen

Reported 2021-01-20 11:34:39 PST

Fix nullptr dereference introduced in r268228

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Alex Christensen

Comment 1 2021-01-20 11:36:50 PST

Created attachment 417983 [details] Patch

Chris Dumez

Comment 2 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 &&

Chris Dumez

Comment 3 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.

Alex Christensen

Comment 4 2021-01-20 11:44:33 PST

Created attachment 417984 [details] Patch

EWS

Comment 5 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].

Radar WebKit Bug Importer

Comment 6 2021-01-20 13:10:15 PST

<rdar://problem/73417187>

Alex Christensen

Comment 7 2021-01-20 17:26:48 PST

This was rdar://problem/71627170

John Hiesey

Comment 8 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.

Chris Dumez

Comment 9 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.

Note You need to log in before you can comment on or make changes to this bug.