Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for issue #86 - Possible classpath scanning issues by using XPathFactory.newInstance() #87

Open
wants to merge 1 commit into
base: release/1.8
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions dxa-framework/dxa-tridion-provider/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,9 @@
<groupId>jdom</groupId>
<artifactId>jdom</artifactId>
</dependency>
<dependency>
<groupId>xalan</groupId>
<artifactId>xalan</artifactId>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import com.sdl.webapp.common.controller.exception.NotFoundException;
import com.sdl.webapp.common.util.NodeListAdapter;
import com.sdl.webapp.common.util.XMLUtils;
import com.sdl.webapp.tridion.xpath.XPathResolver;
import com.sdl.webapp.tridion.xpath.XPathExpressionResolver;
import org.apache.commons.lang3.StringUtils;
import org.dd4t.contentmodel.ComponentPresentation;
import org.dd4t.core.exceptions.FactoryException;
Expand Down Expand Up @@ -196,7 +196,7 @@ private RichText resolveRichText(Document doc, Localization localization)
private List<EntityModel> resolveImages(Document doc, Localization localization) throws ContentProviderException, SemanticMappingException {
List<Node> entityElements;
try {
entityElements = new NodeListAdapter((NodeList) XPathResolver.XPATH_IMAGES.expr().get().evaluate(doc, NODESET));
entityElements = new NodeListAdapter((NodeList) XPathExpressionResolver.XPATH_IMAGES.expr().evaluate(doc, NODESET));
} catch (XPathExpressionException e) {
LOG.warn("Error while evaluation XPath expression", e);
return null;
Expand Down Expand Up @@ -230,7 +230,7 @@ private List<EntityModel> resolveImages(Document doc, Localization localization)
private void resolveLinks(Document document) {
final List<Node> linkElements;
try {
linkElements = new NodeListAdapter((NodeList) XPathResolver.XPATH_LINKS.expr().get().evaluate(document, NODESET));
linkElements = new NodeListAdapter((NodeList) XPathExpressionResolver.XPATH_LINKS.expr().evaluate(document, NODESET));
} catch (XPathExpressionException e) {
LOG.warn("Error while evaluation XPath expression", e);
return;
Expand All @@ -248,7 +248,7 @@ private void resolveLinks(Document document) {

if (isEmpty(linkUrl)) {
// Resolve a dynamic component link
linkUrl = linkResolver.resolveLink(linkElement.getAttributeNS(XPathResolver.XLINK_NS_URI, "href"), null, true);
linkUrl = linkResolver.resolveLink(linkElement.getAttributeNS(XPathExpressionResolver.XLINK_NS_URI, "href"), null, true);
}

if (!isEmpty(linkUrl)) {
Expand Down Expand Up @@ -282,7 +282,7 @@ private void applyHashIfApplicable(Element linkElement) {
}

private String getLinkName(Element linkElement) {
final String componentUri = linkElement.getAttributeNS(XPathResolver.XLINK_NS_URI, "href");
final String componentUri = linkElement.getAttributeNS(XPathExpressionResolver.XLINK_NS_URI, "href");

try {
// NOTE: This DD4T method requires a template URI but it does not actually use it; pass a dummy value
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package com.sdl.webapp.tridion.xpath;

import com.google.common.collect.ImmutableBiMap;
import com.sdl.webapp.common.util.SimpleNamespaceContext;

import javax.xml.namespace.NamespaceContext;
import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;

/**
* Resolver for particular xPath expression string.
*/
public enum XPathExpressionResolver {
XPATH_LINKS("//a[@xlink:href]"),
XPATH_YOUTUBE("//img[@data-youTubeId]"),
XPATH_IMAGES("//img[@data-schemaUri]");


/**
* Constant <code>XHTML_NS_URI="http://www.w3.org/1999/xhtml"</code>
*/
public static final String XHTML_NS_URI = "http://www.w3.org/1999/xhtml";
/**
* Constant <code>XLINK_NS_URI="http://www.w3.org/1999/xlink"</code>
*/
public static final String XLINK_NS_URI = "http://www.w3.org/1999/xlink";

private static final NamespaceContext NAMESPACE_CONTEXT = new SimpleNamespaceContext(
ImmutableBiMap.<String, String>builder()
.put("xhtml", XHTML_NS_URI)
.put("xlink", XLINK_NS_URI)
.build());

private String sourceString;

XPathExpressionResolver(final String sourceString) {
this.sourceString = sourceString;
}

/**
* <p>create the <code>expression</code> on the fly.</p>
*
* @return a {@link XPathExpression} object.
*/
public XPathExpression expr() {
try {
/**
* This hardcoded XPathFactory is needed to avoid the possibility that a wrong implementation is
* being loaded from classpath when using the javax.xml.xpath.XPathFactory. (cfr javadoc newInstance())
*
* Example of a wrong implementation: org.apache.taglibs.standard.tag.common.xml.JSTLXPathFactory.
* Issue: XPathExpression will be null for xpath.compile!
*/
XPathFactory xPathFactory = new org.apache.xpath.jaxp.XPathFactoryImpl();
final XPath xpath = xPathFactory.newXPath();
xpath.setNamespaceContext(NAMESPACE_CONTEXT);
return xpath.compile(sourceString);
} catch (XPathExpressionException e) {
throw new RuntimeException(
"Error while creating XPath expression", e);
}
}

/**
* <p>Getter for the field <code>sourceString</code>.</p>
*
* @return a {@link String} object.
*/
public String getSourceString() {
return sourceString;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* Resolver for particular xPath string.
* Uses {@link javax.xml.xpath.XPathExpression} which is not thread-safe.
*/
@Deprecated
public enum XPathResolver {
XPATH_LINKS("//a[@xlink:href]"),
XPATH_YOUTUBE("//img[@data-youTubeId]"),
Expand Down Expand Up @@ -43,7 +44,15 @@ public enum XPathResolver {
@Override
protected XPathExpression initialValue() {
try {
final XPath xpath = XPathFactory.newInstance().newXPath();
/**
* This hardcoded XPathFactory is needed to avoid the possibility that a wrong implementation is
* being loaded from classpath when using the javax.xml.xpath.XPathFactory. (cfr javadoc newInstance())
*
* Example of a wrong implementation: org.apache.taglibs.standard.tag.common.xml.JSTLXPathFactory.
* Issue: XPathExpression will be null for xpath.compile!
*/
XPathFactory xPathFactory = new org.apache.xpath.jaxp.XPathFactoryImpl();
final XPath xpath = xPathFactory.newXPath();
xpath.setNamespaceContext(NAMESPACE_CONTEXT);
return xpath.compile(sourceString);
} catch (XPathExpressionException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.sdl.webapp.tridion.xpath;

import org.junit.Assert;
import org.junit.Test;

import static org.junit.Assert.*;

public class XPathExpressionResolverTest {

@Test
public void testLinksExpr() throws Exception {
Assert.assertNotNull(XPathExpressionResolver.XPATH_LINKS.expr());
}

@Test
public void testImagesExpr() throws Exception {
Assert.assertNotNull(XPathExpressionResolver.XPATH_IMAGES.expr());
}

@Test
public void testYouTubeExpr() throws Exception {
Assert.assertNotNull(XPathExpressionResolver.XPATH_YOUTUBE.expr());
}

}