sax parser with replace_entities = false still replaces entities #1284
Labels
Comments
|
Hi, Thank for reporting this. Can you provide the output from -m
|
|
|
Is there any update on this issue? I also think that this test nokogiri/test/xml/test_entity_reference.rb Line 175 in 027a151 Nokogiri::XML::SAX::Parser.new gets ignored. You have to pass it to #parse in order to get access to Nokogiri::XML::SAX::ParserContext. As @kostya reported it still has no effect though :-/
|
|
I do also experience this issue with Nokogiri 1.6.8. Here's how I reproduce in class Parser < Nokogiri::XML::SAX::Document;def characters(v); p v; end;end
Nokogiri::HTML::SAX::Parser.new(Parser.new).parse("’"){|ctx| ctx.replace_entities = false}
# I get "\u0092" rather than "’"This is causing some issues later in our application. I'd love to get this bug fixed. Right now the work-around I'm using is to |
|
If you want a failing test, just replace these lines: https://github.com/sparklemotion/nokogiri/blob/master/test/html/sax/test_parser.rb#L108 So that Currently, setting |
flavorjones
added a commit
that referenced
this issue
Jul 1, 2024
On CRuby, this fixes the fact that the parser was registering errors when encountering general (non-predefined) entities. Now these entities are resolved properly and converted into `#characters` callbacks. Fixes #1926. On JRuby, the SAX parser now respects the `#replace_entities` attribute, which was previously ignored AND defaulted incorrectly to `true`. The default now matches CRuby -- `false` -- and the parser behavior matches CRuby with respect to entities. Fixes #614. This commit also includes some granular tests of how the sax parser handles different entities under different circumstances, which should be clarifying for user reports like #1284 and #1500 that expect predefined entities and character references to be treated like parsed entities (which they aren't).
|
As part of my work in #1926, I investigated entity handling and documented its behavior in #3265. Here's a screenshot of the docs:
As you can see, character references and predefined entities will always result in a callback to Sorry it took so long to explain this, and I hope this is helpful. |
flavorjones
added a commit
that referenced
this issue
Jul 2, 2024
On CRuby, this fixes the fact that the parser was registering errors when encountering general (non-predefined) entities. Now these entities are resolved properly and converted into `#characters` callbacks. Fixes #1926. On JRuby, the SAX parser now respects the `#replace_entities` attribute, which was previously ignored AND defaulted incorrectly to `true`. The default now matches CRuby -- `false` -- and the parser behavior matches CRuby with respect to entities. Fixes #614. This commit also includes some granular tests of how the sax parser handles different entities under different circumstances, which should be clarifying for user reports like #1284 and #1500 that expect predefined entities and character references to be treated like parsed entities (which they aren't).
flavorjones
added a commit
that referenced
this issue
Jul 2, 2024
**What problem is this PR intended to solve?** #1926 described an issue wherein the SAX parser was not correctly resolving and replacing internal entities, and was instead reporting an error for each entity reference. This PR includes a fix for that problem. I've removed the unnecessary "SAX tuple" from the SAX implementation, replacing it with the `_private` struct member that libxml2 makes available. Then I set up the parser context structs so that we can use libxml2's standard SAX callbacks where they're useful (which is how I addressed the above issue). This PR also introduces a new feature, a SAX handler callback `Document#reference` which allows callers to get entity-specific name and replacement text information (rather than relying on the `Document#characters` callback). This can be used to solve the original issue in #1926 with this code: searls/eiwa#11 The behavior of the SAX parser with respect to entities is complex enough that I wrote up a short doc in the `XML::SAX::Document` docstring with a table and explanation. I've also added warnings to remind users that `#replace_entities` is not safe to set when parsing untrusted documents. In the Java implementation, I've fixed the `#replace_entities` option in the SAX parser context and set it to the proper default (`false`), fixing #614. I've also corrected the value of the URI argument to `Document#start_element_namespace` which was a blank string when it should have been `nil`. I've added quite a bit of testing around the SAX parser's handling of entities. I added and clarified quite a bit of documentation around SAX parsing generally. Exception messages have been clarified in a couple of places, and made consistent between the C and Java implementations. This should address questions asked in issues #1500 and #1284. Finally, I cleaned up some of the C code that implements SAX parsing, naming functions more explicitly (and moving towards some kind of standard naming convention). Closes #1926. Closes #614. **Have you included adequate test coverage?** Yes! **Does this change affect the behavior of either the C or the Java implementations?** Yes, but the implementations are much more consistent with each other now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1.6.6.2
The text was updated successfully, but these errors were encountered: