vbhanuchander-lang opened a new pull request, #8046:
URL: https://github.com/apache/hop/pull/8046

   Addresses #7789 — with one correction to the scope, which I've also raised 
on the issue.
   
   ### The proxy leg was already fine
   
   Both transforms pass the proxy to 
`HttpClientManager.HttpClientBuilderFacade#setProxy`, which builds
   it with `new HttpHost(scheme, host, port)` and sets it on `RequestConfig`. 
That constructor takes the
   host name verbatim, so `my_proxy.internal` survives it. Verified against 
httpcore5 5.4.
   
   ### The request target was not
   
   `Http#processRow` and `HttpPost#processRow` call `HttpHost.create(uri)`, and 
that overload reads
   `URI.getHost()`. When a host name contains an underscore, `java.net.URI` 
classifies the authority as
   registry-based rather than server-based, and the parsed components are 
simply not available:
   
   ```
   http://my_service.internal:8080/api
      URI.getHost()      = null
      URI.getPort()      = -1
      URI.getUserInfo()  = null
      URI.getAuthority() = my_service.internal:8080
   ```
   
   `HttpHost.create(URI)` then fails with `NullPointerException: Host name`. 
The same call appears in
   `WebService` and in `HttpProtocol`, so four call sites share the defect.
   
   ### Change
   
   `HttpClientManager.createHttpHost(URI)` uses `getHost()` when it is 
available and otherwise parses
   the authority — dropping userinfo, taking the port when one is present, and 
leaving IPv6 literals
   intact. All four call sites now go through it.
   
   Underscores are not legal in host names per RFC 1123, which the reporter 
rightly notes. But they are
   common in internal and container DNS and they resolve, so failing with a 
`NullPointerException`
   rather than either working or reporting a clear error seems the wrong 
outcome either way.
   
   ### Tests
   
   Seven tests in a new `HttpClientManagerTest`:
   
     - a normal server-based authority
     - an underscored host with a port, and without one
     - userinfo dropped from the origin
     - an IPv6 literal kept intact
     - a URI with no host at all, and a non-numeric port — both rejected with 
`IllegalArgumentException`
       rather than an NPE
   
   Five of the seven fail with `NullPointerException: Host name` if 
`createHttpHost` is changed to
   delegate back to `HttpHost.create(uri)`.
   
   ### Verification
   
     - `mvn -pl 
core,plugins/transforms/http,plugins/transforms/httppost,plugins/transforms/webservices
 -Pskip-uitest test`
       — core 1138, http 37, httppost 25, webservices 6, all passing, 0 failures
     - `mvn spotless:check` and `mvn apache-rat:check` pass on the five touched 
modules
   
   If it turns out the reporter really did hit the proxy path rather than the 
target, then this is still
   a genuine fix for the four call sites above but the issue would need to stay 
open — I've asked them
   to confirm.
   
   ------------------------
   
   - [x] I hereby declare this contribution to be licensed under the [Apache 
License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to