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

RuntimeError "Could not parse document" on empty input in strict mode #1349

Closed
semaperepelitsa opened this issue Sep 11, 2015 · 6 comments
Closed
Assignees
Milestone

Comments

@semaperepelitsa
Copy link

I noticed a change when updating from 1.5 to 1.6 probably related to #1005

Previously, empty input would result in an empty document, and now it raises an error. However, the error is a generic RuntimeError instead of the expected Nokogiri::XML::SyntaxError. RuntimeError is very common in Ruby so I don't like to rescue it for this very specific case. Was it ever intended to be a RuntimeError?

Here is an example:

gem "nokogiri", ARGV[0]
require "nokogiri"

begin
  p Nokogiri::XML(""){ |c| c.strict }
rescue => err
  p err
end

begin
  p Nokogiri::XML(" "){ |c| c.strict }
rescue => err
  p err
end
> ruby example.rb 1.6.6.2
#<RuntimeError: Could not parse document>
#<Nokogiri::XML::SyntaxError: Start tag expected, '<' not found>
> ruby example.rb 1.5.11
#<Nokogiri::XML::Document:0x3fdc498fe430 name="document">
#<Nokogiri::XML::SyntaxError: Start tag expected, '<' not found>
@flavorjones
Copy link
Member

It's not due to #1005, as that was in v1.6.2.1 and that version emits a SyntaxError. I'm git bisecting now ...

@flavorjones
Copy link
Member

Ah, wait, strike my last comment. I was misinterpreting the output of your script. Please hold ...

@flavorjones
Copy link
Member

You're right, the offending commit is 2c9b7f1 from #1005.

flavorjones added a commit that referenced this issue Sep 13, 2015
Previously raised RuntimeError in MRI for zero-length docs.

Fixes #1349. Related to #1005 and 2c9b7f1
@flavorjones flavorjones added this to the 1.7.0 milestone Sep 13, 2015
@flavorjones
Copy link
Member

I've pushed a branch that addresses this, flavorjones-gh-1349-syntax-error-on-empty-document.

This will be in 1.7.0, as we're about to release 1.6.7 final. Leaving open until it's merged into master.

@semaperepelitsa
Copy link
Author

Looks good. I will be waiting for the release. Thanks!

@flavorjones
Copy link
Member

Merged onto master, will be in 1.6.8.

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

2 participants