Bug 185782 – [Curl] Allow passing contents of Root CA data directly.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Basuke Suzuki]

Currently the data must be in a file and set by its path. This patch allow application to set root CA data by passing binary data directly, or adding special prefix ":memory:" via regular setCACertPath() method.

[Basuke Suzuki]

Created attachment 340739 [details]
PATCH

[youenn fablet]

Comment on attachment 340739 [details]
PATCH

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

> Source/WebCore/platform/network/curl/CurlSSLHandle.cpp:85
> +        caCertData.append(caCertString.data(), caCertString.length());

This create a string temporarily.
Maybe there is a way to use StringView so that you can directly create a Vector<char> from it.

> Source/WebCore/platform/network/curl/CurlSSLHandle.cpp:86
> +        setCACertData(WTFMove(caCertData));

Early return.

[Basuke Suzuki]

Created attachment 342825 [details]
PATCH

After Youenn's review, we've decided remove :onmemory: hack into path data. Client now should prepare data by itself. It is the right direction as a library interface.

[Basuke Suzuki]

Created attachment 342841 [details]
FIX

[Basuke Suzuki]

Created attachment 342842 [details]
FIX

[Alex Christensen]

Comment on attachment 342842 [details]
FIX

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

> Source/WebCore/platform/network/curl/CurlSSLHandle.h:110
>      String m_caCertPath;
> +    Vector<char> m_caCertData;

It seems like you have one or the other but never both.  If that's the case, you could use Variant<String, Vector<uint8_t>> m_caCertPathOrData.  This would reduce future "I forgot about the case where the cert root info was in the other form.

[Basuke Suzuki]

Is it okay to start using variant?? I thought C++ features other than optional were still on hold state.

[Basuke Suzuki]

Created attachment 342950 [details]
PATCH

Switch the set of member variables into Variant.

[EWS Watchlist]

Attachment 342950 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/curl/CurlSSLHandle.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Basuke Suzuki]

Created attachment 342954 [details]
FIX

[Basuke Suzuki]

Can anybody review this? The main feature was already r+ by Youenn. After that, I removed unnecessary hack and started using Variant from Alex's suggestion.

[Yusuke Suzuki]

Comment on attachment 342954 [details]
FIX

r=me

[WebKit Commit Bot]

Comment on attachment 342954 [details]
FIX

Clearing flags on attachment: 342954

Committed r233002: <https://trac.webkit.org/changeset/233002>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41274600>