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.

Sign up for GitHub

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

Jump to bottom

feat: add proxy support #258

Merged

qicosmos merged 4 commits into qicosmos:master from helintongh:add_proxy_support

Mar 3, 2023

Collaborator

helintongh commented Feb 28, 2023

Lack of means to test connections to proxy servers.
must have proxy server can test it.

the following code execution results are as shown above:

connect to 192.168.102.1 and get baidu.com response.

It's can't use unittest test.

TEST_CASE("test coro http proxy request") {
  coro_http_client client{};
  std::string uri = "http://www.baidu.com";
  // Make sure the host and port are matching with your proxy server
  client.set_proxy("192.168.102.1", 7890);
  resp_data result = async_simple::coro::syncAwait(client.async_get(uri));
  CHECK(!result.net_err);
  CHECK(result.status == status_type::ok);
}

Digest Authentication not implement because need openssl can implement it.

Design ideas:
There is nothing to say about proxy basic authentication and proxy bearer authentication, just add fields in the http header.

How to connect to proxy server?
Actually the program connects to the vpn client in the localhost, and for the program the local vpn client is a proxy server.

So we don't care about how to connect to the proxy server and how to create a tunnel between client and server.

Just get the socket connected to your vpn client to send http requests through the proxy.


          feat: add send http request to proxy

3805b59

helintongh force-pushed the add_proxy_support branch 2 times, most recently from b4d5a5d to 8746ccf Compare

February 28, 2023 12:09


          feat: add basic auth request and bearer token auth request

665da7d

fix:  change test uri for solve heap-use-after-free problem

fix:  range download test website refuse fix

helintongh force-pushed the add_proxy_support branch from 6793514 to 665da7d Compare

February 28, 2023 16:16


          Merge branch 'master' into add_proxy_support

454757d

helintongh force-pushed the add_proxy_support branch from f62ec74 to 27fea75 Compare

March 1, 2023 03:41

qicosmos reviewed

View reviewed changes

include/cinatra/coro_http_client.hpp Outdated

@@ @@ -539,6 +607,14 @@ class coro_http_client { @@
                 std::vector<std::pair<std::string, std::string>> req_headers_;
+                std::string proxy_host_;
+                int proxy_port_ = -1;

Owner

qicosmos Mar 2, 2023

use std::string as port

include/cinatra/coro_http_client.hpp Outdated

+                        }
+                        else {
+                          // all be http
+                          proxy_url += " http://" + u.get_host() + ":";

Owner

qicosmos Mar 2, 2023

these logic can be put into handle_uri function, and the main logic keep the same with before

include/cinatra/coro_http_client.hpp Outdated

Comment on lines 362 to 369

+                  if (!proxy_url.empty()) {
+                    req_str.append(proxy_url);
+                  }
+                  else {
+                    req_str.append(" ").append(u.get_path());
+                    if (!u.query.empty()) {
+                      req_str.append("?").append(u.query);
+                    }

Owner

qicosmos Mar 2, 2023

this logic should put into handle_url

Collaborator Author

helintongh Mar 2, 2023

fix it

helintongh force-pushed the add_proxy_support branch from 47b481d to d532c45 Compare

March 2, 2023 08:02

qicosmos reviewed

View reviewed changes

include/cinatra/coro_http_client.hpp Outdated

Comment on lines 225 to 237

+                        if (!proxy_host_.empty() && !proxy_port_.empty()) {
+                          if (ec = co_await asio_util::async_connect(
+                                  io_ctx_, socket_, proxy_host_, proxy_port_);
+                              ec) {
+                            break;
+                          }
+                        }
+                        else {
+                          if (ec = co_await asio_util::async_connect(
+                                  io_ctx_, socket_, u.get_host(), u.get_port());
+                              ec) {
+                            break;
+                          }

Owner

qicosmos Mar 3, 2023

i think the if -else can be simplified, because handle_uri can deal with proxy, the return value is proxy host, port, try to improve it again.

Collaborator Author

helintongh Mar 3, 2023 •

edited

Loading

I try to improve it, but prepare_request_str must use raw host.
If I change u.host, will can't create correct host field.
I can modify it to like follow:

  std::string host = proxy_host_.empty() ? u.get_host() : proxy_host_;
  std::string port = proxy_port_.empty() ? u.get_port() : proxy_port_;
  if (ec = co_await asio_util::async_connect(
          io_ctx_, socket_,  host , port );
      ec) {
      break;
  }

Owner

qicosmos Mar 3, 2023

Looks good

qicosmos reviewed

View reviewed changes

include/cinatra/coro_http_client.hpp Outdated

Comment on lines 232 to 233

		// Should the connection be actively closed in proxy? Maybe it's
		// better to let the user judge.

Owner

qicosmos Mar 3, 2023

the user can call client.close(), is it satisfy the acquirement?

Collaborator Author

helintongh Mar 3, 2023

Maybe right,I don't know if I should disconnect from the proxy

Owner

qicosmos Mar 3, 2023

so we can remove the comments.

Collaborator Author

helintongh Mar 3, 2023

ok

Collaborator Author

helintongh Mar 3, 2023

finish it.

helintongh force-pushed the add_proxy_support branch from 5d3eda8 to 5d9193c Compare

March 3, 2023 07:17

qicosmos reviewed

View reviewed changes

include/cinatra/coro_http_client.hpp Outdated

Comment on lines 325 to 350

+                  // construct proxy request uri
+                  if (!proxy_host_.empty() && !proxy_port_.empty()) {
+                    if (!proxy_request_uri_.empty())
+                      proxy_request_uri_.clear();
+                    if (u.get_port() == "http") {
+                      proxy_request_uri_ += "http://" + u.get_host() + ":";
+                      proxy_request_uri_ += "80";
+                    }
+                    else if (u.get_port() == "https") {
+                      proxy_request_uri_ += "https://" + u.get_host() + ":";
+                      proxy_request_uri_ += "443";
+                    }
+                    else {
+                      // all be http
+                      proxy_request_uri_ += " http://" + u.get_host() + ":";
+                      proxy_request_uri_ += u.get_port();
+                    }
+                    proxy_request_uri_ += u.get_path();
+                    u.path = std::string_view(proxy_request_uri_);
+                  }

Owner

qicosmos Mar 3, 2023

could you extract such code into a method?

Collaborator Author

helintongh Mar 3, 2023

finished it.now handle_uri only add one statement.


          fix: proxy code logic optimization

45bdd6a

helintongh force-pushed the add_proxy_support branch from 0836c59 to 45bdd6a Compare

March 3, 2023 08:23

qicosmos approved these changes

View reviewed changes

Owner

qicosmos left a comment

LGTM

qicosmos merged commit ca4b8ba into qicosmos:master

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