Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add SSL options #276

Merged
merged 7 commits into from
May 21, 2020
Merged

add SSL options #276

merged 7 commits into from
May 21, 2020

Conversation

flier
Copy link
Contributor

@flier flier commented Jun 14, 2018

resolve #31

include/cpr/ssl_options.h Outdated Show resolved Hide resolved
@COM8
Copy link
Member

COM8 commented Apr 16, 2020

@flier what is the state of this PR?

@zamazan4ik
Copy link

@COM8 @whoshuu @tstack Can anyone take a look on this PR, resolve conflicts and merge? Seems like making HTTPS requests is extremely important and №1 missed feature in the library.

Thank you a lot!

@COM8
Copy link
Member

COM8 commented May 4, 2020

@zamazan4ik #261 (comment)
This looks like a really nice implementation.

@flier
Copy link
Contributor Author

flier commented May 17, 2020

@flier what is the state of this PR?

Just merge the conflicts, but we need review the changes since two years passed

test/error_tests.cpp Show resolved Hide resolved
test/server.cpp Outdated Show resolved Hide resolved
include/cpr/ssl_options.h Show resolved Hide resolved
@COM8
Copy link
Member

COM8 commented May 17, 2020

The CI is failing because of an redefinition of cpr::Verbose’

/home/travis/build/whoshuu/cpr/include/cpr/verbose.h:9:7: error: redefinition of ‘class cpr::Verbose’

https://travis-ci.org/github/whoshuu/cpr/jobs/687963128#L701

@COM8
Copy link
Member

COM8 commented May 17, 2020

Once this has been fixed we still need documentation.
Could you please create a PR for the github-pages brache, which adds docu for this great PR.

@flier Great work!

@flier
Copy link
Contributor Author

flier commented May 17, 2020

The CI is failing because of an redefinition of cpr::Verbose’

/home/travis/build/whoshuu/cpr/include/cpr/verbose.h:9:7: error: redefinition of ‘class cpr::Verbose’

https://travis-ci.org/github/whoshuu/cpr/jobs/687963128#L701

Removed the old code, thanks

@amityadav4a
Copy link

@flier @COM8 @tstack Can anyone close this please.

@COM8
Copy link
Member

COM8 commented May 19, 2020

@amityadav4a Test cases are still broken and documentation for this feature is required.
https://travis-ci.org/github/whoshuu/cpr/jobs/688010350#L1843-L1845

@flier This is probably related to a CMake error/export (add_executable) not working correctly.

@flier
Copy link
Contributor Author

flier commented May 20, 2020

@amityadav4a Test cases are still broken and documentation for this feature is required.
https://travis-ci.org/github/whoshuu/cpr/jobs/688010350#L1843-L1845

@flier This is probably related to a CMake error/export (add_executable) not working correctly.

I have fixed some issues, but there are still some failures

It seems the CI envrionment have some problems?

Adding APT Sources
121.13s$ sudo -E apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
gpg: keyring /tmp/tmpb7m7g8pt/secring.gpg' created gpg: keyring /tmp/tmpb7m7g8pt/pubring.gpg' created
gpg: requesting key BA9EF27F from hkp server keyserver.ubuntu.com
Error: retrieving gpg key timed out.
gpg: keyserver timed out
gpg: keyserver receive failed: keyserver error
The command "sudo -E apt-add-repository -y "ppa:ubuntu-toolchain-r/test"" failed and exited with 1 during .

85.35s$ sudo -E apt-add-repository -y "ppa:ubuntu-toolchain-r/test"
gpg: keyring /tmp/tmpnhh3s_w_/secring.gpg' created gpg: keyring /tmp/tmpnhh3s_w_/pubring.gpg' created
gpg: requesting key BA9EF27F from hkp server keyserver.ubuntu.com
Error: retrieving gpg key timed out.
gpgkeys: key 60C317803A41BA51845E371A1E9377A2BA9EF27F can't be retrieved
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0
gpg: keyserver communications error: keyserver helper general error
gpg: keyserver communications error: unknown pubkey algorithm
gpg: keyserver receive failed: unknown pubkey algorithm
The command "sudo -E apt-add-repository -y "ppa:ubuntu-toolchain-r/test"" failed and exited with 1 during .

It appears that Secure Transport under MacOS does not support specifying client certificate files directly

(iOS and Mac OS X only) This option is ignored if curl was built against Secure Transport. Secure Transport expects the private key to be already present in the keychain or PKCS#12 file containing the certificate.

test 16
Start 16: cpr_ssl_tests
16: Test command: /Users/travis/build/whoshuu/cpr/build/bin/ssl_tests "/Users/travis/build/whoshuu/cpr/test/data/"
16: Test timeout computed to be: 9.99988e+06
16: Note: Randomizing tests' orders with a seed of 19083 .
16: [==========] Running 1 test from 1 test case.
16: [----------] Global test environment set-up.
16: [----------] 1 test from SslTests
16: [ RUN ] SslTests.HelloWorldTest
16: * Trying 127.0.0.1...
16: * Connected to 127.0.0.1 (127.0.0.1) port 8080 (#0)
16: * WARNING: SSL: CURLOPT_SSLKEY is ignored by Secure Transport. The private key must be in the Keychain.
16: * WARNING: SSL: The Security framework only supports loading identities that are in PKCS#12 format.
16: * Closing connection 0
16: unknown file: Failure
16: Unknown C++ exception thrown in the test body.
16: [ FAILED ] SslTests.HelloWorldTest (1040 ms)
16: [----------] 1 test from SslTests (1040 ms total)
16:
16: [----------] Global test environment tear-down
16: [==========] 1 test from 1 test case ran. (1046 ms total)
16: [ PASSED ] 0 tests.
16: [ FAILED ] 1 test, listed below:
16: [ FAILED ] SslTests.HelloWorldTest
16:
16: 1 FAILED TEST
16/16 Test #16: cpr_ssl_tests ....................***Failed 1.06 sec

It seems our build-in libcurl doesn't support https on windows?

16: Test command: C:\projects\cpr\build\bin\Debug\ssl_tests.exe "C:/projects/cpr/test/data/"
16: Test timeout computed to be: 10000000
16: [==========] Running 1 test from 1 test case.
16: [----------] Global test environment set-up.
16: [----------] 1 test from SslTests
16: [ RUN ] SslTests.HelloWorldTest
16: * Protocol "https" not supported or disabled in libcurl
16: * Closing connection -1
16: C:\projects\cpr\test\ssl_tests.cpp(26): error: Value of: response.text
16: Actual: ""
16: Expected: expected_text
16: Which is: "Hello world!"
16: C:\projects\cpr\test\ssl_tests.cpp(28): error: Value of: response.header["content-type"]
16: Actual: ""
16: Expected: std::string{"text/html"}
16: Which is: "text/html"
16: C:\projects\cpr\test\ssl_tests.cpp(29): error: Value of: response.status_code
16: Actual: 0
16: Expected: 200
16: C:\projects\cpr\test\ssl_tests.cpp(30): error: Value of: response.error.code
16: Actual: 4-byte object <0F-00 00-00>
16: Expected: ErrorCode::OK
16: Which is: 4-byte object <00-00 00-00>
16: Protocol "https" not supported or disabled in libcurl
16: [ FAILED ] SslTests.HelloWorldTest (1012 ms)
16: [----------] 1 test from SslTests (1012 ms total)
16:
16: [----------] Global test environment tear-down
16: [==========] 1 test from 1 test case ran. (2016 ms total)
16: [ PASSED ] 0 tests.
16: [ FAILED ] 1 test, listed below:
16: [ FAILED ] SslTests.HelloWorldTest

@COM8
Copy link
Member

COM8 commented May 20, 2020

@flier Yes, should be fine now. CI and CMake are kind of outdated and hard to maintain right now.
What about the documentation for this feature?
If you are planing to write some I'm willing to merge this PR now, if you are fine with that.

@flier
Copy link
Contributor Author

flier commented May 21, 2020

@flier Yes, should be fine now. CI and CMake are kind of outdated and hard to maintain right now.

Agree, I think we need another PR to update those tests later

What about the documentation for this feature?
If you are planing to write some I'm willing to merge this PR now, if you are fine with that.

I have no problem with this, I will submit another PR to supplement the missing documentation.

@COM8
Copy link
Member

COM8 commented May 21, 2020

OK. Then one last rebase please :)

@COM8 COM8 merged commit b036a32 into libcpr:master May 21, 2020
@COM8
Copy link
Member

COM8 commented May 21, 2020

OK. Then one last rebase please :)

Nevermind

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Openssl support
5 participants