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] jruby NPE when serializing an unparented HTML node #2559

Closed
flavorjones opened this issue May 30, 2022 · 2 comments · Fixed by #2895
Closed

[bug] jruby NPE when serializing an unparented HTML node #2559

flavorjones opened this issue May 30, 2022 · 2 comments · Fixed by #2895
Labels
help wanted platform/jruby

Comments

@flavorjones
Copy link
Member

Please describe the bug

Nokogiri::HTML::Document.parse("<div></div>").create_text_node("asdf").to_s

results in

Unhandled Java exception: java.lang.NullPointerException
java.lang.NullPointerException: null
      isHtmlScript at nokogiri/internals/SaveContextVisitor.java:812
             enter at nokogiri/internals/SaveContextVisitor.java:833
            accept at nokogiri/XmlText.java:75
   native_write_to at nokogiri/XmlNode.java:1352
              call at nokogiri/XmlNode$INVOKER$i$1$0$native_write_to.gen:-1
              call at org/jruby/internal/runtime/methods/JavaMethod.java:846
      cacheAndCall at org/jruby/runtime/callsite/CachingCallSite.java:329
              call at org/jruby/runtime/callsite/CachingCallSite.java:87

Expected behavior

Unparented nodes should serialize normally, as they do in the CRuby implementation
@flavorjones flavorjones added state/needs-triage Inbox for non-installation-related bug reports or help requests platform/jruby and removed state/needs-triage Inbox for non-installation-related bug reports or help requests labels May 30, 2022
@cbasguti
Copy link
Contributor

I've investigated the reported bug in the Nokogiri project, and I believe I've identified the cause. It seems that the issue lies within the isHtmlScript and isHtmlStyle functions, where a NullPointerException occurs due to the inability to evaluate a null object in Java.

To address this, I suggest making the following modifications:

  • Add a null check for the getParentNode() method's return value before performing any comparisons.
  • If the parent node is null or does not match the expected tag name, log an appropriate message and return false.

As this is my first contribution to the project, I'm excited to learn from your feedback. I will open a draft PR to propose these changes and verify their effectiveness.

Thank you for your guidance and support!
Best regards.

@cbasguti cbasguti mentioned this issue May 29, 2023
Merged
@flavorjones
Copy link
Member Author

@cbasguti Thank you for taking a look at this issue! I'll take a look at your PR and comment there.

cbasguti added a commit to cbasguti/nokogiri that referenced this issue Jun 1, 2023
@cbasguti
dev: add test use for the issue sparklemotion#2559
6da7a89
flavorjones added a commit that referenced this issue Jun 5, 2023
@flavorjones
Serializing unparented HTML node on JRuby (#2895)
76c891c 
**What problem is this PR intended to solve?**

Closes #2559

This PR aims to solve issue #2559 in the Nokogiri project. After
investigating the reported bug, it was identified that a
NullPointerException occurs in the isHtmlScript and isHtmlStyle
functions due to the inability to evaluate a null object in Java. The
proposed solution involves adding a null check for the getParentNode()
method's return value and logging an appropriate message if the parent
node is null or does not match the expected tag name. A draft PR will be
opened to implement these modifications and gather feedback for
effectiveness.

**Have you included adequate test coverage?**

As this is my first contribution to the project, I am uncertain about
the appropriate location to include the corresponding test. However, I
am committed to ensuring adequate test coverage for this change and will
collaborate with the project team to determine the best approach.

**Does this change affect the behavior of either the C or the Java
implementations?**

This change only affects the Java implementation, as the issue of a
NullPointerException is specific to that platform. The C implementation
does not encounter this problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted platform/jruby
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Serializing unparented HTML node on JRuby cbasguti/nokogiri
2 participants
@flavorjones @cbasguti