Bug 239024 - Fix size computation in WebCore::StorageMap
Summary: Fix size computation in WebCore::StorageMap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Website Storage (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-08 22:01 PDT by Sihui Liu
Modified: 2022-04-11 14:00 PDT (History)
4 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
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].