diff --git a/dev/README.md b/dev/README.md index 12e68cc59..3f4e9a3d3 100644 --- a/dev/README.md +++ b/dev/README.md @@ -1 +1,8 @@ -ADK development utilities such as Spring REST server for agent. \ No newline at end of file +ADK development utilities such as Spring REST server for agent. + +## Serving the dev UI + +The UI and its assets are served under `/dev-ui/`, and both `/` and `/dev-ui` +redirect there, keeping the query string. The assets are not served from the +origin root: `/adk_favicon.svg` and the like return 404, and only the `/dev-ui/` +form resolves. diff --git a/dev/src/main/java/com/google/adk/web/AdkWebServer.java b/dev/src/main/java/com/google/adk/web/AdkWebServer.java index b321cef27..0a6035923 100644 --- a/dev/src/main/java/com/google/adk/web/AdkWebServer.java +++ b/dev/src/main/java/com/google/adk/web/AdkWebServer.java @@ -25,6 +25,7 @@ import com.google.adk.memory.InMemoryMemoryService; import com.google.adk.sessions.BaseSessionService; import com.google.adk.sessions.InMemorySessionService; +import com.google.adk.web.config.DevUiAssets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; @@ -109,48 +110,33 @@ public MappingJackson2HttpMessageConverter mappingJackson2HttpMessageConverter( } /** - * Configures resource handlers for serving static content (like the Dev UI). Maps requests - * starting with "/dev-ui/" to the directory specified by the 'adk.web.ui.dir' system property. + * Maps requests under "/dev-ui/" to the directory named by the 'adk.web.ui.dir' property, or to + * the bundled copy on the classpath when that is unset. */ @Override public void addResourceHandlers(ResourceHandlerRegistry registry) { - if (webUiDir != null && !webUiDir.isEmpty()) { - // Ensure the path uses forward slashes and ends with a slash - String location = webUiDir.replace("\\", "/"); - if (!location.startsWith("file:")) { - location = "file:" + location; // Ensure file: prefix - } - if (!location.endsWith("/")) { - location += "/"; - } - log.debug("Mapping URL path /** to static resources at location: {}", location); - registry - .addResourceHandler("/**") - .addResourceLocations(location) - .setCachePeriod(0) - .resourceChain(true); - - } else { - log.debug( - "System property 'adk.web.ui.dir' or config 'adk.web.ui.dir' is not set. Mapping URL path" - + " /** to classpath:/browser/"); - registry - .addResourceHandler("/**") - .addResourceLocations("classpath:/browser/") - .setCachePeriod(0) - .resourceChain(true); - } + String location = DevUiAssets.assetRoot(webUiDir); + log.debug("Mapping URL path /dev-ui/** to static resources at location: {}", location); + registry + .addResourceHandler("/dev-ui/**") + .addResourceLocations(location) + .setCachePeriod(0) + .resourceChain(true); } /** - * Configures simple automated controllers: - Redirects the root path "/" to "/dev-ui". - Forwards - * requests to "/dev-ui" to "/dev-ui/index.html" so the ResourceHandler serves it. + * Configures simple automated controllers: "/" and "/dev-ui" both redirect to "/dev-ui/", which + * forwards to the UI's index.html. The trailing slash is required: index.html declares a {@code + * }, so served from "/dev-ui" the app resolves its own router path to "dev-ui" + * and matches none of its routes. The query string is carried across because the UI selects its + * agent from {@code ?app=} and the sample READMEs send users to the slashless "/dev-ui", so a + * redirect that dropped it would silently ignore the selection. */ @Override public void addViewControllers(ViewControllerRegistry registry) { - registry.addRedirectViewController("/", "/dev-ui"); - registry.addViewController("/dev-ui").setViewName("forward:/index.html"); - registry.addViewController("/dev-ui/").setViewName("forward:/index.html"); + registry.addRedirectViewController("/", "/dev-ui/").setKeepQueryParams(true); + registry.addRedirectViewController("/dev-ui", "/dev-ui/").setKeepQueryParams(true); + registry.addViewController("/dev-ui/").setViewName("forward:/dev-ui/index.html"); } /** diff --git a/dev/src/main/java/com/google/adk/web/config/DevUiAssets.java b/dev/src/main/java/com/google/adk/web/config/DevUiAssets.java new file mode 100644 index 000000000..775dd7422 --- /dev/null +++ b/dev/src/main/java/com/google/adk/web/config/DevUiAssets.java @@ -0,0 +1,46 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.adk.web.config; + +import org.jspecify.annotations.Nullable; +import org.springframework.core.io.ResourceLoader; + +/** + * Where the dev UI's static assets live. Normalizes {@code adk.web.ui.dir} into a resource + * location, so callers that need it do not each do it differently. + */ +public final class DevUiAssets { + + private static final String CLASSPATH_ROOT = ResourceLoader.CLASSPATH_URL_PREFIX + "/browser/"; + + /** + * The asset root: {@code webUiDir} as a {@code file:} URL when set, else the bundled classpath + * copy. Always ends in a slash. + */ + public static String assetRoot(@Nullable String webUiDir) { + if (webUiDir == null || webUiDir.isEmpty()) { + return CLASSPATH_ROOT; + } + String location = webUiDir.replace("\\", "/"); + if (!location.startsWith("file:")) { + location = "file:" + location; + } + return location.endsWith("/") ? location : location + "/"; + } + + private DevUiAssets() {} +} diff --git a/dev/src/test/java/com/google/adk/web/AdkWebServerConfiguredUiDirTest.java b/dev/src/test/java/com/google/adk/web/AdkWebServerConfiguredUiDirTest.java new file mode 100644 index 000000000..0d4c0109d --- /dev/null +++ b/dev/src/test/java/com/google/adk/web/AdkWebServerConfiguredUiDirTest.java @@ -0,0 +1,77 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.adk.web; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.test.web.servlet.MockMvc; + +/** + * A directory named by {@code adk.web.ui.dir} is really served under {@code /dev-ui/}. The other + * tests cover the bundled classpath copy, and {@code DevUiAssetsTest} only checks the string this + * normalizes to, which cannot tell whether the resource handler can resolve it. + */ +@SpringBootTest +@AutoConfigureMockMvc +public class AdkWebServerConfiguredUiDirTest { + + @TempDir static Path uiDir; + + @DynamicPropertySource + static void configuredUiDir(DynamicPropertyRegistry registry) { + // @SpringBootTest(properties=...) needs a constant; the temp directory is only known now. + registry.add("adk.web.ui.dir", () -> uiDir.toString()); + } + + @BeforeAll + static void writeUi() throws IOException { + Files.writeString(uiDir.resolve("index.html"), "temp ui"); + Files.writeString(uiDir.resolve("asset.txt"), "from the configured dir"); + } + + @Autowired private MockMvc mockMvc; + + @Test + public void configuredUiDir_shouldBeServedBelowDevUi() throws Exception { + // The body, not just a 200: it proves the bytes came from the configured directory. + mockMvc + .perform(get("/dev-ui/asset.txt")) + .andExpect(status().isOk()) + .andExpect(content().string("from the configured dir")); + } + + @Test + public void configuredUiDir_shouldSupplyTheForwardTarget() throws Exception { + mockMvc + .perform(get("/dev-ui/index.html")) + .andExpect(status().isOk()) + .andExpect(content().string("temp ui")); + } +} diff --git a/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java b/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java new file mode 100644 index 000000000..023249e5b --- /dev/null +++ b/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java @@ -0,0 +1,80 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.adk.web; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.test.web.servlet.MockMvc; + +/** + * Spring's own forwarded-header support keeps working: with {@code + * server.forward-headers-strategy=framework} the operator opts in, {@code ForwardedHeaderFilter} + * turns the forwarded prefix into the request's context path, and the redirect picks it up. This + * server adds nothing here and reads no header itself; the test exists so enabling that Spring + * feature keeps behaving as it did. + * + *

The filter honours the standard {@code Forwarded} header as well as {@code X-Forwarded-*}, so + * both are covered. + */ +@SpringBootTest(properties = "server.forward-headers-strategy=framework") +@AutoConfigureMockMvc +public class AdkWebServerProxyRedirectTest { + + @Autowired private MockMvc mockMvc; + + @ParameterizedTest + @ValueSource(strings = {"/", "/dev-ui"}) + public void devUiEntryPoints_withSpringForwardedHeaderSupport_shouldKeepThePrefix(String path) + throws Exception { + mockMvc + .perform( + get(path) + .header("X-Forwarded-Prefix", "/my-app") + .header("X-Forwarded-Host", "gw.example.com") + .header("X-Forwarded-Proto", "https")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("https://gw.example.com/my-app/dev-ui/")); + } + + @ParameterizedTest + @ValueSource(strings = {"/", "/dev-ui"}) + public void devUiEntryPoints_withTheStandardForwardedHeader_shouldUseItsHost(String path) + throws Exception { + // RFC 7239 defines no prefix parameter, so this one moves the host and scheme but not the path. + mockMvc + .perform(get(path).header("Forwarded", "host=gw.example.com;proto=https")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("https://gw.example.com/dev-ui/")); + } + + @Test + public void devUiEntryPoint_withNoForwardedHeaders_shouldNotGainAPrefix() throws Exception { + // Control arm: the filter is installed here, so this pins that it leaves this case alone. + mockMvc + .perform(get("/")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/dev-ui/")); + } +} diff --git a/dev/src/test/java/com/google/adk/web/AdkWebServerUITest.java b/dev/src/test/java/com/google/adk/web/AdkWebServerUITest.java index cfb162db2..5d1b46cce 100644 --- a/dev/src/test/java/com/google/adk/web/AdkWebServerUITest.java +++ b/dev/src/test/java/com/google/adk/web/AdkWebServerUITest.java @@ -17,6 +17,7 @@ package com.google.adk.web; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -40,22 +41,52 @@ public class AdkWebServerUITest { @Autowired private MockMvc mockMvc; - @Test - public void rootShouldRedirectToDevUi() throws Exception { + @ParameterizedTest + @ValueSource(strings = {"/", "/dev-ui"}) + public void devUiEntryPoints_shouldRedirectToTrailingSlashForm(String path) throws Exception { + // index.html declares , which only resolves correctly from "/dev-ui/". mockMvc - .perform(get("/")) + .perform(get(path)) .andExpect(status().is3xxRedirection()) - .andExpect(redirectedUrl("/dev-ui")); + .andExpect(redirectedUrl("/dev-ui/")); + } + + @Test + public void devUiRoot_shouldForwardToIndexHtml() throws Exception { + // MockMvc records a forward without running it, so assert the target, not a status. + mockMvc.perform(get("/dev-ui/")).andExpect(forwardedUrl("/dev-ui/index.html")); + } + + @Test + public void devUiIndexHtml_shouldBeServed() throws Exception { + // The other half: the target the forward names actually resolves through the mount. + mockMvc.perform(get("/dev-ui/index.html")).andExpect(status().isOk()); + } + + @Test + public void devUiAssets_shouldBeServedBelowDevUi() throws Exception { + mockMvc.perform(get("/dev-ui/adk_favicon.svg")).andExpect(status().isOk()); } @ParameterizedTest - @ValueSource(strings = {"/dev-ui", "/dev-ui/"}) - public void devUiEndpointsShouldReturnOk(String path) throws Exception { - mockMvc.perform(get(path)).andExpect(status().isOk()); + @ValueSource(strings = {"/", "/dev-ui"}) + public void devUiEntryPoints_shouldKeepQueryString(String path) throws Exception { + // The UI picks its agent from ?app=, and the sample READMEs send users to "/dev-ui". + mockMvc + .perform(get(path + "?app=my-agent")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/dev-ui/?app=my-agent")); + } + + @Test + public void devUiAssets_shouldNotBeServedAtRoot() throws Exception { + // Narrowing the handler from "/**" to "/dev-ui/**" makes this deliberately unreachable. + mockMvc.perform(get("/adk_favicon.svg")).andExpect(status().isNotFound()); } @Test - public void nonExistentUiPageShouldReturnNotFound() throws Exception { - mockMvc.perform(get("/non-existent-page")).andExpect(status().isNotFound()); + public void nonExistentDevUiPath_shouldReturnNotFound() throws Exception { + // No SPA fallback, deliberately: a deep link 404s rather than serving index.html. + mockMvc.perform(get("/dev-ui/non-existent-page")).andExpect(status().isNotFound()); } } diff --git a/dev/src/test/java/com/google/adk/web/config/DevUiAssetsTest.java b/dev/src/test/java/com/google/adk/web/config/DevUiAssetsTest.java new file mode 100644 index 000000000..3c77c5c82 --- /dev/null +++ b/dev/src/test/java/com/google/adk/web/config/DevUiAssetsTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.adk.web.config; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.jupiter.api.Test; + +/** The {@code adk.web.ui.dir} normalization the resource handler resolves assets through. */ +public class DevUiAssetsTest { + + @Test + public void assetRoot_noDirConfigured_fallsBackToTheBundledCopy() { + assertThat(DevUiAssets.assetRoot(null)).isEqualTo("classpath:/browser/"); + assertThat(DevUiAssets.assetRoot("")).isEqualTo("classpath:/browser/"); + } + + @Test + public void assetRoot_configuredDir_becomesAFileUrl() { + assertThat(DevUiAssets.assetRoot("/srv/ui")).isEqualTo("file:/srv/ui/"); + assertThat(DevUiAssets.assetRoot("/srv/ui/")).isEqualTo("file:/srv/ui/"); + assertThat(DevUiAssets.assetRoot("file:/srv/ui")).isEqualTo("file:/srv/ui/"); + } + + @Test + public void assetRoot_windowsSeparators_areNormalized() { + assertThat(DevUiAssets.assetRoot("C:\\srv\\ui")).isEqualTo("file:C:/srv/ui/"); + } +}