jp-fake-auth-fix #115
jp-fake-auth-fix #115
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes HTTP client port handling issues in Vert.x 4 when making requests to URLs with non-standard ports. The change addresses a problem where HttpClient.request(method, url) doesn't properly parse port numbers from URLs, causing connection failures to services running on non-standard ports.
Changes:
- Updated
FakeAuthenticatorandOidcKeycloakAuthenticatorto useRequestOptionswithsetAbsoluteURI()instead of passing URL strings directly toHttpClient.request() - Added comprehensive test suite demonstrating the port parsing issue and validating the fix
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| FakeAuthenticator.java | Modified HTTP client request to use RequestOptions with setAbsoluteURI for proper port parsing |
| OidcKeycloakAuthenticator.java | Modified HTTP client request to use RequestOptions with setAbsoluteURI for proper port parsing |
| FakeAuthenticatorHttpClientTest.java | Added new test class with two tests validating HTTP client port handling with non-standard ports |
src/test/java/com/github/susom/vertx/base/test/FakeAuthenticatorHttpClientTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/github/susom/vertx/base/test/FakeAuthenticatorHttpClientTest.java
Outdated
Show resolved
Hide resolved
| } else if (response2.statusCode() == 302 || response2.statusCode() == 200) { | ||
| log.info("Test passed: Token endpoint was successfully reached"); | ||
| async.complete(); | ||
| } else { | ||
| // Some other error, but not a port issue | ||
| log.info("Got response code {} which indicates the port was reached", | ||
| response2.statusCode()); | ||
| async.complete(); | ||
| } |
There was a problem hiding this comment.
The test completes successfully for any response code other than a connection refused error, including error codes that might indicate actual problems (e.g., 400, 404). The test should have more specific assertions about what constitutes a successful outcome rather than accepting any response that reaches the server.
…orHttpClientTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…torHttpClientTest Co-authored-by: jpallas <4623910+jpallas@users.noreply.github.com>
Co-authored-by: jpallas <4623910+jpallas@users.noreply.github.com>
Use dynamic port assignment in FakeAuthenticatorHttpClientTest
Change in handling of URIs by HttpClient.