Bug 214665 – [IPC hardening] WebKit::ArgumentCoder<BlobPart>::decode() and encode() should use enum BlobPart::Type

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[David Kilzer (:ddkilzer)]

WebCore::ArgumentCoder<BlobPart>::decode() and encode() should use enum BlobPart::Type.

We can also get rid of a default: case label in decode() while we're here to make -Wimplicit-fallthrough emit a warning if a new enum is added.

<rdar://problem/65777948>

[David Kilzer (:ddkilzer)]

Created attachment 404997 [details]
Patch v1

[Darin Adler]

Comment on attachment 404997 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=404997&action=review

> Source/WebCore/platform/network/BlobPart.h:100
> +namespace WTF {
> +
> +template<> struct EnumTraits<WebCore::BlobPart::Type> {
> +    using values = EnumValues<
> +        WebCore::BlobPart::Type,
> +        WebCore::BlobPart::Type::Data,
> +        WebCore::BlobPart::Type::Blob
> +    >;
> +};
> +
> +} // namespace WTF

Instead of this, I suggest:

    enum class Type : bool { Data, Blob };

[David Kilzer (:ddkilzer)]

Comment on attachment 404997 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=404997&action=review

>> Source/WebCore/platform/network/BlobPart.h:100
>> +} // namespace WTF
> 
> Instead of this, I suggest:
> 
>     enum class Type : bool { Data, Blob };

Will fix before landing.  Thanks!

[David Kilzer (:ddkilzer)]

Created attachment 405051 [details]
Patch for landing

[Darin Adler]

Comment on attachment 405051 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=405051&action=review

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2481
>      case BlobPart::Data:

Seems like this won’t compile: Needs to be BlobPart::Type::Data.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2484
>      case BlobPart::Blob:

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2506
>      case BlobPart::Blob: {

Here too, and above.

[David Kilzer (:ddkilzer)]

Created attachment 405053 [details]
Patch for landing v2

[David Kilzer (:ddkilzer)]

Comment on attachment 405053 [details]
Patch for landing v2

Adding cq+ since there are no logic changes from "Patch v1", and the patch built on all platforms.

[EWS]

Committed r264780: <https://trac.webkit.org/changeset/264780>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405053 [details].