This is an automated email from the ASF dual-hosted git repository. smolnar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push: new 46cdc1593 KNOX-3001 - Avoid double XML-escaping during topology persistence from descriptors (#834) 46cdc1593 is described below commit 46cdc159342b6b637b96f8396c36c515f5b4943e Author: Sandor Molnar <smol...@apache.org> AuthorDate: Thu Jan 18 11:39:55 2024 +0100 KNOX-3001 - Avoid double XML-escaping during topology persistence from descriptors (#834) --- .../simple/SimpleDescriptorHandlerTest.java | 24 +++++++++++++++++++++- .../conf/shared-providers/test-providers.json | 2 +- .../topology/simple/SimpleDescriptorHandler.java | 15 +++++++++++--- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java index 301570ccb..c33500c4b 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandlerTest.java @@ -20,6 +20,8 @@ package org.apache.knox.gateway.topology.simple; import org.apache.commons.io.FileUtils; import org.apache.knox.gateway.config.GatewayConfig; import org.apache.knox.gateway.services.GatewayServices; +import org.apache.knox.gateway.services.ServiceType; +import org.apache.knox.gateway.services.topology.TopologyService; import org.apache.knox.gateway.topology.validation.TopologyValidator; import org.apache.knox.gateway.util.XmlUtils; import org.easymock.EasyMock; @@ -35,6 +37,7 @@ import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; import java.io.ByteArrayInputStream; import java.io.File; @@ -870,15 +873,19 @@ public class SimpleDescriptorHandlerTest { try { final File destDir = new File(System.getProperty("java.io.tmpdir")).getCanonicalFile(); final File descriptorFile = new File(SimpleDescriptorHandlerTest.class.getResource("/conf-full/conf/descriptors/test-topology.json").getFile()); + final TopologyService topologyService = EasyMock.createNiceMock(TopologyService.class); + EasyMock.expect(topologyService.getTopologies()).andReturn(Collections.emptyList()).anyTimes(); final GatewayServices gatewayServices = EasyMock.createNiceMock(GatewayServices.class); + EasyMock.expect(gatewayServices.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(topologyService).anyTimes(); GatewayConfig gc = EasyMock.createNiceMock(GatewayConfig.class); EasyMock.expect(gc.getReadOnlyOverrideTopologyNames()).andReturn(Collections.emptyList()).anyTimes(); - EasyMock.replay(gc); + EasyMock.replay(topologyService, gatewayServices, gc); final Map<String, File> handleResult = SimpleDescriptorHandler.handle(gc, descriptorFile, destDir, gatewayServices); topologyFile = handleResult.get(SimpleDescriptorHandler.RESULT_TOPOLOGY); final Document topologyXml = XmlUtils.readXml(topologyFile); assertThat(topologyXml, hasXPath("/topology/service/role", is(equalTo("KNOX")))); assertThat(topologyXml, hasXPath("/topology/gateway/provider/name", is(equalTo("ShiroProvider")))); + verifyUserSearchFilterParam(topologyXml); } finally { if (topologyFile != null) { topologyFile.delete(); @@ -886,6 +893,21 @@ public class SimpleDescriptorHandlerTest { } } + private void verifyUserSearchFilterParam(final Document topologyXml) throws XPathExpressionException { + final XPath xpath = XPathFactory.newInstance().newXPath(); + final NodeList providerNodes = (NodeList) xpath.compile("/topology/gateway/provider").evaluate(topologyXml, XPathConstants.NODESET); + assertEquals(1, providerNodes.getLength()); + final Node shiroProviderNode = providerNodes.item(0); + final NodeList shiroParamNodes = (NodeList) xpath.compile("param").evaluate(shiroProviderNode, XPathConstants.NODESET); + assertEquals(12, shiroParamNodes.getLength()); + final Node userSearchFilterNode = shiroParamNodes.item(7); + // name + assertEquals(userSearchFilterNode.getChildNodes().item(1).getChildNodes().item(0).getNodeValue(), "main.ldapRealm.userSearchFilter"); + // value + assertEquals(userSearchFilterNode.getChildNodes().item(3).getChildNodes().item(0).getNodeValue(), + "(&(&(objectclass=person)(sAMAccountName={0}))(|(memberOf=CN=SecXX-users,OU=ManagedGroups,OU=Groups,OU=XX,OU=xx,DC=xx,DC=int)(memberOf=CN=SecXX-rls-serviceuser,OU=ManagedGroups,OU=Groups,OU=XX,OU=xx,DC=xx,DC=int)))"); + } + private File writeProviderConfig(String path, String content) throws IOException { File f = new File(path); FileUtils.write(f, content, StandardCharsets.UTF_8); diff --git a/gateway-server/src/test/resources/conf-full/conf/shared-providers/test-providers.json b/gateway-server/src/test/resources/conf-full/conf/shared-providers/test-providers.json index 7139d1208..09fc50fe5 100644 --- a/gateway-server/src/test/resources/conf-full/conf/shared-providers/test-providers.json +++ b/gateway-server/src/test/resources/conf-full/conf/shared-providers/test-providers.json @@ -11,7 +11,7 @@ "main.ldapRealm.contextFactory.authenticationMechanism" : "simple", "main.ldapRealm.contextFactory.url" : "ldap://localhost:33389", "main.ldapRealm.userDnTemplate" : "uid=0ou=people,dc=hadoop,dc=apache,dc=org", - "main.ldapRealm.userSearchFilter" : "(&(&(objectclass=person)(sAMAccountName={0}))(|(memberOf=CN=SecXX-users,OU=ManagedGroups,OU=Groups,OU=XX,OU=xx,DC=xx,DC=int)(memberOf=CN=SecXX-rls-serviceuser,OU=ManagedGroups,OU=Groups,OU=XX,OU=xx,DC=xx,DC=int)))", + "main.ldapRealm.userSearchFilter" : "(&(&(objectclass=person)(sAMAccountName={0}))(|(memberOf=CN=SecXX-users,OU=ManagedGroups,OU=Groups,OU=XX,OU=xx,DC=xx,DC=int)(memberOf=CN=SecXX-rls-serviceuser,OU=ManagedGroups,OU=Groups,OU=XX,OU=xx,DC=xx,DC=int)))", "redirectToUrl" : "/${GATEWAY_PATH}/knoxsso/knoxauth/login.html", "restrictedCookies" : "rememberme,WWW-Authenticate", "sessionTimeout" : "30", diff --git a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java index 1ce217efd..b5419f55e 100644 --- a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java +++ b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java @@ -471,7 +471,7 @@ public class SimpleDescriptorHandler { for (Map.Entry<String, String> param : provider.getParams().entrySet()) { sw.write(" <param>\n"); sw.write(" <name>" + param.getKey() + "</name>\n"); - sw.write(" <value>" + StringEscapeUtils.escapeXml11(param.getValue()) + "</value>\n"); + sw.write(" <value>" + getXmlEscapedValue(param.getValue()) + "</value>\n"); sw.write(" </param>\n"); } @@ -565,7 +565,7 @@ public class SimpleDescriptorHandler { if (!(svcParam.getKey().toLowerCase(Locale.ROOT)).startsWith(SimpleDescriptor.DISCOVERY_PARAM_PREFIX)) { sw.write(" <param>\n"); sw.write(" <name>" + svcParam.getKey() + "</name>\n"); - sw.write(" <value>" + StringEscapeUtils.escapeXml11(svcParam.getValue()) + "</value>\n"); + sw.write(" <value>" + getXmlEscapedValue(svcParam.getValue()) + "</value>\n"); sw.write(" </param>\n"); } } @@ -595,7 +595,7 @@ public class SimpleDescriptorHandler { for (Entry<String, String> entry : appParams.entrySet()) { sw.write(" <param>\n"); sw.write(" <name>" + entry.getKey() + "</name>\n"); - sw.write(" <value>" + StringEscapeUtils.escapeXml11(entry.getValue()) + "</value>\n"); + sw.write(" <value>" + getXmlEscapedValue(entry.getValue()) + "</value>\n"); sw.write(" </param>\n"); } } @@ -635,6 +635,15 @@ public class SimpleDescriptorHandler { return result; } + /* + * First, undoes any previous manual XML escape. + * Second applies XML-escape on the result of the first step. + */ + private static String getXmlEscapedValue(String value) { + final String unescapedValue = StringEscapeUtils.unescapeXml(value); + return StringEscapeUtils.escapeXml10(unescapedValue); + } + /** * Determine whether or not the generated content of the specified topology should be persisted. *