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.

Sign up for GitHub

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

Jump to bottom

[bug] <!----> (empty comment nodes) is not treated as Comment in dom4j. #78

Open
XenoAmess opened this issue Mar 11, 2020 · 4 comments · May be fixed by #80
Open

[bug] <!----> (empty comment nodes) is not treated as Comment in dom4j. #78

XenoAmess opened this issue Mar 11, 2020 · 4 comments · May be fixed by #80
Assignees
@FilipJirsak
Labels
bug
Milestone
2.1.5

Comments

@XenoAmess
Copy link

I built a test for showing this bug.

test code

package com.xenoamess.x8l;

import org.dom4j.*;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class XmlDom4jIgnoreEmptyCommentTest {
    @Test
    public void test() throws DocumentException {
        String xmlString = "" +
                "<a>" +
                "    <!---->" +
                "    <!-- -->" +
                "    <!---->" +
                "    <!-- -->" +
                "</a>";
        int nowNodeCount = 0;
        Document document = DocumentHelper.parseText(xmlString);
        for (int i = 0; i < document.getRootElement().nodeCount(); i++) {
            Node nowNode = document.getRootElement().node(i);
            System.out.println(nowNode.asXML());
            if (nowNode instanceof Comment) {
                nowNodeCount++;
            }
        }
        assertEquals(nowNodeCount, 4);
    }
}

result:

com.xenoamess.x8l.XmlDom4jIgnoreEmptyCommentTest

    
    
 
<!-- -->
    
    
 
<!-- -->


org.opentest4j.AssertionFailedError: 
Expected :2
Actual   :4
<Click to see difference>


	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:510)
	at com.xenoamess.x8l.XmlDom4jIgnoreEmptyCommentTest.test(XmlDom4jIgnoreEmptyCommentTest.java:52)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:567)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:205)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:201)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1507)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1507)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:248)
	at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$5(DefaultLauncher.java:211)
	at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:226)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:199)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:132)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:230)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:58)



Process finished with exit code -1
@carlosame
Copy link
Contributor

Do you have the org.xml.sax.driver system property set to anything? Because you are probably using some version of the Xerces-j SAX parser under the hood, and therefore this would not be a dom4j problem.

Also, you do not mention the dom4j version (very old versions directly used the bundled AElfred parser AFAIR).

@XenoAmess
Copy link
Author

XenoAmess commented Mar 11, 2020 •
edited

Do you have the org.xml.sax.driver system property set to anything? Because you are probably using some version of the Xerces-j SAX parser under the hood, and therefore this would not be a dom4j problem.

Also, you do not mention the dom4j version (very old versions directly used the bundled AElfred parser AFAIR).

  1. I did not set anything. all is default value
  2. latest version.

@XenoAmess
Copy link
Author

For making it easier, I give you the Junit test here.
both my PC and travis-ci can reproduce this bug.

https://github.com/cyanpotion/x8l/blob/master/src/test/java/com/xenoamess/x8l/XmlDom4jIgnoreEmptyCommentTest.java

@FilipJirsak FilipJirsak self-assigned this Mar 12, 2020
@carlosame
Copy link
Contributor

For making it easier, I give you the Junit test here.

Thank you, although the line:

if (nowNode instanceof Comment) {

should be:

if (nowNode instanceof DefaultComment) {

The behaviour is introduced in io.SAXContentHandler line 416:

if (!insideDTDSection && (text.length() > 0)) {

so it is intended behaviour. Fortunately, the dom4j maintainer @FilipJirsak now owns this bug and can decide on what to do.

FilipJirsak added a commit to FilipJirsak/dom4j that referenced this issue Mar 12, 2020
@FilipJirsak
Allow empty comments - fix for dom4j#78.
0d00af9
@FilipJirsak FilipJirsak linked a pull request Mar 12, 2020 that will close this issue
Open
@FilipJirsak FilipJirsak modified the milestones: 2.1.2, 2.0.3 Mar 12, 2020
@FilipJirsak FilipJirsak linked a pull request Mar 12, 2020 that will close this issue
Don't skip empty comment. #80
Open
@FilipJirsak FilipJirsak modified the milestones: 2.1.2, 2.1.4 Apr 22, 2020
@FilipJirsak FilipJirsak modified the milestones: 2.1.4, 2.1.5 Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@FilipJirsak FilipJirsak

Labels
bug
Milestone
2.1.5
Development

Successfully merging a pull request may close this issue.

Don't skip empty comment. FilipJirsak/dom4j
3 participants
@FilipJirsak @XenoAmess @carlosame