eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.25k stars 2.07k forks source link

Cant modify the proxy server for same target url (https) before the httpclient is rebuild use request-level proxy setting. #4478

Closed jr981008 closed 2 years ago

jr981008 commented 2 years ago

Questions

vert x 4.x has request-level proxy, but when replacing proxy ip hots for same target url, it seems that https only uses target server hosts .ip for connection pool select, http use target server hosts .ip and proxy sever host .ip .Why is there such diffrence for http . https proxy? It seems that it becomes impossible to modify the proxy server for same target url (https) before the httpclient is rebuild.

for example: request https://www.example.com with proxy 127.0.0.1:8081 get ok shutdown 8081 proxy . startup proxysever at port 8082. request https://www.example.com with proxy 127.0.0.1:8082 get fail, beacuse httpclient still use 8081 proxy server config。

set proxy use request-level

test Code:

ProxyOptions proxy = new ProxyOptions().setPort(8081).setHost("127.0.0.1").setType(ProxyType.HTTP);
req(proxy);
// shutdown port 8081proxy server,open 8082 proxy port.
ProxyOptions proxy2 = new ProxyOptions().setPort(8082).setHost("127.0.0.1").setType(ProxyType.HTTP);
req(proxy2);`

private void req(ProxyOptions proxy) {
        RequestOptions options = new RequestOptions().setAbsoluteURI("https://www.example.com/").setMethod(HttpMethod.POST)
                .setProxyOptions(proxy);
        httpClient.request(options,
                r -> {
                    if (r.succeeded()) {
                        HttpClientRequest req = r.result();
                        req.send(v -> {
                            if (v.succeeded()) {
                                HttpClientResponse rsp = v.result();
                                System.out.println(rsp.statusCode());
                            } else {
                                r.cause().printStackTrace();
                            }
                        });
                    } else {
                        r.cause().printStackTrace();
                    }
                }
        );
    }

Version

4.3.3 It seems that this code affects the handling of http and https vertx-core-4.3.3-sources.jar!/io/vertx/core/http/impl/HttpClientImpl.java:574

vietj commented 2 years ago

can you reformat the code in the issue with code fences to make it readable ?

jr981008 commented 2 years ago

oh sorry, code example has been updated.

vietj commented 2 years ago

that's expected, the pool is per host, if proxy would be taken in account then the same host could have more connections than expected.

I think we shouldn't have added a proxy option setting on RequestOptions and instead only kept it at the HttpClient level, because it leads to such unexpectations.

jr981008 commented 2 years ago

Thanks your answer or any help. I understand , we can't allow too many connections between the client and the proxy server, but this brings three problems. 1.The request can be set up as a proxy, but the proxy can only be set once for an address, which is ambiguous.

  1. It is subject to security requirements, many The enterprise-level proxy's password will expire in 10-15 minutes, and the password is designed to only access a fixed 1 url address, like this HMac (url, timestamp, secret), we can't solve this problem with vertx3 before at client level proxy, it seems vertx4 can setting the proxy in the request level, but still not solve the problem after the upgrade.
  2. It is observed that for the http request (not https), code is specially processed to make the proxy setting for each request take effect, and the connection pool is also selected by urlhost+proxyhost.I don't understand why this is? can we expand the options for connection select ?
vietj commented 2 years ago

thanks @jr981008 you convinced me to use the proxy address as being part o the pool key

jr981008 commented 2 years ago

Are there any plans for this? For the time being, we may need to find an alternative, similar to AsyncHttpClient, I will migrate back when vertx supports it.

vietj commented 2 years ago

I think it can be done in the next 4.3.4 release as it does not sound too difficult.