WebsiteDataStore.cpp uses switch statements for WebKit::ProcessAccessType enum that fall through ASSERT_NOT_REACHED()

• Component: WebKit2
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

WebsiteDataStore.cpp uses switch statements for WebKit::ProcessAccessType enum that fall through ASSERT_NOT_REACHED().

In these cases, it's not clear whether the statement after the switch() statement should be run for every WebKit::ProcessAccessType enum value.

    auto webProcessAccessType = computeWebProcessAccessTypeForDataFetch(dataTypes, !isPersistent());
    if (webProcessAccessType != ProcessAccessType::None) {
        for (auto& process : processes()) {
            switch (webProcessAccessType) {
            case ProcessAccessType::OnlyIfLaunched:
                if (process.state() != WebProcessProxy::State::Running)
                    continue;
                break;

            case ProcessAccessType::Launch:
                // FIXME: Handle this.
                ASSERT_NOT_REACHED();
                break;

            case ProcessAccessType::None:
                ASSERT_NOT_REACHED();
            }

            process.fetchWebsiteData(m_sessionID, dataTypes, [callbackAggregator](WebsiteData websiteData) {
                callbackAggregator->addWebsiteData(WTFMove(websiteData));
            });
        }
    }

See Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp.

There are seven places where this occurs (in four switch statements):

ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:482:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:486:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:664:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:668:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:751:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:783:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:787:  ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.  [security/assertion_fallthrough] [4]

[Radar WebKit Bug Importer]

<rdar://problem/87272534>

[Chris Dumez]

Created attachment 449490 [details]
Patch

[Darin Adler]

Comment on attachment 449490 [details]
Patch

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

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:550
>      UNUSED_PARAM(isNonPersistentStore);

I suggest we omit the argument name instead of using UNUSED_PARAM. Could even put it in comments. The reason I always try to avoid UNUSED_PARAM is that it doesn’t even prevent the code from using the parameter.

But also, why are we even passing this boolean argument to this function? Let’s just omit it.

[Chris Dumez]

Created attachment 449493 [details]
Patch

[EWS]

Committed r288238 (246192@main): <https://commits.webkit.org/246192@main>

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