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

Description Sihui Liu 2022-04-08 22:01:19 PDT

...

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

rdar://88249235

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

Created attachment 457147 [details]
Patch

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

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

Created attachment 457270 [details]
Patch for landing

Comment 5 Sihui Liu 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.

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

Created attachment 457273 [details]
Patch for landing

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

Patch 457270 does not build

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