Fix NPE in InternalAbstractHttpAsyncClient by adding null check for resolvedTarget #634
Conversation
| } | ||
| if (request.getAuthority() == null) { | ||
| request.setAuthority(new URIAuthority(resolvedTarget)); | ||
| if (resolvedTarget != null) { |
There was a problem hiding this comment.
HI @arturobernalg 👋
We could throw a more precise exception using Objects.requireNonNull(...), which would reduce the nesting. See also org.apache.hc.core5.util.Args.notNull(T, String)
arturobernalg
force-pushed
the
HTTPCLIENT-2367
branch
4 times, most recently
from
db53767 to
622c043
Compare
| setupContext(clientContext); | ||
|
|
||
| final HttpHost resolvedTarget = target != null ? target : RoutingSupport.determineHost(request); | ||
| Args.notNull(resolvedTarget, "Target host for the request"); |
There was a problem hiding this comment.
@arturobernalg This is not right. The target may be null if the request URI cannot be parsed. It should be up to the specific protocol handler to decide if such a request can be executed. H2 will likely reject it but HTTP/1.1 may accept it
…g null check for resolvedTarget.
arturobernalg
force-pushed
the
HTTPCLIENT-2367
branch
from
928d79d to
3515abf
Compare
|
@ok2c should I wait for the original reporter’s feedback before merging? |
@arturobernalg I do not think it is necessary. The fix is quite straight-forward. |
…g null check for resolvedTarget. (#634)
…g null check for resolvedTarget. (#634)
|
@arturobernalg I tweaked the test case a little to make it behave it more similarly to the default route planner. |
|
👍 |
thank you @ok2c |
3 participants
Resolves a
NullPointerExceptioninInternalAbstractHttpAsyncClientby adding a null check for resolvedTarget when RoutingSupport.determineHost returns null. ThrowsIllegalStateExceptionfor invalid requests, ensuring robust error handling. Addresses HTTPCLIENT-2367.