fix: improvements for absolute and relative URI resolving: modelcontextprotocol#103

Author: ashakirin

mcp/src/main/java/io/modelcontextprotocol/client/transport/HttpClientSseClientTransport.java

@@ -70,7 +70,7 @@ public class HttpClientSseClientTransport implements McpClientTransport {
 	private static final String DEFAULT_SSE_ENDPOINT = "/sse";
 
 	/** Base URI for the MCP server */
-	private final String baseUri;
+	private final URI baseUri;
 
 	/** SSE endpoint path */
 	private final String sseEndpoint;
@@ -179,7 +179,7 @@ public HttpClientSseClientTransport(HttpClient.Builder clientBuilder, HttpReques
 		Assert.hasText(sseEndpoint, "sseEndpoint must not be empty");
 		Assert.notNull(httpClient, "httpClient must not be null");
 		Assert.notNull(requestBuilder, "requestBuilder must not be null");
-		this.baseUri = baseUri;
+		this.baseUri = URI.create(baseUri);
 		this.sseEndpoint = sseEndpoint;
 		this.objectMapper = objectMapper;
 		this.httpClient = httpClient;
@@ -341,8 +341,8 @@ public Mono<Void> connect(Function<Mono<JSONRPCMessage>, Mono<JSONRPCMessage>> h
 		CompletableFuture<Void> future = new CompletableFuture<>();
 		connectionFuture.set(future);
 
-		String clientUri = Utils.resolveUri(this.baseUri, this.sseEndpoint);
-		sseClient.subscribe(clientUri, new FlowSseClient.SseEventHandler() {
+		URI clientUri = Utils.resolveUri(this.baseUri, this.sseEndpoint);
+		sseClient.subscribe(clientUri.toString(), new FlowSseClient.SseEventHandler() {
 			@Override
 			public void onEvent(SseEvent event) {
 				if (isClosing) {
@@ -414,7 +414,8 @@ public Mono<Void> sendMessage(JSONRPCMessage message) {
 
 		try {
 			String jsonText = this.objectMapper.writeValueAsString(message);
-			HttpRequest request = this.requestBuilder.uri(URI.create(this.baseUri + endpoint))
+			URI requestUri = Utils.resolveUri(baseUri, endpoint);
+			HttpRequest request = this.requestBuilder.uri(requestUri)
 				.POST(HttpRequest.BodyPublishers.ofString(jsonText))
 				.build();
 

mcp/src/main/java/io/modelcontextprotocol/util/Utils.java

@@ -4,13 +4,12 @@
 
 package io.modelcontextprotocol.util;
 
+import reactor.util.annotation.Nullable;
+
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.Collection;
 import java.util.Map;
 
-import reactor.util.annotation.Nullable;
-
 /**
  * Miscellaneous utility methods.
  *
@@ -65,29 +64,17 @@ public static boolean isEmpty(@Nullable Map<?, ?> map) {
 	 * </ul>
 	 * @param baseUrl The base URL (must be absolute)
 	 * @param endpointUrl The endpoint URL (can be relative or absolute)
-	 * @return The resolved endpoint URL as a string
+	 * @return The resolved endpoint URI
 	 * @throws IllegalArgumentException If the absolute endpoint URL does not match the
 	 * base URL or URI is malformed
 	 */
-	public static String resolveUri(String baseUrl, String endpointUrl) {
-		try {
-			URI baseUri = new URI(baseUrl);
-			URI endpointUri = new URI(endpointUrl);
-			if (!endpointUri.isAbsolute()) {
-				URI resolvedUri = baseUri.resolve(endpointUri);
-				return resolvedUri.toString();
-			}
-			else {
-				if (isUnderBaseUri(baseUri, endpointUri)) {
-					return endpointUri.toString();
-				}
-				else {
-					throw new IllegalArgumentException("Absolute endpoint URL does not match the base URL.");
-				}
-			}
+	public static URI resolveUri(URI baseUrl, String endpointUrl) {
+		URI endpointUri = URI.create(endpointUrl);
+		if (endpointUri.isAbsolute() && !isUnderBaseUri(baseUrl, endpointUri)) {
+			throw new IllegalArgumentException("Absolute endpoint URL does not match the base URL.");
 		}
-		catch (URISyntaxException e) {
-			throw new IllegalArgumentException("Cannot resolve URI: " + e.getMessage(), e);
+		else {
+			return baseUrl.resolve(endpointUri);
 		}
 	}
 
@@ -114,7 +101,6 @@ private static boolean isUnderBaseUri(URI baseUri, URI endpointUri) {
 		if (basePath.endsWith("/")) {
 			basePath = basePath.substring(0, basePath.length() - 1);
 		}
-
 		return endpointPath.startsWith(basePath);
 	}
 

mcp/src/test/java/io/modelcontextprotocol/util/UtilsTests.java

@@ -6,6 +6,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import java.net.URI;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -51,24 +52,18 @@ void testMapIsEmpty() {
 			"http://localhost:8080/root, http://localhost:8080/root/api/v1, http://localhost:8080/root/api/v1",
 			"http://localhost:8080/root, http://localhost:8080/root, http://localhost:8080/root" })
 	void testValidUriResolution(String baseUrl, String endpoint, String expectedResult) {
-		String result = Utils.resolveUri(baseUrl, endpoint);
-		assertThat(result).isEqualTo(expectedResult);
+		URI result = Utils.resolveUri(URI.create(baseUrl), endpoint);
+		assertThat(result.toString()).isEqualTo(expectedResult);
 	}
 
 	@ParameterizedTest
 	@CsvSource({ "http://localhost:8080/root, http://localhost:8080/other/api",
 			"http://localhost:8080/root, http://otherhost/api",
 			"http://localhost:8080/root, http://localhost:9090/root/api" })
 	void testAbsoluteUriNotMatchingBase(String baseUrl, String endpoint) {
-		assertThatThrownBy(() -> Utils.resolveUri(baseUrl, endpoint)).isInstanceOf(IllegalArgumentException.class)
+		assertThatThrownBy(() -> Utils.resolveUri(URI.create(baseUrl), endpoint))
+			.isInstanceOf(IllegalArgumentException.class)
 			.hasMessageContaining("does not match the base URL");
 	}
 
-	@ParameterizedTest
-	@CsvSource({ "http://localhost:8080/<>root", "http://localhost:8080/ root", "http://localhost:8080/root}" })
-	void testInvalidUri(String baseUrl) {
-		assertThatThrownBy(() -> Utils.resolveUri(baseUrl, "")).isInstanceOf(IllegalArgumentException.class)
-			.hasMessageContaining("Cannot resolve URI");
-	}
-
 }