vert.x icon indicating copy to clipboard operation
vert.x copied to clipboard

ConnectionManager for HttpClient disregards proxyOptions

Open zekronium opened this issue 2 years ago • 5 comments

Context

Since the merge of my PR I realized that wrong connections are picked when changing proxy per request as per this PR

During my investigation my assumption is that due to EndpointKey not checking proxyOptions in equals methods causes the wrong connection to be matched which leads to the wrong proxy being used

Steps to reproduce

  1. Create WebClient (without proxy)
  2. Create a request and set a proxy to it and send
  3. Create another request and set a different proxy and send
  4. WebClient will use the first proxy (or any proxy that it already has a connection with that matches other EndPoint requirements)

Proposed Fix in EndpointKey.java

  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;
    EndpointKey that = (EndpointKey) o;
    return ssl == that.ssl && serverAddr.equals(that.serverAddr) && peerAddr.equals(that.peerAddr) && proxyOptions.equals(that.proxyOptions);
  }

  @Override
  public int hashCode() {
    int result = ssl ? 1 : 0;
    result = 31 * result + peerAddr.hashCode();
    result = 31 * result + serverAddr.hashCode();
    result = 31 * result + proxyOptions.hashCode();
    return result;
  }

zekronium avatar Feb 16 '22 19:02 zekronium

this is actually expected, one should use always the same proxy with the same host

vietj avatar Feb 17 '22 11:02 vietj

well actually this is debatable, but it was thought that ppl would use the same proxy for the same host

vietj avatar Feb 17 '22 11:02 vietj

i.e I'm not against changing that

vietj avatar Feb 17 '22 11:02 vietj

I think in quite a few cases for different uses "rotating" your proxy might come in handy even on the same host.

The way it is now its impossible to do so since the input is disregarded

zekronium avatar Feb 17 '22 14:02 zekronium

Hash code check might not suffice for this to work properly.

Due to copied objects of the proxyOptions, even while applying the same proxy it can produce a different hashcode itself.

equals() on proxyOptions likely should be implemented to compare actual host values to improve how this works.

zekronium avatar Aug 06 '22 16:08 zekronium

This has been fixed by another PR, I assume safe to close?

zekronium avatar Oct 21 '22 00:10 zekronium