Skip to content

Commit

Permalink
fix: allow foreign namespaces in SVG content documents
Browse files Browse the repository at this point in the history
Main changes:
- switch to a master NVDL schema for SVG Content Document validation
  (in EPUB 3)
- allow elements and attributes in foreign namespaces in SVG Content
  Documents, except where otherwise forbidden in the spec (i.e. in the
 `foreignObject` element, where only HTML is allowed)
- allow `data-*` attributes in SVG (by removing them at parsing time)
- add more tests

Also:
- slightly refactor the attribute pre-processing in XMLParser

Fix #491
  • Loading branch information
mattgarrish authored and rdeltour committed Feb 25, 2019
1 parent cfca41b commit ca29c89
Show file tree
Hide file tree
Showing 15 changed files with 201 additions and 97 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/adobe/epubcheck/ops/OPSChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class OPSChecker implements ContentChecker, DocumentValidator
.putAll(Predicates.and(mimetype("image/svg+xml"), version(EPUBVersion.VERSION_2)),
XMLValidators.SVG_20_RNG, XMLValidators.IDUNIQUE_20_SCH)
.putAll(Predicates.and(mimetype("image/svg+xml"), version(EPUBVersion.VERSION_3)),
XMLValidators.SVG_30_RNC, XMLValidators.SVG_30_SCH)
XMLValidators.SVG_30_NVDL)
.putAll(
and(or(profile(EPUBProfile.DICT), hasPubType(OPFData.DC_TYPE_DICT)),
mimetype("application/xhtml+xml"), version(EPUBVersion.VERSION_3)),
Expand Down
55 changes: 27 additions & 28 deletions src/main/java/com/adobe/epubcheck/xml/XMLParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import com.adobe.epubcheck.opf.ValidationContext;
import com.adobe.epubcheck.util.EPUBVersion;
import com.adobe.epubcheck.util.ResourceUtil;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.Closer;
import com.thaiopensource.util.PropertyMapBuilder;
import com.thaiopensource.validate.ValidateProperty;
Expand Down Expand Up @@ -527,59 +528,57 @@ public void startElement(String namespaceURI, String localName, String qName,
}
}

// 3.0.1 custom attributes handling
private static final Set<String> knownXHTMLContentDocsNamespaces = new HashSet<String>();
static
{
knownXHTMLContentDocsNamespaces.add(Namespaces.MATHML);
knownXHTMLContentDocsNamespaces.add(Namespaces.OPS);
knownXHTMLContentDocsNamespaces.add(Namespaces.SSML);
knownXHTMLContentDocsNamespaces.add(Namespaces.SVG);
knownXHTMLContentDocsNamespaces.add(Namespaces.XHTML);
knownXHTMLContentDocsNamespaces.add(Namespaces.XMLEVENTS);
knownXHTMLContentDocsNamespaces.add(Namespaces.XML);
knownXHTMLContentDocsNamespaces.add(Namespaces.XLINK);
}

private Attributes preprocessAttributes(String elemNamespace, String elemName, String elemQName,
Attributes originalAttribs)
{
AttributesImpl attributes = new AttributesImpl(originalAttribs);
if ("application/xhtml+xml".equals(context.mimeType)
&& context.version == EPUBVersion.VERSION_3)
try
{
try
for (int i = attributes.getLength() - 1; i >= 0; i--)
{
for (int i = attributes.getLength() - 1; i >= 0; i--)
if (context.version == EPUBVersion.VERSION_3)
{
if (isDataAttribute(attributes, i) || isCustomNamespaceAttribute(attributes, i))
// Remove data-* attributes in both XHTML and SVG
if (isDataAttribute(attributes, i))
{
attributes.removeAttribute(i);
}
else if (Namespaces.XHTML.equals(elemNamespace)
// Remove custom namespace attributes in XHTML
else if ("application/xhtml+xml".equals(context.mimeType)
&& isHTMLCustomNamespace(attributes.getURI(i)))
{
attributes.removeAttribute(i);
}
// Normalize case of case-insensitive attributes in XHTML
else if ("application/xhtml+xml".equals(context.mimeType)
&& Namespaces.XHTML.equals(elemNamespace)
&& isCaseInsensitiveAttribute(attributes, i))
{
attributes.setValue(i, attributes.getValue(i).toLowerCase(Locale.ENGLISH));
}
}
} catch (Exception e)
{
throw new IllegalStateException("data-* removal exception", e);
}
} catch (Exception e)
{
throw new IllegalStateException("Unexpected error when pre-processing attributes", e);
}
return attributes;
}

private static boolean isDataAttribute(Attributes attributes, int index)
{
return attributes.getLocalName(index).startsWith("data-");
return "".equals(attributes.getURI(index))
&& attributes.getLocalName(index).startsWith("data-");
}

private boolean isCustomNamespaceAttribute(Attributes attributes, int index)
private static final Set<String> KNOWN_XHTML_NAMESPACES = ImmutableSet.of(Namespaces.XHTML,
Namespaces.XML, Namespaces.OPS, Namespaces.SVG, Namespaces.MATHML, Namespaces.SSML,
Namespaces.XMLEVENTS, Namespaces.XLINK);

private static boolean isHTMLCustomNamespace(String namespace)
{
String ns = attributes.getURI(index);
return (ns != null && ns.trim().length() > 0
&& !knownXHTMLContentDocsNamespaces.contains(ns.trim()));
if (namespace == null || namespace.trim().isEmpty()) return false;
return !KNOWN_XHTML_NAMESPACES.contains(namespace.trim());
}

private static boolean isCaseInsensitiveAttribute(Attributes attributes, int index)
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/adobe/epubcheck/xml/XMLValidators.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public enum XMLValidators
SIG_30_RNC("schema/30/ocf-signatures-30.rnc"),
SVG_20_RNG("schema/20/rng/svg11.rng"),
SVG_30_RNC("schema/30/epub-svg-30.rnc"),
SVG_30_NVDL("schema/30/epub-svg-30.nvdl"),
SVG_30_SCH("schema/30/epub-svg-30.sch"),
XHTML_20_NVDL("schema/20/rng/ops20.nvdl"),
XHTML_20_SCH("schema/20/sch/xhtml.sch"),
Expand Down
63 changes: 51 additions & 12 deletions src/main/resources/com/adobe/epubcheck/schema/30/epub-svg-30.nvdl
Original file line number Diff line number Diff line change
@@ -1,15 +1,54 @@
<?xml version="1.0" encoding="UTF-8"?>
<rules xmlns="http://purl.oclc.org/dsdl/nvdl/ns/structure/1.0" startMode="svg">
<mode name="svg">
<namespace ns="http://www.w3.org/2000/svg">
<validate schema="epub-svg-30.rnc" schemaType="application/relax-ng-compact-syntax"
useMode="attach"/>
<validate schema="epub-svg-30.sch" useMode="attach"/>
</namespace>
</mode>
<mode name="attach">
<anyNamespace>
<attach/>
</anyNamespace>
</mode>
<mode name="svg">
<namespace ns="http://www.w3.org/2000/svg">
<validate schema="epub-svg-30.rnc" schemaType="application/relax-ng-compact-syntax"
useMode="allowForeignNS">
<context path="foreignObject" useMode="allowHTMLOnly"/>
</validate>
<validate schema="epub-svg-30.sch" useMode="attachAnyNS"/>
</namespace>
</mode>
<mode name="allowForeignNS">
<namespace ns="http://www.w3.org/1999/xhtml">
<attach/>
</namespace>
<namespace ns="http://www.idpf.org/2007/ops" match="attributes">
<attach/>
</namespace>
<namespace ns="http://www.w3.org/1999/xlink" match="attributes">
<attach/>
</namespace>
<namespace ns="http://www.w3.org/XML/1998/namespace" match="attributes">
<attach/>
</namespace>
<namespace ns="" match="attributes">
<attach/>
</namespace>
<anyNamespace match="elements attributes">
<allow/>
</anyNamespace>
</mode>
<mode name="allowHTMLOnly">
<namespace ns="http://www.w3.org/1999/xhtml">
<attach/>
</namespace>
<namespace ns="http://www.idpf.org/2007/ops" match="attributes">
<attach/>
</namespace>
<namespace ns="http://www.w3.org/1999/xlink" match="attributes">
<attach/>
</namespace>
<namespace ns="http://www.w3.org/XML/1998/namespace" match="attributes">
<attach/>
</namespace>
<namespace ns="" match="attributes">
<attach/>
</namespace>
</mode>
<mode name="attachAnyNS">
<anyNamespace>
<attach/>
</anyNamespace>
</mode>
</rules>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

<ns uri="http://www.w3.org/2000/svg" prefix="svg" />

<pattern></pattern>
<include href="./mod/id-unique.sch"/>
<include href="./mod/epub-svg11-re.sch"/>

Expand Down
69 changes: 54 additions & 15 deletions src/test/java/com/adobe/epubcheck/ops/OPSCheckerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,21 +159,7 @@ public void setup()
expectedFatals.clear();
expectedUsage.clear();
}

@Test
public void testValidateSVGRectInvalid()
{
Collections.addAll(expectedErrors, MessageId.RSC_005, MessageId.RSC_005, MessageId.RSC_005,
MessageId.RSC_005);
testValidateDocument("svg/invalid/rect.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVGRectValid()
{
testValidateDocument("svg/valid/rect.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}


@Test
public void testValidateXHTMLEdits001()
{
Expand Down Expand Up @@ -706,13 +692,66 @@ public void testValidateSVG_ValidStyleWithoutType_issue688()
testValidateDocument("svg/valid/issue688.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_WithDataAttribute()
{
testValidateDocument("svg/valid/data-attribute.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_WithCustomNamespace()
{
testValidateDocument("svg/valid/custom-ns.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_Links_MisssingTitle()
{
expectedWarnings.add(MessageId.ACC_011);
testValidateDocument("svg/invalid/svg-links.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_ForeignObject()
{
// tests that 'foreignObject' conforming to the rules is accepted
testValidateDocument("svg/valid/foreignObject.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_ForeignObjectWithInvalidRequiredExtensions()
{
// tests that 'foreignObject' with a 'requiredExtensions' attribute other than the OPS NS is invalid
expectedErrors.add(MessageId.RSC_005);
testValidateDocument("svg/invalid/foreignObject-invalid-requiredExtensions.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_ForeignObjectWithNonXHTMLContent()
{
// tests that 'foreignObject' can't have children that are not HTML content
expectedErrors.add(MessageId.RSC_005);
testValidateDocument("svg/invalid/foreignObject-non-html-content.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}

@Test
public void testValidateSVG_ForeignObjectWithTwoBodyElements()
{
// tests that 'foreignObject' can't have children that are not HTML content
expectedErrors.add(MessageId.RSC_005);
testValidateDocument("svg/invalid/foreignObject-two-body.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}


@Test
public void testValidateSVG_DuplicateIds()
{
// tests that duplicate IDs are detected
Collections.addAll(expectedErrors, MessageId.RSC_005, MessageId.RSC_005);
testValidateDocument("svg/invalid/duplicate-ids.svg", "image/svg+xml", EPUBVersion.VERSION_3);
}


@Test
public void testValidateXHTMLIssue204()
{
Expand Down
7 changes: 7 additions & 0 deletions src/test/resources/30/single/svg/invalid/duplicate-ids.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
23 changes: 0 additions & 23 deletions src/test/resources/30/single/svg/invalid/rect.svg

This file was deleted.

9 changes: 9 additions & 0 deletions src/test/resources/30/single/svg/valid/custom-ns.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions src/test/resources/30/single/svg/valid/data-attribute.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions src/test/resources/30/single/svg/valid/foreignObject.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 0 additions & 18 deletions src/test/resources/30/single/svg/valid/rect.svg

This file was deleted.

0 comments on commit ca29c89

Please sign in to comment.