Summary: | [Curl] Allow passing contents of Root CA data directly. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Basuke Suzuki <Basuke.Suzuki> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, annulen, Basuke.Suzuki, commit-queue, don.olmstead, ews-watchlist, galpeter, pvollan, rniwa, webkit-bug-importer, youennf, ysuzuki | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Basuke Suzuki
2018-05-18 13:16:34 PDT
Created attachment 340739 [details]
PATCH
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. 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.
Created attachment 342841 [details]
FIX
Created attachment 342842 [details]
FIX
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. Is it okay to start using variant?? I thought C++ features other than optional were still on hold state. Created attachment 342950 [details]
PATCH
Switch the set of member variables into Variant.
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.
Created attachment 342954 [details]
FIX
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 on attachment 342954 [details]
FIX
r=me
Comment on attachment 342954 [details] FIX Clearing flags on attachment: 342954 Committed r233002: <https://trac.webkit.org/changeset/233002> All reviewed patches have been landed. Closing bug. |