HTTP Proxy Support #68
Conversation
|
This looks great! Thank you for taking the time to send this pull request! Could you document the usage of this new feature in the README and also add doxygen headers to the newly added method? |
| if (uriProxyUpper.compare(0, 4, "HTTP") != 0) | ||
| this->uriProxy = "http://" + uriProxy; | ||
| else | ||
| this->uriProxy = uriProxy; |
There was a problem hiding this comment.
this line seems to not be covered by a unit test (see here). Do you know of an https proxy you could use to test this behaviour?
There was a problem hiding this comment.
I can add a test similar to "TestProxy" and provide a proxy address that begins with "http" or "https". I think that the proxy I already used in that test is a one using SSL but I might be wrong.
There was a problem hiding this comment.
The most important thing is to mention the port of the proxy at the end of the address, the prefix of the address whether it's http:// or https:// doesn't matter. An HTTPS proxy is considered a HTTP proxy. More info here : https://curl.haxx.se/libcurl/c/CURLOPT_PROXY.html , here https://curl.haxx.se/libcurl/c/CURLOPT_HTTPPROXYTUNNEL.html and here https://curl.haxx.se/libcurl/c/CURLOPT_PROXYTYPE.html
| std::transform(uriProxyUpper.begin(), uriProxyUpper.end(), | ||
| uriProxyUpper.begin(), ::toupper); | ||
|
|
||
| if (uriProxyUpper.compare(0, 4, "HTTP") != 0) |
There was a problem hiding this comment.
please add curly braces to match the overall style of the code
There was a problem hiding this comment.
OK, I ll do it !
| @@ -209,6 +211,21 @@ RestClient::Connection::SetKeyPath(const std::string& keyPath) { | |||
| this->keyPath = keyPath; | |||
| } | |||
|
|
|||
| void | |||
| RestClient::Connection::SetProxy(const std::string& uriProxy) { | |||
| if (uriProxy.empty()) | |||
There was a problem hiding this comment.
please add curly braces to match the overall style of the code
…coverage and fixed code style.
|
merged! Thanks again for taking the time to contribute! |
Fixed code indentation to comply with Cpplint. Added web proxy tunneling support. this adds support for using proxies corresponding to the CURLOPT_PROXY and CURLOPT_HTTPPROXYTUNNEL config options in libcurl. closes mrtazz#68 default to 10s timeout in unit tests this makes the conn object in unit tests default to a 10s timeout in order to fail fast if something is up with the connection or configured hosts.
I added and tested a method to set a web HTTP proxy address and to tunnel an operation through it.
If the provided proxy address is invalid, the operation will fail and it's confirmed with the unit test "TestInvalidProxy".
The test I wrote (TestProxy) is based on a hard-coded proxy address from https://www.sslproxies.org/
Personally, in my Google Test unit tests, I parse a configuration file (via simpleini API) to fetch test environment parameters (proxy/URL addresses, local paths...), this technique can be used to update the proxy address used in the unit test if it's not valid anymore and it will offer versatility for the other parameters used in the other unit tests.