From b4163a3ef0dc75971f286401bc68646104e717ab Mon Sep 17 00:00:00 2001 From: JinwooHwang Date: Mon, 24 Aug 2026 08:29:30 -0400 Subject: [PATCH] GEODE-10612: Encode region path in Pulse region-detail error messages When a region-detail request names a path that does not resolve, the region services return "Region [] is not available" in the errorOnRegion field, which the Pulse UI displays. Paths containing characters such as '<' or '&' did not display correctly. Encode the path with StringEscapeUtils.escapeHtml4 when building the message, so it displays as written. commons-text is already a compile dependency of geode-pulse. Region lookup is unchanged and continues to use the path as supplied. Adds RegionErrorMessageEncodingTest and RegionDetailErrorMessageIntegrationTest. Co-Authored-By: Claude Opus 5 --- ...gionDetailErrorMessageIntegrationTest.java | 155 ++++++++++++++++++ .../service/ClusterSelectedRegionService.java | 5 +- .../ClusterSelectedRegionsMemberService.java | 5 +- .../RegionErrorMessageEncodingTest.java | 132 +++++++++++++++ 4 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java create mode 100644 geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java diff --git a/geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java b/geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java new file mode 100644 index 00000000000..94aa800e6e6 --- /dev/null +++ b/geode-pulse/src/integrationTest/java/org/apache/geode/tools/pulse/controllers/RegionDetailErrorMessageIntegrationTest.java @@ -0,0 +1,155 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You 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 org.apache.geode.tools.pulse.controllers; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; +import static org.mockito.quality.Strictness.LENIENT; +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; +import static org.springframework.http.MediaType.parseMediaType; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.security.Principal; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.context.WebApplicationContext; + +import org.apache.geode.test.junit.categories.PulseTest; +import org.apache.geode.tools.pulse.internal.data.Cluster; +import org.apache.geode.tools.pulse.internal.data.Repository; + +/** + * Covers the region-detail error message end to end, from the {@code /pulseUpdate} request the + * Pulse UI posts through to the JSON it receives back. + */ +@Category({PulseTest.class}) +@RunWith(SpringJUnit4ClassRunner.class) +@WebAppConfiguration +@ContextConfiguration("classpath*:WEB-INF/pulse-servlet.xml") +@ActiveProfiles({"pulse.controller.test"}) +public class RegionDetailErrorMessageIntegrationTest { + + private static final String PATH_WITH_SPECIAL_CHARACTERS = "/orders<2026>&archive"; + private static final String ENCODED_MESSAGE = + "Region [/orders<2026>&archive] is not available"; + + private static final MediaType JSON_MEDIA_TYPE = parseMediaType(APPLICATION_JSON_VALUE); + private static final Principal PRINCIPAL = () -> "test-user"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Rule + public MockitoRule mockitoRule = MockitoJUnit.rule().strictness(LENIENT); + + @Autowired + private WebApplicationContext wac; + + @Autowired + private Repository repository; + + @Mock + Cluster cluster; + + private MockMvc mockMvc; + + @Before + public void setup() { + when(repository.getCluster()).thenReturn(cluster); + when(cluster.getServerName()).thenReturn("mock-cluster"); + // The requested path resolves to no region, so the services take the error branch. + when(cluster.getClusterRegion(anyString())).thenReturn(null); + + mockMvc = MockMvcBuilders.webAppContextSetup(wac).build(); + } + + @Test + public void pulseUpdateEncodesSpecialCharactersForClusterSelectedRegion() throws Exception { + MvcResult result = mockMvc + .perform(post("/pulseUpdate") + .with(csrf()) + .param("pulseData", pulseData("ClusterSelectedRegion", PATH_WITH_SPECIAL_CHARACTERS)) + .principal(PRINCIPAL) + .accept(JSON_MEDIA_TYPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ClusterSelectedRegion.selectedRegion.errorOnRegion") + .value(ENCODED_MESSAGE)) + .andReturn(); + + assertThat(result.getResponse().getContentAsString()) + .contains("/orders<2026>&archive"); + } + + @Test + public void pulseUpdateEncodesSpecialCharactersForClusterSelectedRegionsMember() + throws Exception { + MvcResult result = mockMvc + .perform(post("/pulseUpdate") + .with(csrf()) + .param("pulseData", + pulseData("ClusterSelectedRegionsMember", PATH_WITH_SPECIAL_CHARACTERS)) + .principal(PRINCIPAL) + .accept(JSON_MEDIA_TYPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ClusterSelectedRegionsMember.selectedRegionsMembers.errorOnRegion") + .value(ENCODED_MESSAGE)) + .andReturn(); + + assertThat(result.getResponse().getContentAsString()) + .contains("/orders<2026>&archive"); + } + + @Test + public void pulseUpdateLeavesOrdinaryRegionPathUnchanged() throws Exception { + mockMvc + .perform(post("/pulseUpdate") + .with(csrf()) + .param("pulseData", pulseData("ClusterSelectedRegion", "/mock-region")) + .principal(PRINCIPAL) + .accept(JSON_MEDIA_TYPE)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.ClusterSelectedRegion.selectedRegion.errorOnRegion") + .value("Region [/mock-region] is not available")); + } + + /** Builds the {@code pulseData} body the Pulse frontend posts for the region-detail page. */ + private static String pulseData(String service, String regionFullPath) { + ObjectNode parameters = MAPPER.createObjectNode(); + parameters.put("regionFullPath", regionFullPath); + ObjectNode root = MAPPER.createObjectNode(); + root.set(service, parameters); + return root.toString(); + } +} diff --git a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java index f99c5015504..74f55416ee1 100644 --- a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java +++ b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionService.java @@ -30,6 +30,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -222,7 +223,9 @@ private ObjectNode getSelectedRegionJson(Cluster cluster, String selectedRegionF return regionJSON; } else { ObjectNode responseJSON = mapper.createObjectNode(); - responseJSON.put("errorOnRegion", "Region [" + selectedRegionFullPath + "] is not available"); + responseJSON.put("errorOnRegion", + "Region [" + StringEscapeUtils.escapeHtml4(selectedRegionFullPath) + + "] is not available"); return responseJSON; } } diff --git a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java index 238cdabc898..6d1af501d2c 100644 --- a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java +++ b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/ClusterSelectedRegionsMemberService.java @@ -25,6 +25,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.text.StringEscapeUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -146,7 +147,9 @@ private ObjectNode getSelectedRegionsMembersJson(Cluster cluster, String selecte return regionMemberJSON; } else { ObjectNode responseJSON = mapper.createObjectNode(); - responseJSON.put("errorOnRegion", "Region [" + selectedRegionFullPath + "] is not available"); + responseJSON.put("errorOnRegion", + "Region [" + StringEscapeUtils.escapeHtml4(selectedRegionFullPath) + + "] is not available"); return responseJSON; } } diff --git a/geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java b/geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java new file mode 100644 index 00000000000..2d1b0867444 --- /dev/null +++ b/geode-pulse/src/test/java/org/apache/geode/tools/pulse/internal/service/RegionErrorMessageEncodingTest.java @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You 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 org.apache.geode.tools.pulse.internal.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.security.Principal; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import jakarta.servlet.http.HttpServletRequest; +import org.junit.Before; +import org.junit.Test; + +import org.apache.geode.tools.pulse.internal.data.Cluster; +import org.apache.geode.tools.pulse.internal.data.Repository; + +/** + * Tests the {@code errorOnRegion} message the region-detail services produce when the requested + * region path does not resolve. + * + *

+ * Paths containing characters such as {@code <} or {@code &} are encoded so the message displays + * as written. Lookup is unaffected and uses the path as supplied. + */ +public class RegionErrorMessageEncodingTest { + + private static final String PATH_WITH_SPECIAL_CHARACTERS = "/orders<2026>&archive"; + private static final String ENCODED_MESSAGE = + "Region [/orders<2026>&archive] is not available"; + private static final String ORDINARY_PATH = "/mock-region"; + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private Repository repository; + private Cluster cluster; + private HttpServletRequest request; + + @Before + public void setUp() { + repository = mock(Repository.class); + cluster = mock(Cluster.class); + request = mock(HttpServletRequest.class); + Principal principal = mock(Principal.class); + when(principal.getName()).thenReturn("admin"); + when(request.getUserPrincipal()).thenReturn(principal); + when(repository.getCluster()).thenReturn(cluster); + when(cluster.getServerName()).thenReturn("mock-cluster"); + // No region resolves, so every call below takes the error branch. + when(cluster.getClusterRegion(anyString())).thenReturn(null); + } + + @Test + public void clusterSelectedRegionEncodesSpecialCharactersInErrorMessage() throws Exception { + assertThat(selectedRegionError(PATH_WITH_SPECIAL_CHARACTERS)).isEqualTo(ENCODED_MESSAGE); + } + + @Test + public void clusterSelectedRegionsMemberEncodesSpecialCharactersInErrorMessage() + throws Exception { + assertThat(selectedRegionsMemberError(PATH_WITH_SPECIAL_CHARACTERS)).isEqualTo(ENCODED_MESSAGE); + } + + @Test + public void clusterSelectedRegionLeavesOrdinaryPathUnchanged() throws Exception { + assertThat(selectedRegionError(ORDINARY_PATH)) + .isEqualTo("Region [" + ORDINARY_PATH + "] is not available"); + } + + @Test + public void clusterSelectedRegionsMemberLeavesOrdinaryPathUnchanged() throws Exception { + assertThat(selectedRegionsMemberError(ORDINARY_PATH)) + .isEqualTo("Region [" + ORDINARY_PATH + "] is not available"); + } + + @Test + public void clusterSelectedRegionLooksTheRegionUpByTheSuppliedPath() throws Exception { + selectedRegionError(PATH_WITH_SPECIAL_CHARACTERS); + + verify(cluster).getClusterRegion(PATH_WITH_SPECIAL_CHARACTERS); + } + + @Test + public void clusterSelectedRegionsMemberLooksTheRegionUpByTheSuppliedPath() throws Exception { + selectedRegionsMemberError(PATH_WITH_SPECIAL_CHARACTERS); + + verify(cluster).getClusterRegion(PATH_WITH_SPECIAL_CHARACTERS); + } + + private String selectedRegionError(String regionFullPath) throws Exception { + when(request.getParameter("pulseData")) + .thenReturn(pulseData("ClusterSelectedRegion", regionFullPath)); + + ObjectNode json = new ClusterSelectedRegionService(repository).execute(request); + + return json.get("selectedRegion").get("errorOnRegion").asText(); + } + + private String selectedRegionsMemberError(String regionFullPath) throws Exception { + when(request.getParameter("pulseData")) + .thenReturn(pulseData("ClusterSelectedRegionsMember", regionFullPath)); + + ObjectNode json = new ClusterSelectedRegionsMemberService(repository).execute(request); + + return json.get("selectedRegionsMembers").get("errorOnRegion").asText(); + } + + /** Builds the {@code pulseData} body the Pulse frontend posts for the region-detail page. */ + private static String pulseData(String service, String regionFullPath) { + ObjectNode parameters = MAPPER.createObjectNode(); + parameters.put("regionFullPath", regionFullPath); + ObjectNode root = MAPPER.createObjectNode(); + root.set(service, parameters); + return root.toString(); + } +}