[Webkit-unassigned] [Bug 205902] KeyedDecoderGeneric crashes when it accesses data with non-existing key
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 13 18:41:15 PST 2020
https://bugs.webkit.org/show_bug.cgi?id=205902
--- Comment #8 from Takashi Komori <Takashi.Komori at sony.com> ---
(In reply to Fujii Hironori from comment #3)
> Comment on attachment 387078 [details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387078&action=review
>
> > Source/WebCore/ChangeLog:3
> > + KeyedDecoderGeneric craches when it acesses data with non-existing key.
>
> acesses → accesses
Fixed.
> > Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp:46
> > + bool exists(const String& key) { return !!m_map.get(key); }
>
> It's inefficient to call 'm_map.get(key)' twice.
> Make 'get' method return Node*.
>
> Node* get(const String& key) { return m_map.get(key); }
>
> > Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp:193
> > + return WTF::get_if<T>(dictionary->get(key));
>
> auto node = dictionary->get(key)
> if (!node)
> return nullptr;
> return WTF::get_if<T>(node);
Fixed.
> > Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp:197
> > +bool KeyedDecoderGeneric::getValueFromDictionaryStack(const String& key, T& result)
>
> I think decodeSimpleValue is a better name. KeyedDecoderGlib is using the
> name.
Changed.
> > Tools/TestWebKitAPI/PlatformWin.cmake:65
> > + Tests/WebCore/KeyedCodingGeneric.cpp
>
> I think this tests should be run on all ports.
> Rename KeyedCodingGeneric.cpp KeyedCoding.cpp.
> Add KeyedCoding.cpp for all ports.
Moved to Tools/TestWebKitAPI/CMakeLists.txt but I can't test other ports...
> > Tools/TestWebKitAPI/Tests/WebCore/KeyedCodingGeneric.cpp:42
> > + return false;
>
> Let's use std::memcmp.
Fixed.
> > Tools/TestWebKitAPI/Tests/WebCore/KeyedCodingGeneric.cpp:48
> > +TEST(KeyedCodingGeneric, SetAndGetBytes)
>
> KeyedCodingGeneric → KeyedCoding
Moved.
> > Tools/TestWebKitAPI/Tests/WebCore/KeyedCodingGeneric.cpp:51
> > + const size_t dataLength = sizeof(data) / sizeof(uint8_t);
>
> Use WTF_ARRAY_LENGTH.
Fixed.
> > Tools/TestWebKitAPI/Tests/WebCore/KeyedCodingGeneric.cpp:67
> > + result = decoder->decodeBytes("data", buffer, size);
>
> Rename 'result' to 'ok' or 'success'.
Changed to 'success'.
> > Tools/TestWebKitAPI/Tests/WebCore/KeyedCodingGeneric.cpp:133
> > + EXPECT_EQ(INT32_MAX, value);
>
> I think you should add one more test case checking unavailable to decode
> int32 as other type, for example, bool.
> ok = decoder->decodeBool("zero", value);
> EXPECT_FALSE(ok);
Added tests which try to decode invalid type.
> > Tools/TestWebKitAPI/Tests/WebCore/KeyedCodingGeneric.cpp:263
> > + encoder->encodeDouble("double-max", DBL_MAX);
>
> I think std::numeric_limits<double>::min() and
> std::numeric_limits<double>::max() are better in C++ code.
Fixed.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200114/a1cf3bfd/attachment.htm>
More information about the webkit-unassigned
mailing list