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

NPE in TopicFragmentFilter module [DITA OT 2.x] #2222

Closed
raducoravu opened this Issue Feb 5, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@raducoravu
Member

raducoravu commented Feb 5, 2016

DITA Map:

<!DOCTYPE map PUBLIC "-//OASIS//DTD DITA Map//EN" "map.dtd">
<map>
 <title>DITA Topic Map</title>
 <keydef href="zuzu.dita" keys="zuzu"/>
 <topicref href="test.dita"/>
</map>

zuzu.dita:

<!DOCTYPE dita PUBLIC "-//OASIS//DTD DITA Composite//EN" "ditabase.dtd">
<dita>
  <topic id="a">
    <title>A</title>
  </topic>
  <topic id="Z">
    <title>A</title>
  </topic>
</dita>

test.dita:

<!DOCTYPE topic PUBLIC "-//OASIS//DTD DITA Topic//EN" "topic.dtd">
<topic id="topic_fyk_zzc_x5">
  <title>AAA</title>
  <body/>
  <topic id="id_kq2_11d_x5" conkeyref="zuzu" conrefend="default.dita#default/Z">
    <title/>
  </topic>
</topic>

NPE when publishing:

BUILD FAILED
D:\projects\eXml\frameworks\dita\DITA-OT2.x\build.xml:41: The following error occurred while executing this line:
D:\projects\eXml\frameworks\dita\DITA-OT2.x\plugins\org.dita.base\build_preprocess.xml:226: java.lang.NullPointerException
    at java.util.ArrayDeque.addFirst(Unknown Source)
    at org.dita.dost.writer.TopicFragmentFilter.startElement(TopicFragmentFilter.java:48)
    at org.apache.xerces.parsers.AbstractSAXParser.startElement(Unknown Source)
    at org.ditang.relaxng.defaults.RelaxNGDefaultsComponent.startElement(RelaxNGDefaultsComponent.java:200)
    at org.apache.xerces.impl.XMLNSDocumentScannerImpl.scanStartElement(XMLNSDocumentScannerImpl.java:268)
    at org.apache.xerces.impl.XMLDocumentFragmentScannerImpl$FragmentContentDispatcher.dispatch(XMLDocumentFragmentScannerImpl.java:1655)
    at org.apache.xerces.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:325)

@robander robander added the bug label May 6, 2016

@robander

This comment has been minimized.

Member

robander commented May 6, 2016

Marking as a bug because any NPE is a bug - but in this case the NPE is caused by invalid markup. The conref end value is set to conrefend="default.dita#default/Z", which means "The element with id="Z" inside the topic with id="default", and that does not exist.

@robander robander added the preprocess label May 6, 2016

@raducoravu

This comment has been minimized.

Member

raducoravu commented May 9, 2016

Actually the spec says:

When the conkeyref attribute is used in place of conref, a key is used to address the target of the reference. The conrefend attribute, which indicates the end of a conref range, may not use a key. Instead the the map or topic element addressed by the key name component of the conkeyref is used in place of whatever map or topic element is addressed by the conrefend attribute.

So when using conkeyref, the conrefend attribute just needs to have the proper pattern, the only bit from it used is the element ID. So my example is valid.

@robander

This comment has been minimized.

Member

robander commented May 9, 2016

That ... is odd. I'm sure that was written in the context of elements-within-topics, without regard to topic elements. The value "default.dita#default/Z" on its own would not be valid, for the reason given above. If working with something like <section> or <li> elements, the quoted text from the spec would make sense, replacing default.dita#default with the topic at the start of the range. But in this case, you're referencing actual topics, so you can't actually use "the map or topic element addressed by the key name", because you end up with thetopicID/topicID construct.

This all just feels ... odd. I feel sure this wasn't considered when working on that language about ranges.

@raducoravu

This comment has been minimized.

Member

raducoravu commented May 10, 2016

Looked again at my example and now it also looks wrong to me. Maybe the bottom line would be the fact that you cannot use conkeyref range to pull a range of topic elements. And the NPE would need to be guarded somehow.

@jelovirt

This comment has been minimized.

Member

jelovirt commented May 10, 2016

@robander should we throw an error for conrefend="default.dita#default/Z" because @conrefend points to another topic? Just using Z, even with a warning, feels like too aggressive error recovery.

@robander

This comment has been minimized.

Member

robander commented May 10, 2016

I think so. Probably should raise this at the TC - I think this is probably a case that was never considered, and I'm not confident of a quick working solution that would allow it.

@robander

This comment has been minimized.

Member

robander commented Jul 31, 2018

Still get an NPE from this in 3.1, with both preprocess (html5) and preprocess2 (pdf).
2222.zip

@robander

This comment has been minimized.

Member

robander commented Aug 1, 2018

Ahh - this isn't actually related to conkeyref, or even (directly) to the topic fragment module.

When we evaluate a conref range, the code processes the first element correctly, and then processes everything in sequence from there up to the end. For each node in that sequence (including any nodes in the middle + the end node), the @id attribute is explicitly ignored -- which is correct for most elements. But in this case, it means we generate <topic> elements without an ID. Many DITA-OT modules assume every topic has an ID (it's a pretty basic requirement). The topic fragment module is just the first post-conref one we get to that explicitly tries to read topic/@id and doesn't have exception handling in place for a missing ID.

robander added a commit to robander/dita-ot that referenced this issue Aug 1, 2018

Add failing test case for conref range dita-ot#2222
Signed-off-by: Robert D Anderson <robander@us.ibm.com>

robander added a commit to robander/dita-ot that referenced this issue Aug 1, 2018

Don't create topics without id in conrefrange dita-ot#2222
Signed-off-by: Robert D Anderson <robander@us.ibm.com>

@jelovirt jelovirt added this to the 3.1.2 milestone Aug 4, 2018

@robander

This comment has been minimized.

Member

robander commented Aug 6, 2018

Fixed with #3033

@robander robander closed this Aug 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment