[java-metadata-aggregator] branch master updated: MDA-163 add stage to detect CR characters in metadata

Ian Young ian at iay.org.uk
Tue Apr 19 10:50:31 EDT 2016


This is an automated email from the git hooks/post-receive script.

iay pushed a commit to branch master
in repository java-metadata-aggregator.

The following commit(s) were added to refs/heads/master by this push:
       new  8e39f2d   MDA-163 add stage to detect CR characters in metadata
8e39f2d is described below

commit 8e39f2dd0f25917b134fedd7c5afc1c6ef0bca9e
Author: Ian Young <ian at iay.org.uk>
AuthorDate: Tue Apr 19 15:49:55 2016 +0100

    MDA-163 add stage to detect CR characters in metadata
    
    Detects CR characters in character content (DOM element text and
    attribute values) and marks items containing them as in error. This
    provides a way of mitigating the SSPCPP-684 issue in the Shibboleth
    SP's dependent Xerces libraries.
    
    Code contributed by the UK federation.
---
 .../shibboleth/metadata/dom/CRDetectionStage.java  |  83 ++++++++++++++++
 .../metadata/dom/CRDetectionStageTest.java         | 106 +++++++++++++++++++++
 .../metadata/dom/CRDetectionStage-assumptions.xml  |   2 +
 .../metadata/dom/CRDetectionStage-attribute.xml    |   2 +
 .../metadata/dom/CRDetectionStage-comment.xml      |   6 ++
 .../metadata/dom/CRDetectionStage-element.xml      |   6 ++
 .../metadata/dom/CRDetectionStage-multiple.xml     |   6 ++
 .../dom/CRDetectionStage-nested-attribute.xml      |   9 ++
 .../dom/CRDetectionStage-nested-element.xml        |   9 ++
 .../metadata/dom/CRDetectionStage-ok.xml           |   2 +
 10 files changed, 231 insertions(+)

diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/CRDetectionStage.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/CRDetectionStage.java
new file mode 100644
index 0000000..c981ef7
--- /dev/null
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/CRDetectionStage.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development, 
+ * Inc. (UCAID) under one or more contributor license agreements.  See the 
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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 net.shibboleth.metadata.dom;
+
+import org.w3c.dom.Element;
+import org.w3c.dom.NamedNodeMap;
+import org.w3c.dom.Node;
+
+import net.shibboleth.metadata.Item;
+import net.shibboleth.metadata.pipeline.StageProcessingException;
+
+/**
+ * A stage to examine all the text in {@link Element}-based {@link Item}s (text nodes and attributes) and
+ * mark them as being in error if a CR character appears. This can only be the case if the XML
+ * document contained an explicit character reference such as <code>&#13;</code>.
+ * 
+ * This stage is specifically intended to detect metadata which would trigger the SSPCPP-684
+ * issue in the Shibboleth SP.
+ * 
+ * @see <a href="https://issues.shibboleth.net/jira/browse/SSPCPP-684">SSPCPP-684</a>
+ */
+public class CRDetectionStage extends AbstractDOMTraversalStage {
+
+    /** Character value we are looking for. */
+    private static final char CR = '\r';
+    
+    @Override
+    protected boolean applicable(final Element element) {
+        // all Elements are applicable
+        return true;
+    }
+
+    @Override
+    protected void visit(final Element element, final TraversalContext context) throws StageProcessingException {
+        // Only permit one error; short-circuit any further examinations
+        if (!context.getStash().isEmpty()) {
+            return;
+        }
+
+        final Item<Element> item = context.getItem();
+
+        // Check all text node children of the element
+        for (Node node = element.getFirstChild(); node != null; node = node.getNextSibling()) {
+            /*
+             * There are three kinds of child node capable of including character data. We only need to
+             * check TEXT_NODEs: CDATA sections and comments can't include CR characters, as
+             * character references are not interpreted in either context.
+             */
+            if (node.getNodeType() == Node.TEXT_NODE && node.getNodeValue().indexOf(CR) >= 0) {
+                addError(item, element, "element text content contains a carriage return character");
+                context.getStash().put(Boolean.TRUE);
+                return;
+            }
+        }
+
+        // Also check any attributes on the element
+        final NamedNodeMap attributes = element.getAttributes();
+        for (int index=0; index<attributes.getLength(); index++) {
+            final Node attribute = attributes.item(index);
+            if (attribute.getNodeValue().indexOf(CR) >= 0) {
+                addError(item, element, "attribute value contains a carriage return character");
+                context.getStash().put(Boolean.TRUE);
+                return;
+            }
+        }
+    }
+
+}
diff --git a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/CRDetectionStageTest.java b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/CRDetectionStageTest.java
new file mode 100644
index 0000000..6949412
--- /dev/null
+++ b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/CRDetectionStageTest.java
@@ -0,0 +1,106 @@
+
+package net.shibboleth.metadata.dom;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+import org.w3c.dom.Comment;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+
+import net.shibboleth.metadata.ErrorStatus;
+import net.shibboleth.metadata.InfoStatus;
+import net.shibboleth.metadata.Item;
+import net.shibboleth.metadata.WarningStatus;
+import net.shibboleth.utilities.java.support.xml.BasicParserPool;
+
+public class CRDetectionStageTest extends BaseDOMTest {
+
+    public CRDetectionStageTest() throws Exception {
+        super(CRDetectionStage.class);
+    }
+    
+    // Tests various assumptions about how character references are incorporated into
+    // text and attribute nodes.
+    @Test
+    public void testDocumentAssumptions() throws Exception {
+        final Item<Element> item = readDOMItem("assumptions.xml");
+        final Element doc = item.unwrap();
+        Assert.assertEquals(doc.getTagName(), "root");
+        final Node node = doc.getFirstChild();
+        // text
text turns into a raw CR
+        Assert.assertEquals(node.getNodeValue(), "text\rtext");
+        final Element foo = (Element)node.getNextSibling();
+        // same happens within an attribute value
+        final String fop = foo.getAttribute("fop");
+        Assert.assertEquals(fop, "x\ry");
+    }
+
+    // Tests the assumption that character references are NOT incorporated into
+    // comment nodes.
+    @Test
+    public void testCommentAssumptions() throws Exception {
+        // Make a parser pool which does NOT ignore comments
+        // (Shibboleth stack ignores comments by default, removing all comment nodes)
+        final BasicParserPool commentingParserPool = new BasicParserPool();
+        commentingParserPool.setIgnoreComments(false);
+        commentingParserPool.initialize();
+
+        final InputStream input = getClasspathResource("comment.xml").getInputStream();
+        Assert.assertNotNull(input);
+        final Element doc = commentingParserPool.parse(input).getDocumentElement();
+        Assert.assertEquals(doc.getTagName(), "root");
+        final Node node1 = doc.getFirstChild();
+        Assert.assertEquals(node1.getNodeType(), Node.TEXT_NODE);
+        final Node node2 = node1.getNextSibling();
+        Assert.assertEquals(node2.getNodeType(), Node.COMMENT_NODE);
+        final Comment comment = (Comment)node2;
+        Assert.assertEquals(comment.getData(), " a comment incorporating a 
 ");
+    }
+
+    private List<ErrorStatus> execute(final Item<Element> item) throws Exception {
+        final List<Item<Element>> itemCollection = new ArrayList<>();
+        itemCollection.add(item);
+        final CRDetectionStage stage = new CRDetectionStage();
+        stage.setId("test");
+        stage.initialize();
+        stage.execute(itemCollection);
+        final List<WarningStatus> warnings = item.getItemMetadata().get(WarningStatus.class);
+        Assert.assertTrue(warnings.isEmpty());
+        final List<InfoStatus> infos = item.getItemMetadata().get(InfoStatus.class);
+        Assert.assertTrue(infos.isEmpty());
+        return item.getItemMetadata().get(ErrorStatus.class);
+    }
+
+    private List<ErrorStatus> execute(final String filename) throws Exception {
+        final Item<Element> item = readDOMItem(filename);
+        return execute(item);
+    }
+    
+    private void expectError(final String filename, final String errorContains) throws Exception {
+        final List<ErrorStatus> errors = execute(filename);
+        Assert.assertEquals(errors.size(), 1, "errors size on " + filename);
+        final ErrorStatus error = errors.get(0);
+        Assert.assertTrue(error.getStatusMessage().contains(errorContains),
+                filename + " does not contain " + errorContains);
+    }
+
+    @Test
+    public void testErrors() throws Exception {
+        expectError("element.xml", "element");
+        expectError("attribute.xml", "attribute");
+        expectError("assumptions.xml", "carriage return"); // contains both
+        expectError("nested-element.xml", "element");
+        expectError("nested-attribute.xml", "attribute");
+        expectError("multiple.xml", "element");
+    }
+
+    @Test
+    public void testOK() throws Exception {
+        final List<ErrorStatus> errors = execute("ok.xml");
+        Assert.assertTrue(errors.isEmpty());
+    }
+}
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-assumptions.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-assumptions.xml
new file mode 100644
index 0000000..e4c4f82
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-assumptions.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root>text
text<foo fop="x
y">bar</foo>zap</root>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-attribute.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-attribute.xml
new file mode 100644
index 0000000..658bfd1
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-attribute.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root>texttext<foo fop="x
y">bar</foo>zap</root>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-comment.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-comment.xml
new file mode 100644
index 0000000..ffe0f19
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-comment.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root>
+    some ordinary text
+    <!-- a comment incorporating a 
 -->
+    more text
+</root>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-element.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-element.xml
new file mode 100644
index 0000000..ec3a840
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-element.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root>
+    zap
+    <foo fop="xy">bar</foo>
+    text
text
+</root>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-multiple.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-multiple.xml
new file mode 100644
index 0000000..6e30b2d
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-multiple.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root>
+    zap
+    <foo fop="xy">bar</foo>
+    text
text
+</root>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-nested-attribute.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-nested-attribute.xml
new file mode 100644
index 0000000..cad03f6
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-nested-attribute.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<foo>
+    top level text
+    <bar a="ok">
+        second level text
+        <baz a="
">more stuff here</baz>
+        more text
+    </bar>
+</foo>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-nested-element.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-nested-element.xml
new file mode 100644
index 0000000..9d88492
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-nested-element.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<foo>
+    top level text
+    <bar>
+        second level text
+        <baz>bad stuff
here</baz>
+        more text
+    </bar>
+</foo>
diff --git a/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-ok.xml b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-ok.xml
new file mode 100644
index 0000000..805ed10
--- /dev/null
+++ b/aggregator-pipeline/src/test/resources/net/shibboleth/metadata/dom/CRDetectionStage-ok.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root>texttext<foo fop="xy">bar</foo>zap</root>

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list