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

Prevent switching of ActiveSupport::XmlMini.backend #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DmytroVasin
Copy link

@DmytroVasin DmytroVasin commented May 1, 2020

Title: Ebayr should not change ActiveSupport::XmlMini.backend

This gem unexpectedly switches default ActiveSupport XMLMini backend.
By default XmlMini.backend is equal to 'ActiveSupport::XmlMini_REXML' or 'REXML' (In Rails 5.2 and Rails 4.2)

When we call your gem - backend was unexpectedly switched to 'Nokogiri'

After some research, I found out that you require 'Nokogiri' to achieve correct parsing

PR that adds bug: https://github.com/bjjb/ebayr/pull/20/files

I have added a small spiky-fix that rollback to the old backend.

The more correct solution would look like that (use Nokogiri only for parsing purposes).

module Ebayr #:nodoc:
  class Response < Record
    ...
    hash = self.class.from_xml(Nokogiri::XML(@body)) if @body
    ...
  end
end

But this solution will not work - because of next code
relies on the fact that in a Hash node key will be the first and attributes key will be the last

So I assume that Ebayr::Record should be fixed to not rely on a position in a hash.
But I created the only workaround to solve a bug.

Example:

xml = "<Description><p class=\"p1\"><span class=\"s1\"><br/></span></p></Description>"
obj = Nokogiri::XML(xml).to_s


ActiveSupport::XmlMini.backend = 'REXML'
Hash.from_xml(obj)
=> {"Description"=>{"p"=>{"class"=>"p1", "span"=>{"class"=>"s1", "br"=>nil}}}}
                         |------------|  |--------------------------------|
                            Attributes                 NODEs

ActiveSupport::XmlMini.backend = 'Nokogiri'
Hash.from_xml(obj)
=> {"Description"=>{"p"=>{"span"=>{"br"=>nil, "class"=>"s1"}, "class"=>"p1"}}}
                         |---------------------------------|  |------------|
                                  NODEs                         Attributes


That happens because of the Rails parser implementation:
ActiveSupport::XmlMini_Nokogiri.parse(xml)
=> {"Description"=>{"p"=>[{"span"=>{"br"=>{}, "class"=>"s1"}, "class"=>"p1"}]}}

ActiveSupport::XmlMini_REXML.parse(xml)
=> {"Description"=>{"p"=>[{"class"=>"p1", "span"=>{"class"=>"s1", "br"=>{}}}]}}

Hope that will help.

@DmytroVasin DmytroVasin changed the title Prevent XML backend switch Ebayr should not change ActiveSupport::XmlMini.backend May 1, 2020
@DmytroVasin DmytroVasin changed the title Ebayr should not change ActiveSupport::XmlMini.backend Prevent switching of ActiveSupport::XmlMini.backend May 1, 2020
@DmytroVasin DmytroVasin changed the title Prevent switching of ActiveSupport::XmlMini.backend Prevent switching of ActiveSupport::XmlMini.backend May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant