diff options
| author | Jon Marius Venstad <jonmv@gmail.com> | 2022-04-06 19:35:30 +0200 |
|---|---|---|
| committer | Jon Marius Venstad <jonmv@gmail.com> | 2022-04-06 19:35:30 +0200 |
| commit | 039589faf5f989d80b9fec2b28ed955ac6fd86f6 (patch) | |
| tree | 45c314cc9ede2d5c26a5d6b4f030ad3db2246a91 | |
| parent | ec92b5f8882e400f94b851dffcf0b3511373e890 (diff) | |
Use HttpURL.Path for Path.getRest()
39 files changed, 256 insertions, 215 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index cb1c1a461e3..76d7ff2fc7f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -31,6 +31,7 @@ import com.yahoo.docproc.jdisc.metric.NullMetric; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.Metric; import com.yahoo.path.Path; +import com.yahoo.restapi.HttpURL; import com.yahoo.slime.Slime; import com.yahoo.transaction.NestedTransaction; import com.yahoo.transaction.Transaction; @@ -558,27 +559,24 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } } - public HttpResponse serviceStatusPage(ApplicationId applicationId, String hostName, String serviceName, String pathSuffix) { + public HttpResponse serviceStatusPage(ApplicationId applicationId, String hostName, String serviceName, HttpURL.Path pathSuffix) { // WARNING: pathSuffix may be given by the external user. Make sure no security issues arise... // We should be OK here, because at most, pathSuffix may change the parent path, but cannot otherwise // change the hostname and port. Exposing other paths on the cluster controller should be fine. // TODO: It would be nice to have a simple check to verify pathSuffix doesn't contain /../ components. - String pathPrefix; + HttpURL.Path pathPrefix = HttpURL.Path.empty(); switch (serviceName) { - case "container-clustercontroller": { - pathPrefix = "clustercontroller-status/v1/"; + case "container-clustercontroller": + pathPrefix = pathPrefix.append("clustercontroller-status").append("v1"); break; - } case "distributor": - case "storagenode": { - pathPrefix = ""; + case "storagenode": break; - } default: throw new NotFoundException("No status page for service: " + serviceName); } - return httpProxy.get(getApplication(applicationId), hostName, serviceName, pathPrefix + pathSuffix); + return httpProxy.get(getApplication(applicationId), hostName, serviceName, pathPrefix.append(pathSuffix)); } public Map<String, ClusterReindexing> getClusterReindexingStatus(ApplicationId applicationId) { @@ -659,9 +657,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye return getOptionalApplication(applicationId).map(app -> app.getModel().fileReferences()).orElse(Set.of()); } - public ApplicationFile getApplicationFileFromSession(TenantName tenantName, long sessionId, String path, Session.Mode mode) { + public ApplicationFile getApplicationFileFromSession(TenantName tenantName, long sessionId, HttpURL.Path path, Session.Mode mode) { Tenant tenant = tenantRepository.getTenant(tenantName); - return getLocalSession(tenant, sessionId).getApplicationFile(Path.fromString(path), mode); + return getLocalSession(tenant, sessionId).getApplicationFile(Path.from(path.segments()), mode); } public Tenant getTenant(ApplicationId applicationId) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java index a8915d187f3..14acd1b5630 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java @@ -10,8 +10,14 @@ import com.yahoo.container.jdisc.HttpResponse; import java.util.HashSet; import java.util.List; import java.util.logging.Level; + +import com.yahoo.net.DomainName; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; +import com.yahoo.restapi.HttpURL.Scheme; import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.HttpFetcher; +import com.yahoo.vespa.config.server.http.HttpFetcher.Params; import com.yahoo.vespa.config.server.http.NotFoundException; import com.yahoo.vespa.config.server.http.SimpleHttpFetcher; @@ -33,7 +39,7 @@ public class HttpProxy { this.fetcher = fetcher; } - public HttpResponse get(Application application, String hostName, String serviceType, String relativePath) { + public HttpResponse get(Application application, String hostName, String serviceType, Path relativePath) { HostInfo host = application.getModel().getHosts().stream() .filter(hostInfo -> hostInfo.getHostname().equals(hostName)) .findFirst() @@ -54,18 +60,15 @@ public class HttpProxy { return internalGet(host.getHostname(), port.getPort(), relativePath); } - private HttpResponse internalGet(String hostname, int port, String relativePath) { - String urlString = "http://" + hostname + ":" + port + "/" + relativePath; - URL url; + private HttpResponse internalGet(String hostname, int port, Path relativePath) { + HttpURL url = HttpURL.create(Scheme.http, DomainName.of(hostname), port, relativePath); try { - url = new URL(urlString); + return fetcher.get(new Params(2000), // 2_000 ms read timeout + url.asURI().toURL()); } catch (MalformedURLException e) { - logger.log(Level.WARNING, "Badly formed url: " + urlString, e); + logger.log(Level.WARNING, "Badly formed url: " + url, e); return HttpErrorResponse.internalServerError("Failed to construct URL for backend"); } - - HttpFetcher.Params params = new HttpFetcher.Params(2000); // 2_000 ms read timeout - return fetcher.get(params, url); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java index 13acc121fa0..0ceb459233b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; @@ -32,7 +33,7 @@ public class ContentHandler { public HttpResponse put(ContentRequest request) { ApplicationFile file = request.getFile(); - if (request.getPath().endsWith("/")) { + if (request.getPath().hasTrailingSlash()) { createDirectory(request, file); } else { createFile(request, file); @@ -62,9 +63,9 @@ public class ContentHandler { return new SessionContentStatusResponse(file, urlBase); } - private static List<ApplicationFile> listSortedFiles(ApplicationFile file, String path, boolean recursive) { - if (!path.isEmpty() && !path.endsWith("/")) { - return Arrays.asList(file); + private static List<ApplicationFile> listSortedFiles(ApplicationFile file, Path path, boolean recursive) { + if ( ! path.segments().isEmpty() && ! path.hasTrailingSlash()) { + return List.of(file); } List<ApplicationFile> files = file.listFiles(recursive); Collections.sort(files); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java index 995c4b2dc56..f06e1dabf8c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.restapi.HttpURL.Path; import java.io.InputStream; @@ -20,11 +21,11 @@ public abstract class ContentRequest { enum ReturnType {CONTENT, STATUS} private final long sessionId; - private final String path; + private final Path path; private final ApplicationFile file; private final HttpRequest request; - protected ContentRequest(HttpRequest request, long sessionId, String path, ApplicationFile applicationFile) { + protected ContentRequest(HttpRequest request, long sessionId, Path path, ApplicationFile applicationFile) { this.request = request; this.sessionId = sessionId; this.path = path; @@ -77,7 +78,7 @@ public abstract class ContentRequest { } - String getPath() { + Path getPath() { return path; } @@ -86,8 +87,8 @@ public abstract class ContentRequest { } ApplicationFile getExistingFile() { - if (!file.exists()) { - throw new NotFoundException("Session " + sessionId + " does not contain a file '" + path + "'"); + if ( ! file.exists()) { + throw new NotFoundException("Session " + sessionId + " does not contain a file at " + path); } return file; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java index 0707261bb45..885456ff69c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java @@ -16,6 +16,7 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.Response; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.Path; import com.yahoo.slime.Cursor; @@ -132,23 +133,23 @@ public class ApplicationHandler extends HttpHandler { return HttpServiceResponse.createResponse(response, hostAndPort, request.getUri()); } - private HttpResponse serviceStatusPage(ApplicationId applicationId, String service, String hostname, String pathSuffix) { + private HttpResponse serviceStatusPage(ApplicationId applicationId, String service, String hostname, HttpURL.Path pathSuffix) { return applicationRepository.serviceStatusPage(applicationId, hostname, service, pathSuffix); } - private HttpResponse content(ApplicationId applicationId, String contentPath, HttpRequest request) { + private HttpResponse content(ApplicationId applicationId, HttpURL.Path contentPath, HttpRequest request) { long sessionId = applicationRepository.getSessionIdForApplication(applicationId); ApplicationFile applicationFile = applicationRepository.getApplicationFileFromSession(applicationId.tenant(), - sessionId, - contentPath, - ContentRequest.getApplicationFileMode(request.getMethod())); + sessionId, + contentPath, + ContentRequest.getApplicationFileMode(request.getMethod())); ApplicationContentRequest contentRequest = new ApplicationContentRequest(request, - sessionId, - applicationId, - zone, - contentPath, - applicationFile); + sessionId, + applicationId, + zone, + contentPath, + applicationFile); return new ContentHandler().get(contentRequest); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java index 17a8e1449c4..f6af9e616a9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.http.ContentRequest; import com.yahoo.vespa.config.server.http.ContentHandler; @@ -52,7 +53,7 @@ public class SessionContentHandler extends SessionHandler { TenantName tenantName = Utils.getTenantNameFromSessionRequest(request); validateRequest(tenantName); long sessionId = getSessionIdV2(request); - String contentPath = SessionContentRequestV2.getContentPath(request); + Path contentPath = SessionContentRequestV2.getContentPath(request); ApplicationFile applicationFile = applicationRepository.getApplicationFileFromSession(tenantName, sessionId, diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java index 34c842eca44..15d6c5c18ff 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java @@ -5,6 +5,7 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.config.server.http.ContentRequest; /** @@ -22,7 +23,7 @@ public class ApplicationContentRequest extends ContentRequest { long sessionId, ApplicationId applicationId, Zone zone, - String contentPath, + Path contentPath, ApplicationFile applicationFile) { super(request, sessionId, contentPath, applicationFile); this.applicationId = applicationId; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java index da71fd89054..449058eb911 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java @@ -5,6 +5,8 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.jdisc.application.BindingMatch; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.Path; import com.yahoo.vespa.config.server.http.ContentRequest; import com.yahoo.vespa.config.server.http.Utils; @@ -16,14 +18,14 @@ import com.yahoo.vespa.config.server.http.Utils; * @since 5.3 */ public class SessionContentRequestV2 extends ContentRequest { - private static final String uriPattern = "http://*/application/v2/tenant/*/session/*/content/*"; + private final TenantName tenantName; private final long sessionId; public SessionContentRequestV2(HttpRequest request, long sessionId, TenantName tenantName, - String path, + HttpURL.Path path, ApplicationFile applicationFile) { super(request, sessionId, path, applicationFile); this.tenantName = tenantName; @@ -35,8 +37,11 @@ public class SessionContentRequestV2 extends ContentRequest { return "/application/v2/tenant/" + tenantName.value() + "/session/" + sessionId; } - public static String getContentPath(HttpRequest request) { - BindingMatch<?> bm = Utils.getBindingMatch(request, uriPattern); - return bm.group(4); + public static HttpURL.Path getContentPath(HttpRequest request) { + Path path = new Path(request.getUri()); + if ( ! path.matches("/application/v2/tenant/{tenant}/session/{session}/content/{*}")) + throw new IllegalStateException("error in request routing"); + return path.getRest(); } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java index 30975be61e2..8b9714c3bfb 100644 --- a/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java @@ -5,6 +5,11 @@ import com.fasterxml.jackson.jaxrs.json.JacksonJsonProvider; import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; +import com.yahoo.net.DomainName; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; +import com.yahoo.restapi.HttpURL.Query; +import com.yahoo.restapi.HttpURL.Scheme; import com.yahoo.restapi.RestApi; import com.yahoo.restapi.RestApiRequestHandler; import com.yahoo.restapi.UriBuilder; @@ -106,7 +111,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl String regionName = context.pathParameters().getStringOrThrow("regionName"); String instanceName = context.pathParameters().getStringOrThrow("instanceName"); String identifier = context.pathParameters().getStringOrThrow("serviceIdentifier"); - String apiParams = context.pathParameters().getString("*").orElse(""); + Path apiParams = context.pathParameters().getRest().orElse(Path.empty()); return singleService(context.uriBuilder(), context.request().getUri(), tenantName, applicationName, environmentName, regionName, instanceName, identifier, apiParams); } @@ -125,7 +130,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl } protected HashMap<?, ?> singleService( - UriBuilder uriBuilder, URI requestUri, String tenantName, String applicationName, String environmentName, String regionName, String instanceName, String identifier, String apiParams) { + UriBuilder uriBuilder, URI requestUri, String tenantName, String applicationName, String environmentName, String regionName, String instanceName, String identifier, Path apiParams) { ServiceModel model = new ServiceModel(getModelConfig(tenantName, applicationName, environmentName, regionName, instanceName)); Service s = model.getService(identifier); int requestedPort = s.matchIdentifierWithPort(identifier); @@ -135,11 +140,9 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl return apiResult; } - protected HealthClient getHealthClient(String apiParams, Service s, int requestedPort, String uriQuery, Client client) { - final StringBuilder uriBuffer = new StringBuilder("http://").append(s.host).append(':').append(requestedPort).append('/') - .append(apiParams); - addQuery(uriQuery, uriBuffer); - WebTarget target = client.target(uriBuffer.toString()); + protected HealthClient getHealthClient(Path apiParams, Service s, int requestedPort, String uriQuery, Client client) { + URI uri = HttpURL.create(Scheme.http, DomainName.of(s.host), requestedPort, apiParams, Query.parse(uriQuery)).asURI(); + WebTarget target = client.target(uri); return WebResourceFactory.newResource(HealthClient.class, target); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java index ac0a6491100..83cae04cbfd 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.config.server.http.HttpFetcher; import com.yahoo.vespa.config.server.http.RequestTimeoutException; import com.yahoo.vespa.config.server.http.StaticResponse; @@ -48,7 +49,7 @@ public class HttpProxyTest { when(fetcher.get(actualParams.capture(), actualUrl.capture())).thenReturn(response); HttpResponse actualResponse = proxy.get(applicationMock, hostname, CLUSTERCONTROLLER_CONTAINER.serviceName, - "clustercontroller-status/v1/clusterName"); + Path.parse("clustercontroller-status/v1/clusterName")); assertEquals(1, actualParams.getAllValues().size()); assertEquals(2000, actualParams.getValue().readTimeoutMs); @@ -67,7 +68,7 @@ public class HttpProxyTest { when(fetcher.get(any(), any())).thenThrow(new RequestTimeoutException("timed out")); proxy.get(applicationMock, hostname, CLUSTERCONTROLLER_CONTAINER.serviceName, - "clustercontroller-status/v1/clusterName"); + Path.parse("clustercontroller-status/v1/clusterName")); } private static MockModel createClusterController() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java index 69caf86729f..0732379e0d9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java @@ -34,7 +34,7 @@ public class HandlerTest { if (contentType != null) { assertEquals(renderedString, contentType, response.getContentType()); } - assertTrue(renderedString.contains(message)); + assertTrue("\n" + renderedString + "\n should contain \n" + message, renderedString.contains(message)); } public static void assertHttpStatusCodeErrorCodeAndMessage(HttpResponse response, int statusCode, HttpErrorResponse.ErrorCode errorCode, String message) throws IOException { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java index 005dd715dd4..9f7e539a2e3 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java @@ -15,6 +15,7 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.HttpRequest.Method; +import com.yahoo.restapi.HttpURL; import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.MockLogRetriever; @@ -356,16 +357,20 @@ public class ApplicationHandlerTest { .withHttpProxy(mockHttpProxy) .build(); ApplicationHandler mockHandler = createApplicationHandler(applicationRepository); - doAnswer(invoc -> new StaticResponse(200, "text/html", "<html>" + - "host=" + invoc.getArgument(1, String.class) + "," + - "service=" + invoc.getArgument(2, String.class) + "," + - "path=" + invoc.getArgument(3, String.class) + "</html>")).when(mockHttpProxy).get(any(), any(), any(), any()); + doAnswer(invoc -> new StaticResponse(200, + "text/html", + "<html>" + + "host=" + invoc.getArgument(1, String.class) + "," + + "service=" + invoc.getArgument(2, String.class) + "," + + "path=" + invoc.getArgument(3, HttpURL.Path.class) + + "</html>")) + .when(mockHttpProxy).get(any(), any(), any(), any()); HttpResponse response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/container-clustercontroller/" + host + "/status/some/path/clusterName1", GET)); - assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=container-clustercontroller,path=clustercontroller-status/v1/some/path/clusterName1</html>"); + assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=container-clustercontroller,path=path '/clustercontroller-status/v1/some/path/clusterName1'</html>"); response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/distributor/" + host + "/status/something", GET)); - assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=distributor,path=something</html>"); + assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=distributor,path=path '/something'</html>"); response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/fake-service/" + host + "/status/something", GET)); assertHttpStatusCodeAndMessage(response, 404, "{\"error-code\":\"NOT_FOUND\",\"message\":\"No status page for service: fake-service\"}"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java index 790be6d45ba..7c2e0be0c3a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java @@ -121,13 +121,13 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase { @Test public void require_that_nonexistent_file_returns_not_found_when_deleted() throws IOException { - assertDeleteFile(Response.Status.NOT_FOUND, "/test2.txt", "{\"error-code\":\"NOT_FOUND\",\"message\":\"Session " + sessionId + " does not contain a file 'test2.txt'\"}"); + assertDeleteFile(Response.Status.NOT_FOUND, "/test2.txt", "{\"error-code\":\"NOT_FOUND\",\"message\":\"Session " + sessionId + " does not contain a file at path '/test2.txt'\"}"); } @Test public void require_that_files_can_be_deleted() throws IOException { assertDeleteFile(Response.Status.OK, "/test.txt"); - assertDeleteFile(Response.Status.NOT_FOUND, "/test.txt", "{\"error-code\":\"NOT_FOUND\",\"message\":\"Session " + sessionId + " does not contain a file 'test.txt'\"}"); + assertDeleteFile(Response.Status.NOT_FOUND, "/test.txt", "{\"error-code\":\"NOT_FOUND\",\"message\":\"Session " + sessionId + " does not contain a file at path '/test.txt'\"}"); assertDeleteFile(Response.Status.BAD_REQUEST, "/newtest", "{\"error-code\":\"BAD_REQUEST\",\"message\":\"File 'newtest' is not an empty directory\"}"); assertDeleteFile(Response.Status.OK, "/newtest/testfile.txt"); assertDeleteFile(Response.Status.OK, "/newtest"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java index 702dd2792da..f77c7611858 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.MockProvisioner; import com.yahoo.vespa.config.server.application.CompressedApplicationInputStreamTest; @@ -155,7 +156,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { public void require_that_handler_unpacks_application() throws IOException { File outFile = CompressedApplicationInputStreamTest.createTarFile(); createHandler().handle(post(outFile)); - ApplicationFile applicationFile = applicationRepository.getApplicationFileFromSession(tenant, 2, "services.xml", Session.Mode.READ); + ApplicationFile applicationFile = applicationRepository.getApplicationFileFromSession(tenant, 2, Path.parse("services.xml"), Session.Mode.READ); assertTrue(applicationFile.exists()); } diff --git a/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java index fe025c9e861..35e63c46bf8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.serviceview; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.jdisc.test.MockMetric; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.restapi.UriBuilder; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import com.yahoo.vespa.serviceview.bindings.HealthClient; @@ -45,7 +46,7 @@ public class StateRequestHandlerTest { } @Override - protected HealthClient getHealthClient(String apiParams, Service s, int requestedPort, String uriQuery, Client client) { + protected HealthClient getHealthClient(Path apiParams, Service s, int requestedPort, String uriQuery, Client client) { HealthClient healthClient = Mockito.mock(HealthClient.class); HashMap<Object, Object> dummyHealthData = new HashMap<>(); HashMap<String, String> dummyLink = new HashMap<>(); @@ -75,7 +76,7 @@ public class StateRequestHandlerTest { public final void test() { Service s = correspondingModel.resolve("vespa.yahoo.com", 8080, null); String api = "/state/v1"; - HashMap<?, ?> boom = testHandler.singleService(new UriBuilder("http://someserver:8080"), URI.create(EXTERNAL_BASE_URI), "default", "default", "default", "default", "default", s.getIdentifier(8080), api); + HashMap<?, ?> boom = testHandler.singleService(new UriBuilder("http://someserver:8080"), URI.create(EXTERNAL_BASE_URI), "default", "default", "default", "default", "default", s.getIdentifier(8080), Path.parse(api)); assertEquals(EXTERNAL_BASE_URI + "tenant/default/application/default/environment/default/region/default/instance/default/service/" + s.getIdentifier(8080) + api, ((Map<?, ?>) ((List<?>) boom.get("resources")).get(0)).get("url")); } diff --git a/container-core/src/main/java/com/yahoo/restapi/HttpURL.java b/container-core/src/main/java/com/yahoo/restapi/HttpURL.java index 9705b6f0e40..e890b0fe71a 100644 --- a/container-core/src/main/java/com/yahoo/restapi/HttpURL.java +++ b/container-core/src/main/java/com/yahoo/restapi/HttpURL.java @@ -223,14 +223,24 @@ public class HttpURL { segment, "path segments cannot be \"\", \".\", or \"..\""); } + /** Returns a copy of this where only the first segments are retained, and with a trailing slash. */ + public Path head(int count) { + return count == segments.size() ? this : new Path(segments.subList(0, count), true, validator); + } + + /** Returns a copy of this where only the last segments are retained. */ + public Path tail(int count) { + return count == segments.size() ? this : new Path(segments.subList(segments.size() - count, segments.size()), trailingSlash, validator); + } + /** Returns a copy of this where the first segments are skipped. */ public Path skip(int count) { - return new Path(segments.subList(count, segments.size()), trailingSlash, validator); + return count == 0 ? this : new Path(segments.subList(count, segments.size()), trailingSlash, validator); } - /** Returns a copy of this where the last segments are cut off. */ + /** Returns a copy of this where the last segments are cut off, and with a trailing slash. */ public Path cut(int count) { - return new Path(segments.subList(0, segments.size() - count), trailingSlash, validator); + return count == 0 ? this : new Path(segments.subList(0, segments.size() - count), true, validator); } /** Returns a copy of this with the <em>decoded</em> segment appended at the end; it may not be either of {@code ""}, {@code "."} or {@code ".."}. */ @@ -254,6 +264,11 @@ public class HttpURL { return new Path(copy, trailingSlash, validator); } + /** Whether this path has a trailing slash. */ + public boolean hasTrailingSlash() { + return trailingSlash; + } + /** Returns a copy of this which encodes a trailing slash. */ public Path withTrailingSlash() { return new Path(segments, true, validator); diff --git a/container-core/src/main/java/com/yahoo/restapi/Path.java b/container-core/src/main/java/com/yahoo/restapi/Path.java index 8a494d47be4..c639432db89 100644 --- a/container-core/src/main/java/com/yahoo/restapi/Path.java +++ b/container-core/src/main/java/com/yahoo/restapi/Path.java @@ -66,11 +66,9 @@ public class Path { else if ( ! specElements.get(i).equals(path.segments().get(i))) return false; } - - if (matchPrefix) { - this.rest = path.skip(specElements.size()); - } - + + rest = matchPrefix ? path.skip(specElements.size()) : null; + return true; } @@ -99,12 +97,10 @@ public class Path { } /** - * Returns the rest of the last matched path. - * This is always the empty string (never null) unless the path spec ends with {*} + * Returns the rest of the last matched path, or {@code null} if the path spec didn't end with {*}. */ - public String getRest() { - String raw = rest.raw(); - return raw.isEmpty() ? raw : raw.substring(1); + public HttpURL.Path getRest() { + return rest; } @Override diff --git a/container-core/src/main/java/com/yahoo/restapi/RestApi.java b/container-core/src/main/java/com/yahoo/restapi/RestApi.java index d6cc0cccf3f..353ac3eb5cc 100644 --- a/container-core/src/main/java/com/yahoo/restapi/RestApi.java +++ b/container-core/src/main/java/com/yahoo/restapi/RestApi.java @@ -153,7 +153,9 @@ public interface RestApi { default double getDoubleOrThrow(String name) { return Double.parseDouble(getStringOrThrow(name)); } } - interface PathParameters extends Parameters {} + interface PathParameters extends Parameters { + Optional<HttpURL.Path> getRest(); + } interface QueryParameters extends Parameters { List<String> getStringList(String name); } diff --git a/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java b/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java index 09de7ffa133..cc243a3e92b 100644 --- a/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java +++ b/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java @@ -429,16 +429,16 @@ class RestApiImpl implements RestApi { private class PathParametersImpl implements RestApi.RequestContext.PathParameters { @Override public Optional<String> getString(String name) { - if (name.equals("*")) { - String rest = pathMatcher.getRest(); - return rest.isEmpty() ? Optional.empty() : Optional.of(rest); - } return Optional.ofNullable(pathMatcher.get(name)); } @Override public String getStringOrThrow(String name) { return getString(name) .orElseThrow(() -> new RestApiException.BadRequest("Path parameter '" + name + "' is missing")); } + @Override public Optional<HttpURL.Path> getRest() { + return Optional.ofNullable(pathMatcher.getRest()); + } + } private class QueryParametersImpl implements RestApi.RequestContext.QueryParameters { diff --git a/container-core/src/test/java/com/yahoo/restapi/HttpURLTest.java b/container-core/src/test/java/com/yahoo/restapi/HttpURLTest.java index 05a218b0f04..858513c2a69 100644 --- a/container-core/src/test/java/com/yahoo/restapi/HttpURLTest.java +++ b/container-core/src/test/java/com/yahoo/restapi/HttpURLTest.java @@ -124,6 +124,11 @@ class HttpURLTest { assertEquals(List.of(expected.get(2), expected.get(0)), path.append(path).cut(2).skip(2).segments()); + for (int i = 0; i < 3; i++) { + assertEquals(path.head(i), path.cut(3 - i)); + assertEquals(path.tail(i), path.skip(3 - i)); + } + assertThrows(NullPointerException.class, () -> path.append((String) null)); diff --git a/container-core/src/test/java/com/yahoo/restapi/PathTest.java b/container-core/src/test/java/com/yahoo/restapi/PathTest.java index 065b02be0e4..4786eb9775c 100644 --- a/container-core/src/test/java/com/yahoo/restapi/PathTest.java +++ b/container-core/src/test/java/com/yahoo/restapi/PathTest.java @@ -35,7 +35,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("", path.getRest()); + assertEquals("/", path.getRest().raw()); } { @@ -43,7 +43,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("kanoo", path.getRest()); + assertEquals("/kanoo", path.getRest().raw()); } { @@ -51,7 +51,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("kanoo/trips", path.getRest()); + assertEquals("/kanoo/trips", path.getRest().raw()); } { @@ -59,7 +59,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("kanoo/trips/", path.getRest()); + assertEquals("/kanoo/trips/", path.getRest().raw()); } } diff --git a/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java b/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java index b2b9dd47514..bbce02ed97a 100644 --- a/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java +++ b/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java @@ -28,6 +28,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.UncheckedIOException; import java.util.logging.Level; @@ -71,24 +72,23 @@ public class GUIHandler extends ThreadedHttpRequestHandler { if (path.matches("/querybuilder/")) { return new FileResponse("_includes/index.html", null, null); } - if (!path.matches("/querybuilder/{*}") ) { + if ( ! path.matches("/querybuilder/{*}") ) { return ErrorResponse.notFoundError("Nothing at path:" + path); } - String filepath = path.getRest(); - if (!isValidPath(filepath) && !filepath.equals("config.json")){ + String filepath = String.join("/", path.getRest().segments()); + if ( ! filepath.equals("config.json") && ! isValidPath(filepath)){ return ErrorResponse.notFoundError("Nothing at path:" + filepath); } return new FileResponse(filepath, indexModel, rankProfilesConfig); } private static boolean isValidPath(String path) { - InputStream in = GUIHandler.class.getClassLoader().getResourceAsStream("gui/"+path); - boolean isValid = (in != null); - if(isValid){ - try { in.close(); } catch (IOException e) {/* Problem with closing inputstream */} + InputStream in = GUIHandler.class.getClassLoader().getResourceAsStream("gui/" + path); + if (in != null){ + try { in.close(); } catch (IOException e) { throw new UncheckedIOException(e); } + return true; } - - return isValid; + return false; } private static class FileResponse extends HttpResponse { @@ -112,12 +112,12 @@ public class GUIHandler extends ThreadedHttpRequestHandler { String json = "{}"; try { json = getGUIConfig(); } catch (IOException e) { /*Something happened while parsing JSON */ } is = new ByteArrayInputStream(json.getBytes()); - } else{ - is = GUIHandler.class.getClassLoader().getResourceAsStream("gui/"+this.path); + } else { + is = GUIHandler.class.getClassLoader().getResourceAsStream("gui/" + this.path); } byte[] buf = new byte[1024]; int numRead; - while ( (numRead = is.read(buf) ) >= 0) { + while ((numRead = is.read(buf)) >= 0) { out.write(buf, 0, numRead); } } @@ -152,8 +152,6 @@ public class GUIHandler extends ThreadedHttpRequestHandler { return "image/x-icon"; } else if (path.endsWith(".json")) { return "application/json"; - } else if (path.endsWith(".ttf")) { - return "font/ttf"; } return "text/html"; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index b9b7881745a..42368fa358d 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.controller.api.integration.configserver; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.net.DomainName; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData; @@ -52,9 +54,9 @@ public interface ConfigServer { ApplicationView getApplicationView(String tenantName, String applicationName, String instanceName, String environment, String region); - Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, String restPath); + Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, Path restPath); - String getServiceStatusPage(DeploymentId deployment, String serviceName, String node, String subPath); + String getServiceStatusPage(DeploymentId deployment, String serviceName, DomainName node, Path subPath); /** * Gets the Vespa logs of the given deployment. @@ -73,7 +75,7 @@ public interface ConfigServer { * @param path path within package to get * @param requestUri request URI on the controller, used to rewrite paths in response from config server */ - ProxyResponse getApplicationPackageContent(DeploymentId deployment, String path, URI requestUri); + ProxyResponse getApplicationPackageContent(DeploymentId deployment, Path path, URI requestUri); List<ClusterMetrics> getDeploymentMetrics(DeploymentId deployment); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java index 2a1b8a19475..7e1c9c8884f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java @@ -3,6 +3,9 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; +import com.yahoo.restapi.HttpURL.Query; import com.yahoo.text.Text; import java.io.InputStream; import java.net.URI; @@ -22,28 +25,27 @@ import static com.yahoo.jdisc.http.HttpRequest.Method; public class ProxyRequest { private final Method method; - private final URI requestUri; + private final HttpURL requestUri; private final Map<String, List<String>> headers; private final InputStream requestData; private final List<URI> targets; - private final String targetPath; + private final Path targetPath; - ProxyRequest(Method method, URI url, Map<String, List<String>> headers, InputStream body, List<URI> targets, - String path) { - Objects.requireNonNull(url); - if (!url.getPath().endsWith(path)) { - throw new IllegalArgumentException(Text.format("Request path '%s' does not end with proxy path '%s'", url.getPath(), path)); + ProxyRequest(Method method, URI uri, Map<String, List<String>> headers, InputStream body, List<URI> targets, Path path) { + this.requestUri = HttpURL.from(uri); + if ( requestUri.path().segments().size() < path.segments().size() + || ! requestUri.path().tail(path.segments().size()).equals(path)) { + throw new IllegalArgumentException(Text.format("Request %s does not end with proxy %s", requestUri.path(), path)); } if (targets.isEmpty()) { throw new IllegalArgumentException("targets must be non-empty"); } this.method = Objects.requireNonNull(method); - this.requestUri = Objects.requireNonNull(url); this.headers = Objects.requireNonNull(headers); this.requestData = body; this.targets = List.copyOf(targets); - this.targetPath = path.startsWith("/") ? path : "/" + path; + this.targetPath = path; } @@ -64,24 +66,12 @@ public class ProxyRequest { } public URI createConfigServerRequestUri(URI baseURI) { - try { - return new URI(baseURI.getScheme(), baseURI.getUserInfo(), baseURI.getHost(), - baseURI.getPort(), targetPath, requestUri.getQuery(), requestUri.getFragment()); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } + return HttpURL.from(baseURI).withPath(targetPath).withQuery(requestUri.query()).asURI(); } public URI getControllerPrefixUri() { - String prefixPath = targetPath.equals("/") && !requestUri.getPath().endsWith("/") ? - requestUri.getPath() + targetPath : - requestUri.getPath().substring(0, requestUri.getPath().length() - targetPath.length() + 1); - try { - return new URI(requestUri.getScheme(), requestUri.getUserInfo(), requestUri.getHost(), - requestUri.getPort(), prefixPath, null, null); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } + Path prefixPath = requestUri.path().cut(targetPath.segments().size()).withTrailingSlash(); + return requestUri.withPath(prefixPath).withQuery(Query.empty()).asURI(); } @Override @@ -90,7 +80,7 @@ public class ProxyRequest { } /** Create a proxy request that repeatedly tries a single target */ - public static ProxyRequest tryOne(URI target, String path, HttpRequest request) { + public static ProxyRequest tryOne(URI target, Path path, HttpRequest request) { return new ProxyRequest(request.getMethod(), request.getUri(), request.getJDiscRequest().headers(), request.getData(), List.of(target), path); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java index 0b629a577d0..9ac30898f8b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; import org.apache.http.client.utils.URIBuilder; import java.io.IOException; @@ -29,19 +31,8 @@ public class ProxyResponse extends HttpResponse { super(statusResponse); this.contentType = contentType; - final String configServerPrefix; - final String controllerRequestPrefix; - try { - configServerPrefix = new URIBuilder() - .setScheme(configServer.getScheme()) - .setHost(configServer.getHost()) - .setPort(configServer.getPort()) - .setPath("/") - .build().toString(); - controllerRequestPrefix = controllerRequest.getControllerPrefixUri().toString(); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } + String configServerPrefix = HttpURL.from(configServer).withPath(Path.empty()).asURI().toString(); + String controllerRequestPrefix = controllerRequest.getControllerPrefixUri().toString(); bodyResponseRewritten = bodyResponse.replace(configServerPrefix, controllerRequestPrefix); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index d9a38a5b578..c2c4049bcc3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import ai.vespa.hosted.api.Signatures; +import ai.vespa.validation.Validation; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Joiner; @@ -25,8 +26,10 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.io.IOUtils; +import com.yahoo.net.DomainName; import com.yahoo.restapi.ByteArrayResponse; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.Path; import com.yahoo.restapi.ResourceResponse; @@ -155,6 +158,7 @@ import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.Stream; +import static ai.vespa.validation.Validation.requireAtLeast; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.CONFLICT; import static com.yahoo.yolean.Exceptions.uncheck; @@ -270,7 +274,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return getReindexing(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/suspended")) return suspended(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/state/v1/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{host}/status/{*}")) return status(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.get("host"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/clusters")) return clusters(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/content/{*}")) return content(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.getRest(), request); @@ -284,7 +289,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/suspended")) return suspended(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/state/v1/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{host}/status/{*}")) return status(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.get("host"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/clusters")) return clusters(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/logs")) return logs(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request.propertyMap()); @@ -1857,40 +1863,29 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { return response; } - private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String restPath, HttpRequest request) { + private HttpResponse status(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String host, HttpURL.Path restPath, HttpRequest request) { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); + String result = controller.serviceRegistry().configServer().getServiceStatusPage(deploymentId, + serviceName, + DomainName.of(host), + restPath); // TODO: add query + return new HtmlResponse(result); + } - if (restPath.contains("/status/")) { - String[] parts = restPath.split("/status/", 2); - String result = controller.serviceRegistry().configServer().getServiceStatusPage(deploymentId, serviceName, parts[0], parts[1]); - return new HtmlResponse(result); - } - - String normalizedRestPath = URI.create(restPath).normalize().toString(); - // Only state/v1 is allowed - if (! normalizedRestPath.startsWith("state/v1/")) { - return ErrorResponse.forbidden("Access denied"); - } - + private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, HttpURL.Path restPath, HttpRequest request) { + DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); Map<?,?> result = controller.serviceRegistry().configServer().getServiceApiResponse(deploymentId, serviceName, restPath); ServiceApiResponse response = new ServiceApiResponse(deploymentId.zoneId(), deploymentId.applicationId(), List.of(controller.zoneRegistry().getConfigServerVipUri(deploymentId.zoneId())), request.getUri()); - response.setResponse(result, serviceName, restPath); + response.setResponse(result, serviceName, HttpURL.Path.parse("/state/v1").append(restPath)); return response; } - private HttpResponse content(String tenantName, String applicationName, String instanceName, String environment, String region, String restPath, HttpRequest request) { + private HttpResponse content(String tenantName, String applicationName, String instanceName, String environment, String region, HttpURL.Path restPath, HttpRequest request) { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); - - String normalizedRestPath = URI.create("content/" + restPath).normalize().toString(); - // Only content/ is allowed - if ( ! normalizedRestPath.startsWith("content/")) { - return ErrorResponse.forbidden("Access denied"); - } - - return controller.serviceRegistry().configServer().getApplicationPackageContent(deploymentId, "/" + restPath, request.getUri()); + return controller.serviceRegistry().configServer().getApplicationPackageContent(deploymentId, restPath, request.getUri()); } private HttpResponse updateTenant(String tenantName, HttpRequest request) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java index 888c402b6ec..a21f93bdaea 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java @@ -4,6 +4,9 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL; +import com.yahoo.restapi.HttpURL.Path; +import com.yahoo.restapi.HttpURL.Query; import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; @@ -34,11 +37,11 @@ class ServiceApiResponse extends HttpResponse { private final ApplicationId application; private final List<URI> configServerURIs; private final Slime slime; - private final UriBuilder requestUri; + private final HttpURL requestUri; // Only set for one of the setResponse calls private String serviceName = null; - private String restPath = null; + private Path restPath = null; public ServiceApiResponse(ZoneId zone, ApplicationId application, List<URI> configServerURIs, URI requestUri) { super(200); @@ -46,7 +49,7 @@ class ServiceApiResponse extends HttpResponse { this.application = application; this.configServerURIs = configServerURIs; this.slime = new Slime(); - this.requestUri = new UriBuilder(requestUri).withoutParameters(); + this.requestUri = HttpURL.from(requestUri).withQuery(Query.empty()); } public void setResponse(ApplicationView applicationView) { @@ -68,7 +71,7 @@ class ServiceApiResponse extends HttpResponse { } } - public void setResponse(Map<?,?> responseData, String serviceName, String restPath) { + public void setResponse(Map<?,?> responseData, String serviceName, Path restPath) { this.serviceName = serviceName; this.restPath = restPath; mapToSlime(responseData, slime.setObject()); @@ -138,7 +141,7 @@ class ServiceApiResponse extends HttpResponse { mapToSlime((Map)entry, array.addObject()); } - private String rewriteIfUrl(String urlOrAnyString, UriBuilder requestUri) { + private String rewriteIfUrl(String urlOrAnyString, HttpURL requestUri) { if (urlOrAnyString == null) return null; String hostPattern = "(" + @@ -163,19 +166,18 @@ class ServiceApiResponse extends HttpResponse { if (matcher.find()) { String proxiedPath = urlOrAnyString.substring(matcher.group().length()); - return requestUri.append(proxiedPath).toString(); + return requestUri.withPath(requestUri.path().append(Path.parse(proxiedPath))).asURI().toString(); } else { return urlOrAnyString; // not a service url } } - private UriBuilder generateLocalLinkPrefix(String identifier, String restPath) { - String proxiedPath = identifier + "/" + restPath; - - if (this.requestUri.toString().endsWith(proxiedPath)) { - return new UriBuilder(this.requestUri.toString().substring(0, this.requestUri.toString().length() - proxiedPath.length())); + private HttpURL generateLocalLinkPrefix(String identifier, Path restPath) { + Path proxiedPath = Path.parse(identifier).append(restPath); + if (requestUri.path().tail(proxiedPath.segments().size()).equals(proxiedPath)) { + return requestUri.withPath(requestUri.path().cut(proxiedPath.segments().size())); } else { - throw new IllegalStateException("Expected the resource path '" + this.requestUri + "' to end with '" + proxiedPath + "'"); + throw new IllegalStateException("Expected the resource " + requestUri.path() + " to end with " + proxiedPath); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java index 0e09825ec41..27a8cbeaf3e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; @@ -21,8 +22,11 @@ import com.yahoo.yolean.Exceptions; import java.net.URI; import java.util.List; import java.util.logging.Level; +import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.restapi.HttpURL.Path.parse; + /** * REST API for proxying operator APIs to config servers in a given zone. * @@ -32,7 +36,9 @@ import java.util.stream.Stream; public class ConfigServerApiHandler extends AuditLoggingRequestHandler { private static final URI CONTROLLER_URI = URI.create("https://localhost:4443/"); - private static final List<String> WHITELISTED_APIS = List.of("/flags/v1/", "/nodes/v2/", "/orchestrator/v1/"); + private static final List<HttpURL.Path> WHITELISTED_APIS = List.of(parse("/flags/v1/"), + parse("/nodes/v2/"), + parse("/orchestrator/v1/")); private final ZoneRegistry zoneRegistry; private final ConfigServerRestExecutor proxy; @@ -84,17 +90,18 @@ public class ConfigServerApiHandler extends AuditLoggingRequestHandler { } ZoneId zoneId = ZoneId.from(path.get("environment"), path.get("region")); - if (! zoneRegistry.hasZone(zoneId) && ! controllerZone.equals(zoneId)) { + if ( ! zoneRegistry.hasZone(zoneId) && ! controllerZone.equals(zoneId)) { throw new IllegalArgumentException("No such zone: " + zoneId.value()); } - String cfgPath = "/" + path.getRest(); - if (WHITELISTED_APIS.stream().noneMatch(cfgPath::startsWith)) { - return ErrorResponse.forbidden("Cannot access '" + cfgPath + - "' through /configserver/v1, following APIs are permitted: " + String.join(", ", WHITELISTED_APIS)); + if (path.getRest().segments().size() < 2 || ! WHITELISTED_APIS.contains(path.getRest().head(2).withTrailingSlash())) { + return ErrorResponse.forbidden("Cannot access " + path.getRest() + + " through /configserver/v1, following APIs are permitted: " + WHITELISTED_APIS.stream() + .map(p -> "/" + String.join("/", p.segments()) + "/") + .collect(Collectors.joining(", "))); } - return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), cfgPath, request)); + return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), path.getRest(), request)); } private HttpResponse root(HttpRequest request) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java index a7416cfa0ed..40a402f209b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java @@ -7,6 +7,7 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; @@ -107,7 +108,7 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { return ErrorResponse.notFoundError("Nothing at " + path); } - private ProxyRequest proxyRequest(ZoneId zoneId, String path, HttpRequest request) { + private ProxyRequest proxyRequest(ZoneId zoneId, HttpURL.Path path, HttpRequest request) { return ProxyRequest.tryOne(zoneRegistry.getConfigServerVipUri(zoneId), path, request); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index e0ce1c060bc..1643f1f818b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -13,6 +13,8 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.net.DomainName; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.text.Text; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -521,7 +523,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer // Returns a canned example response @Override - public Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, String restPath) { + public Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, Path restPath) { Map<String,List<?>> root = new HashMap<>(); List<Map<?,?>> resources = new ArrayList<>(); Map<String,String> resource = new HashMap<>(); @@ -532,7 +534,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public String getServiceStatusPage(DeploymentId deployment, String serviceName, String node, String subPath) { + public String getServiceStatusPage(DeploymentId deployment, String serviceName, DomainName node, Path subPath) { return "<h1>OK</h1>"; } @@ -567,8 +569,8 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public ProxyResponse getApplicationPackageContent(DeploymentId deployment, String path, URI requestUri) { - return new ProxyResponse(("{\"path\":\"" + path + "\"}").getBytes(StandardCharsets.UTF_8), "application/json", 200); + public ProxyResponse getApplicationPackageContent(DeploymentId deployment, Path path, URI requestUri) { + return new ProxyResponse(("{\"path\":\"/" + String.join("/", path.segments()) + "\"}").getBytes(StandardCharsets.UTF_8), "application/json", 200); } public void setLogStream(Supplier<String> log) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java index 0b28d94386d..e10a41cbae3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java @@ -6,6 +6,7 @@ import com.github.tomakehurst.wiremock.stubbing.Scenario; import com.yahoo.config.provision.SystemName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.restapi.HttpURL.Path; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.yolean.concurrent.Sleeper; import org.apache.http.protocol.HttpContext; @@ -46,7 +47,7 @@ public class ConfigServerRestExecutorImplTest { stubRequests(path); HttpRequest request = HttpRequest.createTestRequest(url.toString(), com.yahoo.jdisc.http.HttpRequest.Method.GET); - ProxyRequest proxyRequest = ProxyRequest.tryOne(url, path, request); + ProxyRequest proxyRequest = ProxyRequest.tryOne(url, Path.parse(path), request); // Request is retried HttpResponse response = proxy.handle(proxyRequest); @@ -71,7 +72,7 @@ public class ConfigServerRestExecutorImplTest { stubRequests(path); HttpRequest request = HttpRequest.createTestRequest(url.toString(), com.yahoo.jdisc.http.HttpRequest.Method.GET); - ProxyRequest proxyRequest = ProxyRequest.tryOne(url, path, request); + ProxyRequest proxyRequest = ProxyRequest.tryOne(url, Path.parse(path), request); // Connections are reused assertEquals(200, proxy.handle(proxyRequest).getStatus()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java index 8542530628c..e8b5df7efa1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.jdisc.http.HttpRequest; +import com.yahoo.restapi.HttpURL.Path; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -11,20 +12,18 @@ import java.util.List; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; /** * @author Haakon Dybdahl */ public class ProxyRequestTest { - @SuppressWarnings("deprecation") - @Rule - public final ExpectedException exception = ExpectedException.none(); - @Test public void testBadUri() { - exception.expectMessage("Request path '/path' does not end with proxy path '/zone/v2/'"); - testRequest("http://domain.tld/path", "/zone/v2/"); + assertThrows("Request path '/path' does not end with proxy path '/zone/v2/'", + IllegalArgumentException.class, + () -> testRequest("http://domain.tld/path", "/zone/v2/")); } @Test @@ -33,7 +32,7 @@ public class ProxyRequestTest { // Root request ProxyRequest request = testRequest("http://controller.domain.tld/my/path", ""); assertEquals(URI.create("http://controller.domain.tld/my/path/"), request.getControllerPrefixUri()); - assertEquals(URI.create("https://cfg.prod.us-north-1.domain.tld:1234/"), + assertEquals(URI.create("https://cfg.prod.us-north-1.domain.tld:1234"), request.createConfigServerRequestUri(URI.create("https://cfg.prod.us-north-1.domain.tld:1234/"))); } @@ -64,7 +63,7 @@ public class ProxyRequestTest { private static ProxyRequest testRequest(String url, String pathPrefix) { return new ProxyRequest(HttpRequest.Method.GET, URI.create(url), Map.of(), null, - List.of(URI.create("http://example.com")), pathPrefix); + List.of(URI.create("http://example.com")), Path.parse(pathPrefix)); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java index 4cc2c1cc79e..1ba0200eec3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.jdisc.http.HttpRequest; +import com.yahoo.restapi.HttpURL.Path; import org.junit.Test; import java.io.ByteArrayOutputStream; @@ -20,7 +21,7 @@ public class ProxyResponseTest { @Test public void testRewriteUrl() throws Exception { ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("http://domain.tld/zone/v2/dev/us-north-1/configserver"), - Map.of(), null, List.of(URI.create("http://example.com")), "configserver"); + Map.of(), null, List.of(URI.create("http://example.com")), Path.parse("configserver")); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", @@ -31,14 +32,14 @@ public class ProxyResponseTest { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); proxyResponse.render(outputStream); - String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); + String document = outputStream.toString(StandardCharsets.UTF_8); assertEquals("response link is http://domain.tld/zone/v2/dev/us-north-1/bla/bla/", document); } @Test public void testRewriteSecureUrl() throws Exception { ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("https://domain.tld/zone/v2/prod/eu-south-3/configserver"), - Map.of(), null, List.of(URI.create("http://example.com")), "configserver"); + Map.of(), null, List.of(URI.create("http://example.com")), Path.parse("configserver")); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", @@ -49,7 +50,7 @@ public class ProxyResponseTest { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); proxyResponse.render(outputStream); - String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); + String document = outputStream.toString(StandardCharsets.UTF_8); assertEquals("response link is https://domain.tld/zone/v2/prod/eu-south-3/bla/bla/", document); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 581c0160640..9736745d257 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -1657,15 +1657,15 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request(serviceApi + "/storagenode-awe3slno6mmq2fye191y324jl/document/v1/", GET) .userIdentity(USER_ID) .oAuthCredentials(OKTA_CREDENTIALS), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Access denied\"}", - 403); + "{\"error-code\":\"NOT_FOUND\",\"message\":\"Nothing at path '/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/service/storagenode-awe3slno6mmq2fye191y324jl/document/v1/'\"}", + 404); // Test path traversal tester.assertResponse(request(serviceApi + "/storagenode-awe3slno6mmq2fye191y324jl/state/v1/../../document/v1/", GET) .userIdentity(USER_ID) .oAuthCredentials(OKTA_CREDENTIALS), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Access denied\"}", - 403); + "{\"error-code\":\"NOT_FOUND\",\"message\":\"Nothing at path '/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/service/storagenode-awe3slno6mmq2fye191y324jl/document/v1/'\"}", + 404); // Test urlencoded path traversal tester.assertResponse(request(serviceApi + "/storagenode-awe3slno6mmq2fye191y324jl/state%2Fv1%2F..%2F..%2Fdocument%2Fv1%2F", GET) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java index 9d0054f327a..ce6f713a13d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java @@ -50,6 +50,11 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest { tester.assertResponse(operatorRequest("http://localhost:8080/configserver/v1"), new File("root.json")); + // GET /configserver/v1/nodes/v2 + tester.assertResponse(operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2"), + "ok"); + assertLastRequest("https://cfg.prod.us-north-1.test.vip:4443/", "GET"); + // GET /configserver/v1/nodes/v2/node/?recursive=true tester.assertResponse(operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node/?recursive=true"), "ok"); @@ -83,12 +88,12 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest { @Test public void test_allowed_apis() { // GET /configserver/v1/prod/us-north-1 - tester.assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1"), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access '/' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", + tester.assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/"), + "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access path '/' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", 403); tester.assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/application/v2/tenant/vespa"), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access '/application/v2/tenant/vespa' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", + "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access path '/application/v2/tenant/vespa' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}", 403); } diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index f59b9f1e372..29c572422c3 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -1472,7 +1472,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { return new DocumentId("id:" + requireNonNull(path.get("namespace")) + ":" + requireNonNull(path.get("documentType")) + ":" + group.map(Group::docIdPart).orElse("") + - ":" + requireNonNull(path.getRest())); + ":" + String.join("/", requireNonNull(path.getRest()).segments())); // :'( } String rawPath() { return rawPath; } diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index e69631e8375..642fdd5c16f 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -583,6 +583,7 @@ "final" ], "methods": [ + "public static com.yahoo.path.Path from(java.util.List)", "public boolean isChildOf(com.yahoo.path.Path)", "public com.yahoo.path.Path append(java.lang.String)", "public com.yahoo.path.Path append(com.yahoo.path.Path)", diff --git a/vespajlib/src/main/java/com/yahoo/path/Path.java b/vespajlib/src/main/java/com/yahoo/path/Path.java index 16370059c8c..737a27c57d8 100644 --- a/vespajlib/src/main/java/com/yahoo/path/Path.java +++ b/vespajlib/src/main/java/com/yahoo/path/Path.java @@ -41,6 +41,11 @@ public final class Path { this.delimiter = delimiter; } + /** Creates a new path with the given segments. */ + public static Path from(List<String> segments) { + return new Path(segments, "/"); + } + /** Returns whether this path is an immediate child of the given path */ public boolean isChildOf(Path parent) { return toString().startsWith(parent.toString()) && this.elements.size() -1 == parent.elements.size(); diff --git a/vespajlib/src/main/java/com/yahoo/text/Text.java b/vespajlib/src/main/java/com/yahoo/text/Text.java index 30eba3ebd65..501ca980187 100644 --- a/vespajlib/src/main/java/com/yahoo/text/Text.java +++ b/vespajlib/src/main/java/com/yahoo/text/Text.java @@ -53,6 +53,7 @@ public final class Text { : isTextCharAboveUsAscii(codepoint); } private static boolean isTextCharAboveUsAscii(int codepoint) { + // TODO jonmv: compute modulo? if (codepoint < 0xFDD0) return true; if (codepoint <= 0xFDDF) return false; if (codepoint < 0x1FFFE) return true; |