Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpUtil: fix invalid URI #4546

Merged
merged 5 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLConnection;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
Expand Down Expand Up @@ -193,9 +192,17 @@ private static ContentResponse executeUrlAndGetReponse(String httpMethod, String
// Get shared http client from factory "on-demand"
final HttpClient httpClient = httpClientFactory.getCommonHttpClient();

URI uri = null;
try {
uri = new URI(url);
} catch (NullPointerException | URISyntaxException e) {
LOGGER.debug("String {} can not be parsed as URI reference", url);
throw new IOException(e);
}

HttpProxy proxy = null;
// Only configure a proxy if a host is provided
if (proxyHost != null && !proxyHost.isBlank() && proxyPort != null && shouldUseProxy(url, nonProxyHosts)) {
if (proxyHost != null && !proxyHost.isBlank() && proxyPort != null && shouldUseProxy(uri, nonProxyHosts)) {
AuthenticationStore authStore = httpClient.getAuthenticationStore();
ProxyConfiguration proxyConfig = httpClient.getProxyConfiguration();
List<Proxy> proxies = proxyConfig.getProxies();
Expand All @@ -209,7 +216,7 @@ private static ContentResponse executeUrlAndGetReponse(String httpMethod, String

final HttpMethod method = HttpUtil.createHttpMethod(httpMethod);

final Request request = httpClient.newRequest(url).method(method).timeout(timeout, TimeUnit.MILLISECONDS);
final Request request = httpClient.newRequest(uri).method(method).timeout(timeout, TimeUnit.MILLISECONDS);

if (httpHeaders != null) {
for (String httpHeaderKey : httpHeaders.stringPropertyNames()) {
Expand All @@ -222,20 +229,15 @@ private static ContentResponse executeUrlAndGetReponse(String httpMethod, String
}

// add basic auth header, if url contains user info
try {
URI uri = new URI(url);
if (uri.getUserInfo() != null) {
String[] userInfo = uri.getUserInfo().split(":");
if (uri.getUserInfo() != null) {
String[] userInfo = uri.getUserInfo().split(":");

String user = userInfo[0];
String password = userInfo[1];
String user = userInfo[0];
String password = userInfo[1];

String basicAuthentication = "Basic "
+ Base64.getEncoder().encodeToString((user + ":" + password).getBytes());
request.header(HttpHeader.AUTHORIZATION, basicAuthentication);
}
} catch (URISyntaxException e) {
LOGGER.debug("String {} can not be parsed as URI reference", url);
String basicAuthentication = "Basic "
+ Base64.getEncoder().encodeToString((user + ":" + password).getBytes());
request.header(HttpHeader.AUTHORIZATION, basicAuthentication);
}

// add content if a valid method is given ...
Expand Down Expand Up @@ -296,22 +298,21 @@ private static ProxyParams prepareProxyParams() {

/**
* Determines whether the list of <code>nonProxyHosts</code> contains the
* host (which is part of the given <code>urlString</code> or not.
* url host (which is part of the given <code>uri</code> or not.
*
* @param urlString
* @param uri
* @param nonProxyHosts
* @return <code>false</code> if the host of the given <code>urlString</code> is contained in
* @return <code>false</code> if the host of the given <code>uri</code> is contained in
* <code>nonProxyHosts</code>-list and <code>true</code> otherwise
*/
private static boolean shouldUseProxy(String urlString, String nonProxyHosts) {
private static boolean shouldUseProxy(URI uri, String nonProxyHosts) {
if (nonProxyHosts != null && !nonProxyHosts.isBlank()) {
String givenHost = urlString;
String givenHost = uri.toString();

try {
URL url = new URL(urlString);
givenHost = url.getHost();
} catch (MalformedURLException e) {
LOGGER.error("the given url {} is malformed", urlString);
givenHost = uri.toURL().getHost();
} catch (IllegalArgumentException | MalformedURLException e) {
LOGGER.error("the given url {} is malformed", uri.toString());
}

String[] hosts = nonProxyHosts.split("\\|");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static org.mockito.Mockito.when;

import java.lang.reflect.Field;
import java.net.URI;
import java.util.concurrent.TimeUnit;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -55,7 +56,7 @@ public void setUp() throws Exception {
httpClientFactory.set(null, clientFactoryMock);

when(clientFactoryMock.getCommonHttpClient()).thenReturn(httpClientMock);
when(httpClientMock.newRequest(URL)).thenReturn(requestMock);
when(httpClientMock.newRequest(URI.create(URL))).thenReturn(requestMock);
when(requestMock.method(any(HttpMethod.class))).thenReturn(requestMock);
when(requestMock.timeout(anyLong(), any(TimeUnit.class))).thenReturn(requestMock);
when(requestMock.send()).thenReturn(contentResponseMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.verify;

import java.net.URI;
import java.nio.Buffer;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -45,7 +46,7 @@ public void baseTest() throws Exception {

assertEquals("Some content", result);

verify(httpClientMock).newRequest(URL);
verify(httpClientMock).newRequest(URI.create(URL));
verify(requestMock).method(HttpMethod.GET);
verify(requestMock).send();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

import java.net.URI;
import java.util.concurrent.TimeUnit;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand All @@ -40,15 +41,15 @@ public void baseTest() throws Exception {

assertEquals("Some content", result);

verify(httpClientMock).newRequest(URL);
verify(httpClientMock).newRequest(URI.create(URL));
verify(requestMock).method(HttpMethod.GET);
verify(requestMock).timeout(500, TimeUnit.MILLISECONDS);
verify(requestMock).send();
}

@Test
public void testAuthentication() throws Exception {
when(httpClientMock.newRequest("http://john:doe@example.org/")).thenReturn(requestMock);
when(httpClientMock.newRequest(URI.create("http://john:doe@example.org/"))).thenReturn(requestMock);
mockResponse(HttpStatus.OK_200);

String result = HttpUtil.executeUrl("GET", "http://john:doe@example.org/", 500);
Expand Down