Bug 239024

Summary:

Fix size computation in WebCore::StorageMap

Product:

WebKit

Reporter:

Sihui Liu <sihui_liu>

Component:

Website Storage

Assignee:

Sihui Liu <sihui_liu>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, cdumez, sihui_liu, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

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

Sihui Liu

Reported 2022-04-08 22:01:19 PDT

...

Attachments
Patch (10.20 KB, patch) 2022-04-08 22:39 PDT, Sihui Liu	no flags	Details Formatted Diff Diff
Patch for landing (10.37 KB, patch) 2022-04-11 10:23 PDT, Sihui Liu	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch for landing (10.37 KB, patch) 2022-04-11 11:11 PDT, Sihui Liu	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Sihui Liu

Comment 1 2022-04-08 22:01:39 PDT

rdar://88249235

Sihui Liu

Comment 2 2022-04-08 22:39:23 PDT

Created attachment 457147 [details] Patch

Chris Dumez

Comment 3 2022-04-10 16:45:38 PDT

Comment on attachment 457147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457147&action=review r=me > Source/WebCore/storage/StorageMap.cpp:167 > + for (auto& pair : m_impl->map) Why not use `for (auto& [key, value] : m_impl->map) {` like you did below? > Source/WebCore/storage/StorageMap.cpp:168 > + newSize += (pair.key.sizeInBytes() + pair.value.sizeInBytes()); Might need to be written like so for overflow checks: newSize += (pair.key.sizeInBytes(); newSize += pair.value.sizeInBytes()); Otherwise, there might be an issue if `pair.key.sizeInBytes() + pair.value.sizeInBytes()` overflows, no? > Source/WebCore/storage/StorageMap.cpp:174 > + newSize += key.sizeInBytes() + value.sizeInBytes(); Same comment as above about overflows.

Sihui Liu

Comment 4 2022-04-11 10:23:07 PDT

Created attachment 457270 [details] Patch for landing

Sihui Liu

Comment 5 2022-04-11 10:23:56 PDT

(In reply to Chris Dumez from comment #3) > Comment on attachment 457147 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=457147&action=review > > r=me > > > Source/WebCore/storage/StorageMap.cpp:167 > > + for (auto& pair : m_impl->map) > > Why not use `for (auto& [key, value] : m_impl->map) {` like you did below? Updated. > > > Source/WebCore/storage/StorageMap.cpp:168 > > + newSize += (pair.key.sizeInBytes() + pair.value.sizeInBytes()); > > Might need to be written like so for overflow checks: > newSize += (pair.key.sizeInBytes(); > newSize += pair.value.sizeInBytes()); > > Otherwise, there might be an issue if `pair.key.sizeInBytes() + > pair.value.sizeInBytes()` overflows, no? Yes, updated. > > > Source/WebCore/storage/StorageMap.cpp:174 > > + newSize += key.sizeInBytes() + value.sizeInBytes(); > > Same comment as above about overflows. Updated.

Sihui Liu

Comment 6 2022-04-11 11:11:17 PDT

Created attachment 457273 [details] Patch for landing

EWS

Comment 7 2022-04-11 11:18:09 PDT

Patch 457270 does not build

EWS

Comment 8 2022-04-11 14:00:24 PDT

Committed r292721 (249507@main): <https://commits.webkit.org/249507@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 457273 [details].

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