Bug 261104

Summary:	[Curl] Support multipart response
Product:	WebKit	Reporter:	Kenji Shukuwa <kenji.shukuwa>
Component:	Platform	Assignee:	Fujii Hironori <Hironori.Fujii>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	Hironori.Fujii, webkit-bug-importer
Priority:	P2	Keywords:	InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified

Description Kenji Shukuwa 2023-09-04 02:00:04 PDT

WinCairo WK1 supported multipart response. However, WinCairo WK1 has been removed.
Also, WinCairo WK2 does not support multipart responses. Because of this, current
WinCairo (curl port) does not support multipart responses.

We still have the multipart response code from WinCairo WK1, so let's enable it in WinCairo WK2.

Comment 1 Kenji Shukuwa 2023-09-04 02:15:10 PDT

Pull request: https://github.com/WebKit/WebKit/pull/17403

Comment 2 Radar WebKit Bug Importer 2023-09-11 02:01:14 PDT

<rdar://problem/115277024>

Comment 3 EWS 2023-09-19 23:00:19 PDT

Committed 268180@main (a3b8bd468c68): <https://commits.webkit.org/268180@main>

Reviewed commits have been landed. Closing PR #17403 and removing active labels.

Comment 4 Fujii Hironori 2023-09-20 13:09:53 PDT

Reverted by https://github.com/WebKit/WebKit/pull/17987

Comment 5 Fujii Hironori 2023-09-20 13:09:55 PDT

Re-opening for pull request https://github.com/WebKit/WebKit/pull/17987

Comment 6 Fujii Hironori 2023-09-20 13:12:43 PDT

WinCairo was crasing.

https://build.webkit.org/#/builders/727/builds/20953
https://build.webkit.org/#/builders/728/builds/2181

Comment 7 EWS 2023-09-20 13:15:18 PDT

Committed 268213@main (c12cd61edd95): <https://commits.webkit.org/268213@main>

Reviewed commits have been landed. Closing PR #17987 and removing active labels.

Comment 8 Kenji Shukuwa 2023-09-27 23:25:08 PDT

> WinCairo was crasing.
>
> https://build.webkit.org/#/builders/727/builds/20953
> https://build.webkit.org/#/builders/728/builds/2181

Crash trace:

---
 	WebCore.dll!WTF::StringImpl::ref() line 1142	C++
 	WebCore.dll!WTF::DefaultRefDerefTraits<WTF::StringImpl>::refIfNotNull(WTF::StringImpl * ptr) line 37	C++
 	WebCore.dll!WTF::RefPtr<WTF::StringImpl,WTF::RawPtrTraits<WTF::StringImpl>,WTF::DefaultRefDerefTraits<WTF::StringImpl>>::RefPtr<WTF::StringImpl,WTF::RawPtrTraits<WTF::StringImpl>,WTF::DefaultRefDerefTraits<WTF::StringImpl>>(const WTF::RefPtr<WTF::StringImpl,WTF::RawPtrTraits<WTF::StringImpl>,WTF::DefaultRefDerefTraits<WTF::StringImpl>> & o) line 64	C++
 	WebCore.dll!WTF::String::String(const WTF::String & __that) line 84	C++
 	WebCore.dll!WebCore::ParsedContentType::mimeType() line 51	C++
>	WebCore.dll!WebCore::extractBoundary(const WebCore::CurlResponse & response) line 54	C++
 	WebCore.dll!WebCore::CurlMultipartHandle::createIfNeeded(WebCore::CurlMultipartHandleClient & client, const WebCore::CurlResponse & response) line 70	C++
 	WebCore.dll!WebCore::CurlRequest::didReceiveHeader(WTF::String && header) line 339	C++
 	WebCore.dll!WebCore::CurlRequest::didReceiveHeaderCallback(char * ptr, unsigned __int64 blockSize, unsigned __int64 numberOfBlocks, void * userData) line 637	C++
 	[external code]	
 	WebCore.dll!WebCore::CurlMultiHandle::perform(int & runningHandles) line 284	C++
 	WebCore.dll!WebCore::CurlRequestScheduler::workerThread() line 176	C++
 	WebCore.dll!`WebCore::CurlRequestScheduler::startOrWakeUpThread'::`2'::<lambda_1>::operator()() line 99	C++
 	WebCore.dll!WTF::Detail::CallableWrapper<`WebCore::CurlRequestScheduler::startOrWakeUpThread'::`2'::<lambda_1>,void>::call() line 53	C++
 	WTF.dll!WTF::Function<void __cdecl(void)>::operator()() line 83	C++
 	WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext) line 259	C++
 	WTF.dll!WTF::wtfThreadEntryPoint(void * data) line 151	C++
 	[external code]	
---

Comment 9 Kenji Shukuwa 2023-09-27 23:32:12 PDT

ParsedContentType::create() returns std::nullopt if parsing the Content-Type fails. 

https://github.com/WebKit/WebKit/blob/ac6be1003b5936badc0a7333bfa630df2a8f498c/Source/WebCore/platform/network/ParsedContentType.cpp#L330-L336
---
std::optional<ParsedContentType> ParsedContentType::create(const String& contentType, Mode mode)
{
    ParsedContentType parsedContentType(mode == Mode::Rfc2045 ? contentType : contentType.trim(isASCIIWhitespaceWithoutFF<UChar>));
    if (!parsedContentType.parseContentType(mode))
        return std::nullopt; // ### here ###
    return { WTFMove(parsedContentType) };
}
---

However, the extractBoundary() function in CurlMultipartHandle.cpp did not include guard handling for that case.

---
static std::optional<CString> extractBoundary(const CurlResponse& response)
{
    static const auto contentTypeLength = strlen("content-type:");

    for (auto header : response.headers) {
        if (!header.startsWithIgnoringASCIICase("content-type:"_s))
            continue;

        auto contentType = ParsedContentType::create(header.substring(contentTypeLength));
        // ### Need guard process ###

        if (!equalLettersIgnoringASCIICase(contentType->mimeType(), "multipart/x-mixed-replace"_s))
            return std::nullopt;

---

Comment 10 Kenji Shukuwa 2023-09-27 23:37:14 PDT

Since PR#17403 has been reverted, I will create the PR again.

Comment 11 Kenji Shukuwa 2023-09-27 23:57:25 PDT

Pull request: https://github.com/WebKit/WebKit/pull/18332

Comment 12 EWS 2023-09-28 00:36:25 PDT

Committed 268569@main (5379982beccd): <https://commits.webkit.org/268569@main>

Reviewed commits have been landed. Closing PR #18332 and removing active labels.