Bug 239024 – Fix size computation in WebCore::StorageMap

• Status: RESOLVED
• Resolution: FIXED
• Component: Website Storage

[Sihui Liu]

...

[Sihui Liu]

rdar://88249235

[Sihui Liu]

Created attachment 457147 [details]
Patch

[Chris Dumez]

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]

Created attachment 457270 [details]
Patch for landing

[Sihui Liu]

(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]

Created attachment 457273 [details]
Patch for landing

[EWS]

Patch 457270 does not build

[EWS]

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