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] Nokogiri::CSS throws cryptic error messages when missing a parameter #2700

Closed
xeruf opened this issue Nov 16, 2022 · 5 comments · Fixed by #2701
Closed

[bug] Nokogiri::CSS throws cryptic error messages when missing a parameter #2700

xeruf opened this issue Nov 16, 2022 · 5 comments · Fixed by #2701
Labels
needs/more-info

Comments

@xeruf
Copy link

xeruf commented Nov 16, 2022

Please describe the bug

When calling a Nokogiri::CSS selector without a parameter, this message appears:

Nokogiri::CSS::SyntaxError: unexpected '$' after ''

https://stackoverflow.com/questions/74457363/capybara-rspec-empty-html-nokogiricsssyntaxerror-unexpected-after?noredirect=1#comment131440070_74457363

Expected behavior

Tell the user a parameter is missing

Environment

❯ docker-compose exec -it app bundle exec nokogiri -v
WARN[0000] The "CONTAINER_UID" variable is not set. Defaulting to a blank string. 
WARN[0000] The "CONTAINER_GID" variable is not set. Defaulting to a blank string. 
WARN[0000] The "CONTAINER_UID" variable is not set. Defaulting to a blank string. 
WARN[0000] The "CONTAINER_GID" variable is not set. Defaulting to a blank string. 
# Nokogiri (1.12.5)
    ---
    warnings: []
    nokogiri:
      version: 1.12.5
      cppflags:
      - "-I/usr/local/lib/app-bundle/gems/nokogiri-1.12.5-x86_64-linux/ext/nokogiri"
      - "-I/usr/local/lib/app-bundle/gems/nokogiri-1.12.5-x86_64-linux/ext/nokogiri/include"
      - "-I/usr/local/lib/app-bundle/gems/nokogiri-1.12.5-x86_64-linux/ext/nokogiri/include/libxml2"
      ldflags: []
    ruby:
      version: 2.6.10
      platform: x86_64-linux
      gem_platform: x86_64-linux
      description: ruby 2.6.10p210 (2022-04-12 revision 67958) [x86_64-linux]
      engine: ruby
    libxml:
      source: packaged
      precompiled: true
      patches:
      - 0001-Remove-script-macro-support.patch
      - 0002-Update-entities-to-remove-handling-of-ssi.patch
      - 0003-libxml2.la-is-in-top_builddir.patch
      - 0004-use-glibc-strlen.patch
      - 0005-avoid-isnan-isinf.patch
      - 0006-update-automake-files-for-arm64.patch
      - 0007-Fix-XPath-recursion-limit.patch
      libxml2_path: "/usr/local/lib/app-bundle/gems/nokogiri-1.12.5-x86_64-linux/ext/nokogiri"
      memory_management: ruby
      iconv_enabled: true
      compiled: 2.9.12
      loaded: 2.9.12
    libxslt:
      source: packaged
      precompiled: true
      patches:
      - 0001-update-automake-files-for-arm64.patch
      - 0002-Fix-xml2-config-check-in-configure-script.patch
      datetime_enabled: true
      compiled: 1.1.34
      loaded: 1.1.34
    other_libraries:
      zlib: 1.2.11
      libgumbo: 1.0.0-nokogiri
@xeruf xeruf added the state/needs-triage Inbox for non-installation-related bug reports or help requests label Nov 16, 2022
@flavorjones
Copy link
Member

Hi, I'm sorry you're having a problem. I'll try to help.

You skipped the bit in the issue template entitled "Help us reproduce what you're seeing." I spent about 15 minutes trying to reproduce this, and looking through the Capybara source code, and I can't.

As a prompt, here's the section you skipped:

If possible, please include a complete, self-contained script that reproduces the behavior you're seeing. Please try to remove external dependencies like Rails or other libraries that may be wrapping Nokogiri.

Here's an example of how you might structure such a script:

#! /usr/bin/env ruby

require 'nokogiri'
require 'minitest/autorun'

class Test < MiniTest::Spec
  describe "Node#css" do
    it "should find a div using chained classes" do
      html = <<~HEREDOC
        <html>
          <body>
            <div class="foo"> one</div> 
            <div class="bar">two</div> 
            <div class="foo bar">three</div> 
      HEREDOC
      
      doc = Nokogiri::HTML::Document.parse(html)
      
      assert_equal 1, doc.css("div.foo.bar").length
      assert_equal "three", doc.at_css("div.foo.bar").text
    end
  end
end

If you haven't included a test, please provide whatever code you can, the inputs (if any), along with the output that you're seeing. We need to reproduce what you're seeing to be able to help!

Can you also help me understand why you're reporting this here instead of with the Capybara project?

@flavorjones flavorjones added needs/more-info and removed state/needs-triage Inbox for non-installation-related bug reports or help requests labels Nov 16, 2022
@xeruf
Copy link
Author

xeruf commented Nov 17, 2022

Well the code does not really matter, it is about the shell interaction.
I thought it might be trivial to reproduce in any project with Capybara. I don't know whether this is a downstream or upstream issue, parameter checking generally seems a bit awkward in ruby...

This is also a bit of a legacy project which I am slowly trying to bring up to date, so I did not want to invest too much time upfront to investigate something that has already been fixed.

@flavorjones
Copy link
Member

Please put yourself in my spot: I don't use Capybara and don't know how to set up an isolated test case. What would you do with this issue?

Thankfully after a few more minutes I have reproduced what you're seeing:

Nokogiri::HTML5.parse("<div></div>").css("")

I think you should also report this upstream at Capybara because it doesn't appear to be checking the number of arguments passed to those methods. Although Nokogiri could be raising a better exception here, Capybara could be a bit more defensive about how it calls Nokogiri.

flavorjones added a commit that referenced this issue Nov 17, 2022
@flavorjones
feat: better exception message when a CSS selector is empty
c1fc653 
Closes #2700
@flavorjones flavorjones mentioned this issue Nov 17, 2022
Merged
@flavorjones
Copy link
Member

See #2701 for the fix which will be in Nokogiri 1.14.0 when it's released.

@xeruf
Copy link
Author

xeruf commented Nov 17, 2022

thank you so much, I am rather new to the whole ruby on rails ecosystem and still struggling to grasp how everything interacts :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs/more-info
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: better exception message when a CSS selector is empty sparklemotion/nokogiri
2 participants
@flavorjones @xeruf