Bug 185782 - [Curl] Allow passing contents of Root CA data directly.
Summary: [Curl] Allow passing contents of Root CA data directly.
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
Keywords: InRadar
Depends on:
Reported: 2018-05-18 13:16 PDT by Basuke Suzuki
Modified: 2018-06-19 19:59 PDT (History)
12 users (show)

See Also:

PATCH (7.53 KB, patch)
2018-05-18 14:46 PDT, Basuke Suzuki
youennf: review+
Details | Formatted Diff | Diff
PATCH (7.17 KB, patch)
2018-06-15 10:41 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff
FIX (14.71 KB, patch)
2018-06-15 13:51 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff
FIX (7.19 KB, patch)
2018-06-15 13:53 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff
PATCH (8.39 KB, patch)
2018-06-18 10:43 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff
FIX (8.30 KB, patch)
2018-06-18 11:14 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 2018-05-18 13:16:34 PDT
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.
Comment 1 Basuke Suzuki 2018-05-18 14:46:39 PDT
Created attachment 340739 [details]
Comment 2 youenn fablet 2018-05-24 10:25:49 PDT
Comment on attachment 340739 [details]

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.
Comment 3 Basuke Suzuki 2018-06-15 10:41:23 PDT
Created attachment 342825 [details]

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.
Comment 4 Basuke Suzuki 2018-06-15 13:51:36 PDT
Created attachment 342841 [details]
Comment 5 Basuke Suzuki 2018-06-15 13:53:01 PDT
Created attachment 342842 [details]
Comment 6 Alex Christensen 2018-06-15 14:01:36 PDT
Comment on attachment 342842 [details]

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.
Comment 7 Basuke Suzuki 2018-06-15 14:10:47 PDT
Is it okay to start using variant?? I thought C++ features other than optional were still on hold state.
Comment 8 Basuke Suzuki 2018-06-18 10:43:41 PDT
Created attachment 342950 [details]

Switch the set of member variables into Variant.
Comment 9 Build Bot 2018-06-18 10:45:10 PDT
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.
Comment 10 Basuke Suzuki 2018-06-18 11:14:00 PDT
Created attachment 342954 [details]
Comment 11 Basuke Suzuki 2018-06-19 10:22:59 PDT
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.
Comment 12 Yusuke Suzuki 2018-06-19 19:22:48 PDT
Comment on attachment 342954 [details]

Comment 13 WebKit Commit Bot 2018-06-19 19:58:33 PDT
Comment on attachment 342954 [details]

Clearing flags on attachment: 342954

Committed r233002: <https://trac.webkit.org/changeset/233002>
Comment 14 WebKit Commit Bot 2018-06-19 19:58:34 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2018-06-19 19:59:19 PDT