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.

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

[bug] Nokogiri::XML::SyntaxError when parsing <img> tag with 2+ query parameters #2219

Closed
halgorithm opened this issue Apr 9, 2021 · 3 comments

Comments

@halgorithm
Copy link

Please describe the bug
It appears Nokogiri's HTML parser returns parse errors for valid img tags with src urls that contain two or more query parameters.

Help us reproduce what you're seeing
Open irb and run the following script. Observe that parsing one_qp does not produce any errors, whereas parsing two_qp does.

one_qp = "<img src='https://example.com?a=1' />"
Nokogiri::HTML.parse(one_qp).errors # []
two_qp = "<img src='https://example.com?a=1&b=2' />"
Nokogiri::HTML.parse(two_qp).errors # [#<Nokogiri::XML::SyntaxError: 1:36: ERROR: htmlParseEntityRef: expecting ';'>]

Expected behavior
Parsing a string containing a single image tag with two query parameters should not return any errors.

Environment

# Nokogiri (1.11.3)
    ---
    warnings: []
    nokogiri:
      version: 1.11.3
      cppflags:
      - "-I/Users/hal/.gem/gems/nokogiri-1.11.3-x86_64-darwin/ext/nokogiri"
      - "-I/Users/hal/.gem/gems/nokogiri-1.11.3-x86_64-darwin/ext/nokogiri/include"
      - "-I/Users/hal/.gem/gems/nokogiri-1.11.3-x86_64-darwin/ext/nokogiri/include/libxml2"
      ldflags: []
    ruby:
      version: 2.6.5
      platform: x86_64-darwin19
      gem_platform: x86_64-darwin-19
      description: ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin19]
      engine: ruby
    libxml:
      source: packaged
      precompiled: true
      patches:
      - 0001-Revert-Do-not-URI-escape-in-server-side-includes.patch
      - 0002-Remove-script-macro-support.patch
      - 0003-Update-entities-to-remove-handling-of-ssi.patch
      - 0004-libxml2.la-is-in-top_builddir.patch
      - 0005-Fix-infinite-loop-in-xmlStringLenDecodeEntities.patch
      - 0006-htmlParseComment-treat-as-if-it-closed-the-comment.patch
      - 0007-use-new-htmlParseLookupCommentEnd-to-find-comment-en.patch
      - '0008-use-glibc-strlen.patch'
      - '0009-avoid-isnan-isinf.patch'
      - 0010-parser.c-shrink-the-input-buffer-when-appropriate.patch
      - 0011-update-automake-files-for-arm64.patch
      libxml2_path: "/Users/hal/.gem/gems/nokogiri-1.11.3-x86_64-darwin/ext/nokogiri"
      iconv_enabled: true
      compiled: 2.9.10
      loaded: 2.9.10
    libxslt:
      source: packaged
      precompiled: true
      patches:
      - 0001-update-automake-files-for-arm64.patch
      compiled: 1.1.34
      loaded: 1.1.34
    other_libraries:
      zlib: 1.2.11
      libiconv: '1.15'
@halgorithm halgorithm added the state/needs-triage Inbox for non-installation-related bug reports or help requests label Apr 9, 2021
@rubys
Copy link
Contributor

rubys commented Apr 9, 2021

Suggestion: try nokogumbo.

Longer version:

  1. To be valid XML, you would need to escape the ampersand as &amp;. Nokogiri currently relies on the libxml parser for HTML which handles some, but not all, of the differences between HTML and XML.

  2. The plans are to merge nokogumbo into nokogiri. But you don't need to wait, you can use nokogumbo today.

@flavorjones
Copy link
Member

@halgorithm Sam's response above is right on.

The behavior you're describing is inherited from the underlying HTML library, libxml2. Here's the C code that controls URI-escaping of certain HTML attributes at serialization-time (when the document is printed):

https://gitlab.gnome.org/GNOME/libxml2/blob/v2.9.2/HTMLtree.c#L714-718

Specifically, href, action, src, and name (but only within an anchor) are always escaped when generating HTML -- basically, anything that could be a URI reference.

Using Nokogumbo along with Nokogiri for HTML should be a better experience; and as Sam mentioned we're working on improving the integration between the two libraries.

@flavorjones flavorjones added topic/HTML5 state/will-close and removed state/needs-triage Inbox for non-installation-related bug reports or help requests labels Apr 9, 2021
@halgorithm
Copy link
Author

I see, thank you both for the detailed responses! I'll look into using Nokogumbo for now then and make sure to keep an eye on the project merge tracking issue. Not sure if it might be helpful to add a small temporary note to the documentation somewhere recommending Nokogumbo for HTML5 parsing (perhaps there already is one somewhere but I didn't notice any when searching for information on this error). Closing this issue, good luck with the merge!

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

No branches or pull requests

3 participants