Bug 261104 - [Curl] Support multipart response
Summary: [Curl] Support multipart response
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-09-04 02:00 PDT by Kenji Shukuwa
Modified: 2023-09-28 00:36 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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 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.