[java-metadata-aggregator] branch master updated: MDA-223 - Revise handling of errors in XPathFilteringStage
Ian Young
ian at iay.org.uk
Fri Mar 13 11:50:05 EDT 2020
This is an automated email from the git hooks/post-receive script.
iay pushed a commit to branch master
in repository java-metadata-aggregator.
View the commit online:
http://git.shibboleth.net/view/?p=java-metadata-aggregator.git;a=commit;h=4a9b905e2de6e53a8b3332f56c97c7408428bf88
The following commit(s) were added to refs/heads/master by this push:
new 4a9b905 MDA-223 - Revise handling of errors in XPathFilteringStage
4a9b905 is described below
commit 4a9b905e2de6e53a8b3332f56c97c7408428bf88
Author: Ian Young <ian at iay.org.uk>
AuthorDate: Fri Mar 13 15:49:57 2020 +0000
MDA-223 - Revise handling of errors in XPathFilteringStage
https://issues.shibboleth.net/jira/browse/MDA-232
---
.../metadata/dom/XPathFilteringStage.java | 55 +++++++++++++---------
.../metadata/dom/XPathFilteringStageTest.java | 27 +++++++++++
2 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XPathFilteringStage.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XPathFilteringStage.java
index 0a8c1bd..053ec9f 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XPathFilteringStage.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XPathFilteringStage.java
@@ -25,13 +25,18 @@ import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
import javax.xml.namespace.NamespaceContext;
import javax.xml.xpath.XPath;
-import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Element;
+
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.pipeline.AbstractStage;
+import net.shibboleth.metadata.pipeline.StageProcessingException;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
@@ -40,10 +45,6 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
import net.shibboleth.utilities.java.support.xml.SimpleNamespaceContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.w3c.dom.Element;
-
/**
* Pipeline stage which allows filtering of @{link DomElementItem}s according to an XPath expression. Each
* {@link DOMElementItem} is removed if the XPath expression evaluates as {@code true}.
@@ -65,6 +66,7 @@ public class XPathFilteringStage extends AbstractStage<Element> {
private String xpathExpression;
/** The {@link NamespaceContext} to use in interpreting the XPath expression. */
+ @Nonnull
private NamespaceContext namespaceContext = new SimpleNamespaceContext();
/**
@@ -72,7 +74,7 @@ public class XPathFilteringStage extends AbstractStage<Element> {
*
* @return XPath expression to execute on each {@link DOMElementItem}
*/
- @Nullable public String getXPathExpression() {
+ @NonnullAfterInit @NotEmpty public String getXPathExpression() {
return xpathExpression;
}
@@ -114,52 +116,61 @@ public class XPathFilteringStage extends AbstractStage<Element> {
}
}
- /** {@inheritDoc} */
- @Override public void doExecute(@Nonnull @NonnullElements final Collection<Item<Element>> metadataCollection) {
+ @Override
+ public void doExecute(@Nonnull @NonnullElements final Collection<Item<Element>> metadataCollection)
+ throws StageProcessingException {
final XPathFactory factory = XPathFactory.newInstance();
final XPath xpath = factory.newXPath();
- if (namespaceContext != null) {
- xpath.setNamespaceContext(namespaceContext);
- }
+ xpath.setNamespaceContext(namespaceContext);
final XPathExpression compiledExpression;
try {
compiledExpression = xpath.compile(xpathExpression);
} catch (final XPathExpressionException e) {
- log.error("error compiling XPath expression; no filtering performed", e);
- return;
+ // This should never occur, as we attempted the same operation at initialization time.
+ throw new StageProcessingException("error compiling XPath expression", e);
}
final Iterator<Item<Element>> iterator = metadataCollection.iterator();
while (iterator.hasNext()) {
final Item<Element> item = iterator.next();
try {
- final Boolean filterThis = (Boolean) compiledExpression.evaluate(item.unwrap(), XPathConstants.BOOLEAN);
- if (filterThis) {
+ if (compiledExpression.evaluateExpression(item.unwrap(), Boolean.class)) {
log.debug("removing item matching XPath condition");
iterator.remove();
}
} catch (final XPathExpressionException e) {
- log.error("removing item due to XPath expression error", e);
- iterator.remove();
+ // Rare in practice; happens, for example, if you use a $variable, as there is
+ // no variable resolver attached to our XPath object.
+ throw new StageProcessingException("error evaluating XPath expression", e);
}
}
}
- /** {@inheritDoc} */
- @Override protected void doDestroy() {
+ @Override
+ protected void doDestroy() {
xpathExpression = null;
namespaceContext = null;
super.doDestroy();
}
- /** {@inheritDoc} */
- @Override protected void doInitialize() throws ComponentInitializationException {
+ @Override
+ protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
if (xpathExpression == null) {
throw new ComponentInitializationException("XPath expression can not be null or empty");
}
+
+ // Check to see if the expression is valid
+ final var factory = XPathFactory.newInstance();
+ final var xpath = factory.newXPath();
+ xpath.setNamespaceContext(namespaceContext);
+ try {
+ xpath.compile(xpathExpression);
+ } catch (final XPathExpressionException e) {
+ throw new ComponentInitializationException("error compiling XPath expression", e);
+ }
}
-}
\ No newline at end of file
+}
diff --git a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XPathFilteringStageTest.java b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XPathFilteringStageTest.java
index b0c8b04..d5dc23f 100644
--- a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XPathFilteringStageTest.java
+++ b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XPathFilteringStageTest.java
@@ -23,6 +23,8 @@ import java.util.List;
import java.util.Map;
import net.shibboleth.metadata.Item;
+import net.shibboleth.metadata.pipeline.StageProcessingException;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.xml.SimpleNamespaceContext;
import org.testng.Assert;
@@ -68,4 +70,29 @@ public class XPathFilteringStageTest extends BaseDOMTest {
Assert.assertEquals(id, "entity2");
}
+ // Test that an invalid expression results in an exception at initialization time
+ @Test(expectedExceptions=ComponentInitializationException.class)
+ public void testMDA223init() throws Exception {
+ final var stage = new XPathFilteringStage();
+ stage.setId("test");
+ stage.setXPathExpression("a:b");
+ stage.initialize();
+ }
+
+ // Test that a run-time expression error results in an exception
+ // by referencing a variable. As we haven't attached a variable
+ // resolver, this is an error, but not one that can be determined at
+ // compile time.
+ @Test(expectedExceptions=StageProcessingException.class)
+ public void testMDA223exec() throws Exception {
+ final var stage = new XPathFilteringStage();
+ stage.setId("test");
+ stage.setXPathExpression("$foo");
+ stage.initialize();
+
+ final List<Item<Element>> items = new ArrayList<>();
+ items.add(new DOMElementItem(readXMLData("1.xml")));
+ stage.execute(items);
+ }
+
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list