feat(mcp): resolve absolute and relative message endpoint URIs (#150)
Improve endpoint URI handling by supporting both relative paths and properly validated absolute URIs. - Implement URI resolution in HttpClientSseClientTransport: - Change baseUri field from String to URI type - Add Utils.resolveUri method to handle both absolute and relative URIs - Resolve relative URIs against the base URI - Validate absolute URIs to ensure they match base URI's scheme, authority, and path - Add parameterized tests for various URI resolution scenarios - Add ByteBuddy dependency for HttpClient mocking and update Mockito Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
This commit is contained in:
parent
734d1732d6
commit
344f1b8383
7 changed files with 136 additions and 10 deletions
14
mcp/pom.xml
14
mcp/pom.xml
|
|
@ -126,12 +126,26 @@
|
||||||
<version>${junit.version}</version>
|
<version>${junit.version}</version>
|
||||||
<scope>test</scope>
|
<scope>test</scope>
|
||||||
</dependency>
|
</dependency>
|
||||||
|
<dependency>
|
||||||
|
<groupId>org.junit.jupiter</groupId>
|
||||||
|
<artifactId>junit-jupiter-params</artifactId>
|
||||||
|
<version>${junit.version}</version>
|
||||||
|
<scope>test</scope>
|
||||||
|
</dependency>
|
||||||
<dependency>
|
<dependency>
|
||||||
<groupId>org.mockito</groupId>
|
<groupId>org.mockito</groupId>
|
||||||
<artifactId>mockito-core</artifactId>
|
<artifactId>mockito-core</artifactId>
|
||||||
<version>${mockito.version}</version>
|
<version>${mockito.version}</version>
|
||||||
<scope>test</scope>
|
<scope>test</scope>
|
||||||
</dependency>
|
</dependency>
|
||||||
|
|
||||||
|
<!-- Mockito cannot mock this class: class java.net.http.HttpClient. the bytebuddy helps. -->
|
||||||
|
<dependency>
|
||||||
|
<groupId>net.bytebuddy</groupId>
|
||||||
|
<artifactId>byte-buddy</artifactId>
|
||||||
|
<version>${byte-buddy.version}</version>
|
||||||
|
<scope>test</scope>
|
||||||
|
</dependency>
|
||||||
<dependency>
|
<dependency>
|
||||||
<groupId>io.projectreactor</groupId>
|
<groupId>io.projectreactor</groupId>
|
||||||
<artifactId>reactor-test</artifactId>
|
<artifactId>reactor-test</artifactId>
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ import io.modelcontextprotocol.spec.McpError;
|
||||||
import io.modelcontextprotocol.spec.McpSchema;
|
import io.modelcontextprotocol.spec.McpSchema;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage;
|
import io.modelcontextprotocol.spec.McpSchema.JSONRPCMessage;
|
||||||
import io.modelcontextprotocol.util.Assert;
|
import io.modelcontextprotocol.util.Assert;
|
||||||
|
import io.modelcontextprotocol.util.Utils;
|
||||||
import org.slf4j.Logger;
|
import org.slf4j.Logger;
|
||||||
import org.slf4j.LoggerFactory;
|
import org.slf4j.LoggerFactory;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
@ -69,7 +70,7 @@ public class HttpClientSseClientTransport implements McpClientTransport {
|
||||||
private static final String DEFAULT_SSE_ENDPOINT = "/sse";
|
private static final String DEFAULT_SSE_ENDPOINT = "/sse";
|
||||||
|
|
||||||
/** Base URI for the MCP server */
|
/** Base URI for the MCP server */
|
||||||
private final String baseUri;
|
private final URI baseUri;
|
||||||
|
|
||||||
/** SSE endpoint path */
|
/** SSE endpoint path */
|
||||||
private final String sseEndpoint;
|
private final String sseEndpoint;
|
||||||
|
|
@ -178,7 +179,7 @@ public class HttpClientSseClientTransport implements McpClientTransport {
|
||||||
Assert.hasText(sseEndpoint, "sseEndpoint must not be empty");
|
Assert.hasText(sseEndpoint, "sseEndpoint must not be empty");
|
||||||
Assert.notNull(httpClient, "httpClient must not be null");
|
Assert.notNull(httpClient, "httpClient must not be null");
|
||||||
Assert.notNull(requestBuilder, "requestBuilder must not be null");
|
Assert.notNull(requestBuilder, "requestBuilder must not be null");
|
||||||
this.baseUri = baseUri;
|
this.baseUri = URI.create(baseUri);
|
||||||
this.sseEndpoint = sseEndpoint;
|
this.sseEndpoint = sseEndpoint;
|
||||||
this.objectMapper = objectMapper;
|
this.objectMapper = objectMapper;
|
||||||
this.httpClient = httpClient;
|
this.httpClient = httpClient;
|
||||||
|
|
@ -340,7 +341,8 @@ public class HttpClientSseClientTransport implements McpClientTransport {
|
||||||
CompletableFuture<Void> future = new CompletableFuture<>();
|
CompletableFuture<Void> future = new CompletableFuture<>();
|
||||||
connectionFuture.set(future);
|
connectionFuture.set(future);
|
||||||
|
|
||||||
sseClient.subscribe(this.baseUri + this.sseEndpoint, new FlowSseClient.SseEventHandler() {
|
URI clientUri = Utils.resolveUri(this.baseUri, this.sseEndpoint);
|
||||||
|
sseClient.subscribe(clientUri.toString(), new FlowSseClient.SseEventHandler() {
|
||||||
@Override
|
@Override
|
||||||
public void onEvent(SseEvent event) {
|
public void onEvent(SseEvent event) {
|
||||||
if (isClosing) {
|
if (isClosing) {
|
||||||
|
|
@ -412,7 +414,8 @@ public class HttpClientSseClientTransport implements McpClientTransport {
|
||||||
|
|
||||||
try {
|
try {
|
||||||
String jsonText = this.objectMapper.writeValueAsString(message);
|
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))
|
.POST(HttpRequest.BodyPublishers.ofString(jsonText))
|
||||||
.build();
|
.build();
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4,11 +4,12 @@
|
||||||
|
|
||||||
package io.modelcontextprotocol.util;
|
package io.modelcontextprotocol.util;
|
||||||
|
|
||||||
|
import reactor.util.annotation.Nullable;
|
||||||
|
|
||||||
|
import java.net.URI;
|
||||||
import java.util.Collection;
|
import java.util.Collection;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
|
||||||
import reactor.util.annotation.Nullable;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Miscellaneous utility methods.
|
* Miscellaneous utility methods.
|
||||||
*
|
*
|
||||||
|
|
@ -52,4 +53,55 @@ public final class Utils {
|
||||||
return (map == null || map.isEmpty());
|
return (map == null || map.isEmpty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolves the given endpoint URL against the base URL.
|
||||||
|
* <ul>
|
||||||
|
* <li>If the endpoint URL is relative, it will be resolved against the base URL.</li>
|
||||||
|
* <li>If the endpoint URL is absolute, it will be validated to ensure it matches the
|
||||||
|
* base URL's scheme, authority, and path prefix.</li>
|
||||||
|
* <li>If validation fails for an absolute URL, an {@link IllegalArgumentException} is
|
||||||
|
* thrown.</li>
|
||||||
|
* </ul>
|
||||||
|
* @param baseUrl The base URL (must be absolute)
|
||||||
|
* @param endpointUrl The endpoint URL (can be relative or absolute)
|
||||||
|
* @return The resolved endpoint URI
|
||||||
|
* @throws IllegalArgumentException If the absolute endpoint URL does not match the
|
||||||
|
* base URL or URI is malformed
|
||||||
|
*/
|
||||||
|
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.");
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
return baseUrl.resolve(endpointUri);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Checks if the given absolute endpoint URI falls under the base URI. It validates
|
||||||
|
* the scheme, authority (host and port), and ensures that the base path is a prefix
|
||||||
|
* of the endpoint path.
|
||||||
|
* @param baseUri The base URI
|
||||||
|
* @param endpointUri The endpoint URI to check
|
||||||
|
* @return true if endpointUri is within baseUri's hierarchy, false otherwise
|
||||||
|
*/
|
||||||
|
private static boolean isUnderBaseUri(URI baseUri, URI endpointUri) {
|
||||||
|
if (!baseUri.getScheme().equals(endpointUri.getScheme())
|
||||||
|
|| !baseUri.getAuthority().equals(endpointUri.getAuthority())) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
URI normalizedBase = baseUri.normalize();
|
||||||
|
URI normalizedEndpoint = endpointUri.normalize();
|
||||||
|
|
||||||
|
String basePath = normalizedBase.getPath();
|
||||||
|
String endpointPath = normalizedEndpoint.getPath();
|
||||||
|
|
||||||
|
if (basePath.endsWith("/")) {
|
||||||
|
basePath = basePath.substring(0, basePath.length() - 1);
|
||||||
|
}
|
||||||
|
return endpointPath.startsWith(basePath);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,12 +7,13 @@ package io.modelcontextprotocol.client.transport;
|
||||||
import java.net.URI;
|
import java.net.URI;
|
||||||
import java.net.http.HttpClient;
|
import java.net.http.HttpClient;
|
||||||
import java.net.http.HttpRequest;
|
import java.net.http.HttpRequest;
|
||||||
|
import java.net.http.HttpResponse;
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
import java.util.concurrent.CompletableFuture;
|
||||||
import java.util.concurrent.atomic.AtomicBoolean;
|
import java.util.concurrent.atomic.AtomicBoolean;
|
||||||
import java.util.concurrent.atomic.AtomicInteger;
|
import java.util.concurrent.atomic.AtomicInteger;
|
||||||
import java.util.concurrent.atomic.AtomicReference;
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
import java.util.function.Consumer;
|
|
||||||
import java.util.function.Function;
|
import java.util.function.Function;
|
||||||
|
|
||||||
import io.modelcontextprotocol.spec.McpSchema;
|
import io.modelcontextprotocol.spec.McpSchema;
|
||||||
|
|
@ -21,6 +22,8 @@ import org.junit.jupiter.api.AfterEach;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.Timeout;
|
import org.junit.jupiter.api.Timeout;
|
||||||
|
import org.mockito.ArgumentCaptor;
|
||||||
|
import org.mockito.Mockito;
|
||||||
import org.testcontainers.containers.GenericContainer;
|
import org.testcontainers.containers.GenericContainer;
|
||||||
import org.testcontainers.containers.wait.strategy.Wait;
|
import org.testcontainers.containers.wait.strategy.Wait;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
@ -31,6 +34,9 @@ import org.springframework.http.codec.ServerSentEvent;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
import static org.assertj.core.api.Assertions.assertThatCode;
|
import static org.assertj.core.api.Assertions.assertThatCode;
|
||||||
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
|
import static org.mockito.Mockito.verify;
|
||||||
|
import static org.mockito.Mockito.when;
|
||||||
|
|
||||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||||
|
|
||||||
|
|
@ -364,4 +370,25 @@ class HttpClientSseClientTransportTests {
|
||||||
customizedTransport.closeGracefully().block();
|
customizedTransport.closeGracefully().block();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@SuppressWarnings("unchecked")
|
||||||
|
void testResolvingClientEndpoint() {
|
||||||
|
HttpClient httpClient = Mockito.mock(HttpClient.class);
|
||||||
|
HttpResponse<Void> httpResponse = Mockito.mock(HttpResponse.class);
|
||||||
|
CompletableFuture<HttpResponse<Void>> future = new CompletableFuture<>();
|
||||||
|
future.complete(httpResponse);
|
||||||
|
when(httpClient.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(future);
|
||||||
|
|
||||||
|
HttpClientSseClientTransport transport = new HttpClientSseClientTransport(httpClient, HttpRequest.newBuilder(),
|
||||||
|
"http://example.com", "http://example.com/sse", new ObjectMapper());
|
||||||
|
|
||||||
|
transport.connect(Function.identity());
|
||||||
|
|
||||||
|
ArgumentCaptor<HttpRequest> httpRequestCaptor = ArgumentCaptor.forClass(HttpRequest.class);
|
||||||
|
verify(httpClient).sendAsync(httpRequestCaptor.capture(), any(HttpResponse.BodyHandler.class));
|
||||||
|
assertThat(httpRequestCaptor.getValue().uri()).isEqualTo(URI.create("http://example.com/sse"));
|
||||||
|
|
||||||
|
transport.closeGracefully().block();
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -6,12 +6,17 @@ package io.modelcontextprotocol.util;
|
||||||
|
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
import java.net.URI;
|
||||||
import java.util.Collection;
|
import java.util.Collection;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
|
import org.junit.jupiter.params.provider.CsvSource;
|
||||||
|
|
||||||
class UtilsTests {
|
class UtilsTests {
|
||||||
|
|
||||||
|
|
@ -37,4 +42,28 @@ class UtilsTests {
|
||||||
assertFalse(Utils.isEmpty(Map.of("key", "value")));
|
assertFalse(Utils.isEmpty(Map.of("key", "value")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ParameterizedTest
|
||||||
|
@CsvSource({
|
||||||
|
// relative endpoints
|
||||||
|
"http://localhost:8080/root, /api/v1, http://localhost:8080/api/v1",
|
||||||
|
"http://localhost:8080/root/, api, http://localhost:8080/root/api",
|
||||||
|
"http://localhost:8080, /api, http://localhost:8080/api",
|
||||||
|
// absolute endpoints matching base
|
||||||
|
"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) {
|
||||||
|
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(URI.create(baseUrl), endpoint))
|
||||||
|
.isInstanceOf(IllegalArgumentException.class)
|
||||||
|
.hasMessageContaining("does not match the base URL");
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
3
pom.xml
3
pom.xml
|
|
@ -60,8 +60,9 @@
|
||||||
|
|
||||||
<assert4j.version>3.26.3</assert4j.version>
|
<assert4j.version>3.26.3</assert4j.version>
|
||||||
<junit.version>5.10.2</junit.version>
|
<junit.version>5.10.2</junit.version>
|
||||||
<mockito.version>5.11.0</mockito.version>
|
<mockito.version>5.17.0</mockito.version>
|
||||||
<testcontainers.version>1.20.4</testcontainers.version>
|
<testcontainers.version>1.20.4</testcontainers.version>
|
||||||
|
<byte-buddy.version>1.17.5</byte-buddy.version>
|
||||||
|
|
||||||
<slf4j-api.version>2.0.16</slf4j-api.version>
|
<slf4j-api.version>2.0.16</slf4j-api.version>
|
||||||
<logback.version>1.5.15</logback.version>
|
<logback.version>1.5.15</logback.version>
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue