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

Nokogiri do_xinclude (not very good) - consider to change #1321

Closed
etm opened this issue Jul 23, 2015 · 2 comments
Closed

Nokogiri do_xinclude (not very good) - consider to change #1321

etm opened this issue Jul 23, 2015 · 2 comments

Comments

@etm
Copy link

etm commented Jul 23, 2015

xinclude in libxml2 is not very capable. It uses nanohttp, which has no https support.

  require 'openssl'
  require 'open-uri'

  class Node
    def do_xinclude_manual
      ctx = XPathContext.new(self)
      ctx.register_namespaces "xi"=>"http://www.w3.org/2001/XInclude"
      ctx.evaluate('//xi:include').each do |ele|
        name = ele.attributes['href'].value
        content = open(name,{ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE}).read
        insert = begin 
          Nokogiri::XML::parse(content){|config| config.noblanks.noent.nsclean.strict }.root
        rescue
          content
        end
        insert['xml:base'] = name unless File.dirname(ele.attributes['href'].value) == '.' 
        x = ele.replace insert
        if x.is_a? Nokogiri::XML::Element
          x.do_xinclude_manual
        else
          x.each do |n|
            n.do_xinclude_manual if n.is_a? Nokogiri::XML::Element
          end
        end                                                                            
      end
    end
  end

the above code implements a simple xinclude. If the result parses as XML, root is inserted - because xml could include processing instructions which would break the resulting code. If the result does not parse as XML (e.g. text) it is just inserted and we hope for the best ;-)

consider changing to this implementation, after all https becomes more common ;-)

@flavorjones
Copy link
Member

Hi, @etm. Thanks for suggesting this.

I'm curious why you didn't send this as a PR? Also, do you happen to have any tests that you wrote during implementation?

@tenderlove
Copy link
Member

I like this, but it needs tests and should use OpenSSL::SSL::VERIFY_NONE, and probably should use net/http instead of open-uri. Can you send a PR with tests for this? Thanks!

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

No branches or pull requests

3 participants