Bug 187874 – [Curl] Fix issue that extra cookie is added when redirect happens.

• Component: Platform
• Status: RESOLVED FIXED

[Basuke Suzuki]

The implementation modified original request to add cookie headers. That caused the issues when redirect happens which was shared while redirecting.

[Basuke Suzuki]

Created attachment 348185 [details]
PATCH

[Alex Christensen]

Comment on attachment 348185 [details]
PATCH

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

> LayoutTests/http/tests/cookies/multiple-redirect-and-set-cookie.php:3
> + * This is test for https://bugs.webkit.org/show_bug.cgi?id=187874 .

Probably not necessary.  version control can be used for anyone who wants to find this out

> Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:136
> -Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest& request, RequestStatus status)
> +Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest request, RequestStatus status)

This could probably be a ResourceRequest&&

[Basuke Suzuki]

Created attachment 348313 [details]
PATCH

[Basuke Suzuki]

Comment on attachment 348313 [details]
PATCH

Alex, right. Simple copying was a little bit uncomfortable for me. Thanks for that advice.

[Alex Christensen]

Comment on attachment 348313 [details]
PATCH

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

> Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:136
> +    return CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes);

This could also probably take a ResourceRequest&&

[Basuke Suzuki]

Comment on attachment 348313 [details]
PATCH

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

thanks for r+.

>> Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:136
>> +    return CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes);
> 
> This could also probably take a ResourceRequest&&

I'll open other bug for this. I'm taking a look into CurlRequest in detail and researching who gonna deref-ing the instance. If I understand correctly, if the object was deleted in main thread, there's no need to isolateCopy for the argument's ResourceRequest even if they referred from worker thread. So by calling deref() from main thread, the need of isolatedCopy() can be removed and we can store moved request directly. Please correct me if something wrong, Alex.

[WebKit Commit Bot]

Comment on attachment 348313 [details]
PATCH

Clearing flags on attachment: 348313

Committed r235437: <https://trac.webkit.org/changeset/235437>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43812677>