CVE-2019-5477

@@ -1,5 +1,18 @@
 # Nokogiri Changelog
 
+## 1.10.4 / 2019-08-07
+
+### Security
+
+#### Address CVE-2019-5477 (#1915)
+
+A command injection vulnerability in Nokogiri v1.10.3 and earlier allows commands to be executed in a subprocess by Ruby's `Kernel.open` method. Processes are vulnerable only if the undocumented method `Nokogiri::CSS::Tokenizer#load_file` is being passed untrusted user input.
+
+This vulnerability appears in code generated by the Rexical gem versions v1.0.6 and earlier. Rexical is used by Nokogiri to generate lexical scanner code for parsing CSS queries. The underlying vulnerability was addressed in Rexical v1.0.7 and Nokogiri upgraded to this version of Rexical in Nokogiri v1.10.4.
+
+This CVE's public notice is https://github.com/sparklemotion/nokogiri/issues/1915
+
+
 ## 1.10.3 / 2019-04-22
 
 ### Security Notes

@@ -17,6 +17,7 @@ gem "rake", "~>12.0", :group => [:development, :test]
 gem "rake-compiler", "~>1.0.3", :group => [:development, :test]
 gem "rake-compiler-dock", "~>0.7.0", :group => [:development, :test]
 gem "rexical", "~>1.0.5", :group => [:development, :test]
+gem "rubocop", "~>0.73", :group => [:development, :test]
 gem "simplecov", "~>0.16", :group => [:development, :test]
 gem "rdoc", ">=4.0", "<7", :group => [:development, :test]
 gem "hoe", "~>3.17", :group => [:development, :test]

@@ -1,151 +1,152 @@
 #--
 # DO NOT MODIFY!!!!
-# This file is automatically generated by rex 1.0.5
+# This file is automatically generated by rex 1.0.7
 # from lexical definition file "lib/nokogiri/css/tokenizer.rex".
 #++
 
 module Nokogiri
 module CSS
 class Tokenizer # :nodoc:
-  require 'strscan'
+      require 'strscan'
 
-  class ScanError < StandardError ; end
+      class ScanError < StandardError ; end
 
-  attr_reader   :lineno
-  attr_reader   :filename
-  attr_accessor :state
+      attr_reader   :lineno
+      attr_reader   :filename
+      attr_accessor :state
 
-  def scan_setup(str)
-    @ss = StringScanner.new(str)
-    @lineno =  1
-    @state  = nil
-  end
+      def scan_setup(str)
+        @ss = StringScanner.new(str)
+        @lineno =  1
+        @state  = nil
+      end
 
-  def action
-    yield
-  end
+      def action
+        yield
+      end
 
-  def scan_str(str)
-    scan_setup(str)
-    do_parse
-  end
-  alias :scan :scan_str
+      def scan_str(str)
+        scan_setup(str)
+        do_parse
+      end
+      alias :scan :scan_str
 
-  def load_file( filename )
-    @filename = filename
-    open(filename, "r") do |f|
-      scan_setup(f.read)
-    end
-  end
+      def load_file( filename )
+        @filename = filename
+        File.open(filename, "r") do |f|
+          scan_setup(f.read)
+        end
+      end
 
-  def scan_file( filename )
-    load_file(filename)
-    do_parse
-  end
+      def scan_file( filename )
+        load_file(filename)
+        do_parse
+      end
 
 
-  def next_token
-    return if @ss.eos?
-    
-    # skips empty actions
-    until token = _next_token or @ss.eos?; end
-    token
-  end
+        def next_token
+          return if @ss.eos?
 
-  def _next_token
-    text = @ss.peek(1)
-    @lineno  +=  1  if text == "\n"
-    token = case @state
-    when nil
-      case
-      when (text = @ss.scan(/has\([\s]*/))
-         action { [:HAS, text] }
+          # skips empty actions
+          until token = _next_token or @ss.eos?; end
+          token
+        end
 
-      when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*\([\s]*/))
-         action { [:FUNCTION, text] }
+        def _next_token
+          text = @ss.peek(1)
+          @lineno  +=  1  if text == "\n"
+          token = case @state
+            when nil
+          case
+                  when (text = @ss.scan(/has\([\s]*/))
+                     action { [:HAS, text] }
 
-      when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*/))
-         action { [:IDENT, text] }
+                  when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*\([\s]*/))
+                     action { [:FUNCTION, text] }
 
-      when (text = @ss.scan(/\#([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])+/))
-         action { [:HASH, text] }
+                  when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*/))
+                     action { [:IDENT, text] }
 
-      when (text = @ss.scan(/[\s]*~=[\s]*/))
-         action { [:INCLUDES, text] }
+                  when (text = @ss.scan(/\#([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])+/))
+                     action { [:HASH, text] }
 
-      when (text = @ss.scan(/[\s]*\|=[\s]*/))
-         action { [:DASHMATCH, text] }
+                  when (text = @ss.scan(/[\s]*~=[\s]*/))
+                     action { [:INCLUDES, text] }
 
-      when (text = @ss.scan(/[\s]*\^=[\s]*/))
-         action { [:PREFIXMATCH, text] }
+                  when (text = @ss.scan(/[\s]*\|=[\s]*/))
+                     action { [:DASHMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*\$=[\s]*/))
-         action { [:SUFFIXMATCH, text] }
+                  when (text = @ss.scan(/[\s]*\^=[\s]*/))
+                     action { [:PREFIXMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*\*=[\s]*/))
-         action { [:SUBSTRINGMATCH, text] }
+                  when (text = @ss.scan(/[\s]*\$=[\s]*/))
+                     action { [:SUFFIXMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*!=[\s]*/))
-         action { [:NOT_EQUAL, text] }
+                  when (text = @ss.scan(/[\s]*\*=[\s]*/))
+                     action { [:SUBSTRINGMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*=[\s]*/))
-         action { [:EQUAL, text] }
+                  when (text = @ss.scan(/[\s]*!=[\s]*/))
+                     action { [:NOT_EQUAL, text] }
 
-      when (text = @ss.scan(/[\s]*\)/))
-         action { [:RPAREN, text] }
+                  when (text = @ss.scan(/[\s]*=[\s]*/))
+                     action { [:EQUAL, text] }
 
-      when (text = @ss.scan(/\[[\s]*/))
-         action { [:LSQUARE, text] }
+                  when (text = @ss.scan(/[\s]*\)/))
+                     action { [:RPAREN, text] }
 
-      when (text = @ss.scan(/[\s]*\]/))
-         action { [:RSQUARE, text] }
+                  when (text = @ss.scan(/\[[\s]*/))
+                     action { [:LSQUARE, text] }
 
-      when (text = @ss.scan(/[\s]*\+[\s]*/))
-         action { [:PLUS, text] }
+                  when (text = @ss.scan(/[\s]*\]/))
+                     action { [:RSQUARE, text] }
 
-      when (text = @ss.scan(/[\s]*>[\s]*/))
-         action { [:GREATER, text] }
+                  when (text = @ss.scan(/[\s]*\+[\s]*/))
+                     action { [:PLUS, text] }
 
-      when (text = @ss.scan(/[\s]*,[\s]*/))
-         action { [:COMMA, text] }
+                  when (text = @ss.scan(/[\s]*>[\s]*/))
+                     action { [:GREATER, text] }
 
-      when (text = @ss.scan(/[\s]*~[\s]*/))
-         action { [:TILDE, text] }
+                  when (text = @ss.scan(/[\s]*,[\s]*/))
+                     action { [:COMMA, text] }
 
-      when (text = @ss.scan(/\:not\([\s]*/))
-         action { [:NOT, text] }
+                  when (text = @ss.scan(/[\s]*~[\s]*/))
+                     action { [:TILDE, text] }
 
-      when (text = @ss.scan(/-?([0-9]+|[0-9]*\.[0-9]+)/))
-         action { [:NUMBER, text] }
+                  when (text = @ss.scan(/\:not\([\s]*/))
+                     action { [:NOT, text] }
 
-      when (text = @ss.scan(/[\s]*\/\/[\s]*/))
-         action { [:DOUBLESLASH, text] }
+                  when (text = @ss.scan(/-?([0-9]+|[0-9]*\.[0-9]+)/))
+                     action { [:NUMBER, text] }
 
-      when (text = @ss.scan(/[\s]*\/[\s]*/))
-         action { [:SLASH, text] }
+                  when (text = @ss.scan(/[\s]*\/\/[\s]*/))
+                     action { [:DOUBLESLASH, text] }
 
-      when (text = @ss.scan(/U\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})?/))
-         action {[:UNICODE_RANGE, text] }
+                  when (text = @ss.scan(/[\s]*\/[\s]*/))
+                     action { [:SLASH, text] }
 
-      when (text = @ss.scan(/[\s]+/))
-         action { [:S, text] }
+                  when (text = @ss.scan(/U\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})?/))
+                     action {[:UNICODE_RANGE, text] }
 
-      when (text = @ss.scan(/"([^\n\r\f"]|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*"|'([^\n\r\f']|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*'/))
-         action { [:STRING, text] }
+                  when (text = @ss.scan(/[\s]+/))
+                     action { [:S, text] }
 
-      when (text = @ss.scan(/./))
-         action { [text, text] }
+                  when (text = @ss.scan(/"([^\n\r\f"]|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*"|'([^\n\r\f']|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*'/))
+                     action { [:STRING, text] }
 
-      else
-        text = @ss.string[@ss.pos .. -1]
-        raise  ScanError, "can not match: '" + text + "'"
-      end  # if
+                  when (text = @ss.scan(/./))
+                     action { [text, text] }
 
-    else
-      raise  ScanError, "undefined state: '" + state.to_s + "'"
-    end  # case state
-    token
-  end  # def _next_token
+          
+          else
+            text = @ss.string[@ss.pos .. -1]
+            raise  ScanError, "can not match: '" + text + "'"
+          end  # if
+
+        else
+          raise  ScanError, "undefined state: '" + state.to_s + "'"
+        end  # case state
+          token
+        end  # def _next_token
 
 end # class
 end

@@ -213,7 +213,7 @@ module XML
     #       xml.foo
     #     end
     #   end
-    #   
+    #
     #   puts builder.to_xml
     #
     # Will output this xml:
@@ -250,7 +250,7 @@ class Builder
       #     xml.awesome # add the "awesome" tag below "some_tag"
       #   end
       #
-      def self.with root, &block
+      def self.with(root, &block)
         new({}, root, &block)
       end
 
@@ -263,31 +263,33 @@ def self.with root, &block
       #   Nokogiri::XML::Builder.new(:encoding => 'UTF-8') do |xml|
       #     ...
       #   end
-      def initialize options = {}, root = nil, &block
-
+      def initialize(options = {}, root = nil, &block)
         if root
-          @doc    = root.document
+          @doc = root.document
           @parent = root
         else
-          namespace     = self.class.name.split('::')
-          namespace[-1] = 'Document'
-          @doc          = eval(namespace.join('::')).new
-          @parent       = @doc
+          klassname = "::" + (self.class.name.split("::")[0..-2] + ["Document"]).join("::")
+          klass = begin
+                    Object.const_get(klassname)
+                  rescue NameError
+                    Nokogiri::XML::Document
+                  end
+          @parent = @doc = klass.new
         end
 
-        @context  = nil
-        @arity    = nil
-        @ns       = nil
+        @context = nil
+        @arity = nil
+        @ns = nil
 
-        options.each do |k,v|
+        options.each do |k, v|
           @doc.send(:"#{k}=", v)
         end
 
         return unless block_given?
 
         @arity = block.arity
         if @arity <= 0
-          @context = eval('self', block.binding)
+          @context = eval("self", block.binding)
           instance_eval(&block)
         else
           yield self
@@ -298,26 +300,26 @@ def initialize options = {}, root = nil, &block
 
       ###
       # Create a Text Node with content of +string+
-      def text string
+      def text(string)
         insert @doc.create_text_node(string)
       end
 
       ###
       # Create a CDATA Node with content of +string+
-      def cdata string
+      def cdata(string)
         insert doc.create_cdata(string)
       end
 
       ###
       # Create a Comment Node with content of +string+
-      def comment string
+      def comment(string)
         insert doc.create_comment(string)
       end
 
       ###
       # Build a tag that is associated with namespace +ns+.  Raises an
       # ArgumentError if +ns+ has not been defined higher in the tree.
-      def [] ns
+      def [](ns)
         if @parent != @doc
           @ns = @parent.namespace_definitions.find { |x| x.prefix == ns.to_s }
         end
@@ -348,15 +350,15 @@ def to_xml(*args)
 
       ###
       # Append the given raw XML +string+ to the document
-      def << string
+      def <<(string)
         @doc.fragment(string).children.each { |x| insert(x) }
       end
 
-      def method_missing method, *args, &block # :nodoc:
+      def method_missing(method, *args, &block) # :nodoc:
         if @context && @context.respond_to?(method)
           @context.send(method, *args, &block)
         else
-          node = @doc.create_element(method.to_s.sub(/[_!]$/, ''),*args) { |n|
+          node = @doc.create_element(method.to_s.sub(/[_!]$/, ""), *args) { |n|
             # Set up the namespace
             if @ns.is_a? Nokogiri::XML::Namespace
               n.namespace = @ns
@@ -377,13 +379,14 @@ def method_missing method, *args, &block # :nodoc:
       end
 
       private
+
       ###
       # Insert +node+ as a child of the current Node
       def insert(node, &block)
         node = @parent.add_child(node)
         if block_given?
           old_parent = @parent
-          @parent    = node
+          @parent = node
           @arity ||= block.arity
           if @arity <= 0
             instance_eval(&block)
@@ -396,36 +399,36 @@ def insert(node, &block)
       end
 
       class NodeBuilder # :nodoc:
-        def initialize node, doc_builder
+        def initialize(node, doc_builder)
           @node = node
           @doc_builder = doc_builder
         end
 
-        def []= k, v
+        def []=(k, v)
           @node[k] = v
         end
 
-        def [] k
+        def [](k)
           @node[k]
         end
 
         def method_missing(method, *args, &block)
           opts = args.last.is_a?(Hash) ? args.pop : {}
           case method.to_s
           when /^(.*)!$/
-            @node['id'] = $1
+            @node["id"] = $1
             @node.content = args.first if args.first
           when /^(.*)=/
             @node[$1] = args.first
           else
-            @node['class'] =
-              ((@node['class'] || '').split(/\s/) + [method.to_s]).join(' ')
+            @node["class"] =
+              ((@node["class"] || "").split(/\s/) + [method.to_s]).join(" ")
             @node.content = args.first if args.first
           end
 
           # Assign any extra options
-          opts.each do |k,v|
-            @node[k.to_s] = ((@node[k.to_s] || '').split(/\s/) + [v]).join(' ')
+          opts.each do |k, v|
+            @node[k.to_s] = ((@node[k.to_s] || "").split(/\s/) + [v]).join(" ")
           end
 
           if block_given?

@@ -143,6 +143,7 @@ HOE = Hoe.spec 'nokogiri' do
     ["rake-compiler",      "~> 1.0.3"],
     ["rake-compiler-dock", "~> 0.7.0"],
     ["rexical",            "~> 1.0.5"],
+    ["rubocop",            "~> 0.73"],
     ["simplecov",          "~> 0.16"],
   ]
 
@@ -275,6 +276,11 @@ task :java_debug do
 end
 Rake::Task[:test].prerequisites << :java_debug
 
+task :rubocop_security do
+  sh "rubocop lib --only Security"
+end
+Rake::Task[:test].prerequisites << :rubocop_security
+
 if Hoe.plugins.include?(:debugging)
   ['valgrind', 'valgrind:mem', 'valgrind:mem0'].each do |task_name|
     Rake::Task["test:#{task_name}"].prerequisites << :compile

@@ -10,7 +10,7 @@ def test_attribute_sensitivity
           x.tag "hello", "abcDef" => "world"
         }.to_xml
         doc = Nokogiri.XML xml
-        assert_equal 'world', doc.root['abcDef']
+        assert_equal "world", doc.root["abcDef"]
       end
 
       def test_builder_multiple_nodes
@@ -21,7 +21,6 @@ def test_builder_multiple_nodes
         end
       end
 
-
       def test_builder_with_utf8_text
         text = "test ﺵ "
         doc = Nokogiri::XML::Builder.new(:encoding => "UTF-8") { |xml| xml.test text }.doc
@@ -33,8 +32,8 @@ def test_builder_escape
           x.condition "value < 1", :attr => "value < 1"
         }.to_xml
         doc = Nokogiri.XML xml
-        assert_equal 'value < 1', doc.root['attr']
-        assert_equal 'value < 1', doc.root.content
+        assert_equal "value < 1", doc.root["attr"]
+        assert_equal "value < 1", doc.root.content
       end
 
       def test_builder_namespace
@@ -44,10 +43,10 @@ def test_builder_namespace
           end
         }.doc
 
-        b = doc.at('b')
+        b = doc.at("b")
         assert b
-        assert_equal({"xmlns:a"=>"x", "xmlns:b"=>"y"}, b.namespaces)
-        assert_equal({"xmlns:b"=>"y"}, namespaces_defined_on(b))
+        assert_equal({ "xmlns:a" => "x", "xmlns:b" => "y" }, b.namespaces)
+        assert_equal({ "xmlns:b" => "y" }, namespaces_defined_on(b))
       end
 
       def test_builder_namespace_part_deux
@@ -57,10 +56,10 @@ def test_builder_namespace_part_deux
           end
         }.doc
 
-        b = doc.at('b')
+        b = doc.at("b")
         assert b
-        assert_equal({"xmlns:a"=>"x", "xmlns:b"=>"y", "xmlns:c"=>"z"}, b.namespaces)
-        assert_equal({"xmlns:a"=>"x", "xmlns:c"=>"z"}, namespaces_defined_on(b))
+        assert_equal({ "xmlns:a" => "x", "xmlns:b" => "y", "xmlns:c" => "z" }, b.namespaces)
+        assert_equal({ "xmlns:a" => "x", "xmlns:c" => "z" }, namespaces_defined_on(b))
       end
 
       def test_builder_with_unlink
@@ -75,58 +74,58 @@ def test_builder_with_unlink
 
       def test_with_root
         doc = Nokogiri::XML(File.read(XML_FILE))
-        Nokogiri::XML::Builder.with(doc.at('employee')) do |xml|
+        Nokogiri::XML::Builder.with(doc.at("employee")) do |xml|
           xml.foo
         end
-        assert_equal 1, doc.xpath('//employee/foo').length
+        assert_equal 1, doc.xpath("//employee/foo").length
       end
 
       def test_root_namespace_default_decl
-        b = Nokogiri::XML::Builder.new { |xml| xml.root(:xmlns => 'one:two') }
+        b = Nokogiri::XML::Builder.new { |xml| xml.root(:xmlns => "one:two") }
         doc = b.doc
-        assert_equal 'one:two', doc.root.namespace.href
-        assert_equal({ 'xmlns' => 'one:two' }, doc.root.namespaces)
+        assert_equal "one:two", doc.root.namespace.href
+        assert_equal({ "xmlns" => "one:two" }, doc.root.namespaces)
       end
 
       def test_root_namespace_multi_decl
         b = Nokogiri::XML::Builder.new { |xml|
-          xml.root(:xmlns => 'one:two', 'xmlns:foo' => 'bar') do
+          xml.root(:xmlns => "one:two", "xmlns:foo" => "bar") do
             xml.hello
           end
         }
         doc = b.doc
-        assert_equal 'one:two', doc.root.namespace.href
-        assert_equal({ 'xmlns' => 'one:two', 'xmlns:foo' => 'bar' }, doc.root.namespaces)
+        assert_equal "one:two", doc.root.namespace.href
+        assert_equal({ "xmlns" => "one:two", "xmlns:foo" => "bar" }, doc.root.namespaces)
 
-        assert_equal 'one:two', doc.at('hello').namespace.href
+        assert_equal "one:two", doc.at("hello").namespace.href
       end
 
       def test_non_root_namespace
         b = Nokogiri::XML::Builder.new { |xml|
-          xml.root { xml.hello(:xmlns => 'one') }
+          xml.root { xml.hello(:xmlns => "one") }
         }
-        assert_equal 'one', b.doc.at('hello', 'xmlns' => 'one').namespace.href
+        assert_equal "one", b.doc.at("hello", "xmlns" => "one").namespace.href
       end
 
       def test_specify_namespace
         b = Nokogiri::XML::Builder.new { |xml|
-          xml.root('xmlns:foo' => 'bar') do
+          xml.root("xmlns:foo" => "bar") do
             xml[:foo].bar
-            xml['foo'].baz
+            xml["foo"].baz
           end
         }
         doc = b.doc
-        assert_equal 'bar', doc.at('foo|bar', 'foo' => 'bar').namespace.href
-        assert_equal 'bar', doc.at('foo|baz', 'foo' => 'bar').namespace.href
+        assert_equal "bar", doc.at("foo|bar", "foo" => "bar").namespace.href
+        assert_equal "bar", doc.at("foo|baz", "foo" => "bar").namespace.href
       end
 
       def test_dtd_in_builder_output
         builder = Nokogiri::XML::Builder.new do |xml|
           xml.doc.create_internal_subset(
-                                         'html',
-                                         "-//W3C//DTD HTML 4.01 Transitional//EN",
-                                         "http://www.w3.org/TR/html4/loose.dtd"
-                                         )
+            "html",
+            "-//W3C//DTD HTML 4.01 Transitional//EN",
+            "http://www.w3.org/TR/html4/loose.dtd"
+          )
           xml.root do
             xml.foo
           end
@@ -137,19 +136,19 @@ def test_dtd_in_builder_output
 
       def test_specify_namespace_nested
         b = Nokogiri::XML::Builder.new { |xml|
-          xml.root('xmlns:foo' => 'bar') do
+          xml.root("xmlns:foo" => "bar") do
             xml.yay do
               xml[:foo].bar
 
               xml.yikes do
-                xml['foo'].baz
+                xml["foo"].baz
               end
             end
           end
         }
         doc = b.doc
-        assert_equal 'bar', doc.at('foo|bar', 'foo' => 'bar').namespace.href
-        assert_equal 'bar', doc.at('foo|baz', 'foo' => 'bar').namespace.href
+        assert_equal "bar", doc.at("foo|bar", "foo" => "bar").namespace.href
+        assert_equal "bar", doc.at("foo|baz", "foo" => "bar").namespace.href
       end
 
       def test_specified_namespace_postdeclared
@@ -158,12 +157,12 @@ def test_specified_namespace_postdeclared
             xml[:foo].b("xmlns:foo" => "bar")
           end
         }.doc
-        a = doc.at('a')
+        a = doc.at("a")
         assert_equal({}, a.namespaces)
 
-        b = doc.at_xpath('//foo:b', {:foo=>'bar'})
+        b = doc.at_xpath("//foo:b", { :foo => "bar" })
         assert b
-        assert_equal({"xmlns:foo"=>"bar"}, b.namespaces)
+        assert_equal({ "xmlns:foo" => "bar" }, b.namespaces)
         assert_equal("b", b.name)
         assert_equal("bar", b.namespace.href)
       end
@@ -179,38 +178,38 @@ def test_specified_namespace_undeclared
       end
 
       def test_set_encoding
-        builder = Nokogiri::XML::Builder.new(:encoding => 'UTF-8') do |xml|
+        builder = Nokogiri::XML::Builder.new(:encoding => "UTF-8") do |xml|
           xml.root do
-            xml.bar 'blah'
+            xml.bar "blah"
           end
         end
-        assert_match 'UTF-8', builder.to_xml
+        assert_match "UTF-8", builder.to_xml
       end
 
       def test_bang_and_underscore_is_escaped
         builder = Nokogiri::XML::Builder.new do |xml|
           xml.root do
-            xml.p_('adsfadsf')
-            xml.p!('adsfadsf')
+            xml.p_("adsfadsf")
+            xml.p!("adsfadsf")
           end
         end
-        assert_equal 2, builder.doc.xpath('//p').length
+        assert_equal 2, builder.doc.xpath("//p").length
       end
 
       def test_square_brackets_set_attributes
         builder = Nokogiri::XML::Builder.new do |xml|
           xml.root do
             foo = xml.foo
-            foo['id'] = 'hello'
-            assert_equal 'hello', foo['id']
+            foo["id"] = "hello"
+            assert_equal "hello", foo["id"]
           end
         end
         assert_equal 1, builder.doc.xpath('//foo[@id = "hello"]').length
       end
 
       def test_nested_local_variable
-        @ivar     = 'hello'
-        local_var = 'hello world'
+        @ivar = "hello"
+        local_var = "hello world"
         builder = Nokogiri::XML::Builder.new do |xml|
           xml.root do
             xml.foo local_var
@@ -221,40 +220,40 @@ def test_nested_local_variable
           end
         end
 
-        assert_equal 'hello world', builder.doc.at('//root/foo').content
-        assert_equal 'hello', builder.doc.at('//root/bar').content
-        assert_equal 'hello', builder.doc.at('baz').content
+        assert_equal "hello world", builder.doc.at("//root/foo").content
+        assert_equal "hello", builder.doc.at("//root/bar").content
+        assert_equal "hello", builder.doc.at("baz").content
       end
 
       def test_raw_append
         builder = Nokogiri::XML::Builder.new do |xml|
           xml.root do
-            xml << 'hello'
+            xml << "hello"
           end
         end
 
-        assert_equal 'hello', builder.doc.at('/root').content
+        assert_equal "hello", builder.doc.at("/root").content
       end
 
       def test_raw_append_with_instance_eval
         builder = Nokogiri::XML::Builder.new do
           root do
-            self << 'hello'
+            self << "hello"
           end
         end
 
-        assert_equal 'hello', builder.doc.at('/root').content
+        assert_equal "hello", builder.doc.at("/root").content
       end
 
       def test_raw_xml_append
         builder = Nokogiri::XML::Builder.new do |xml|
           xml.root do
-            xml << '<aaa><bbb/><ccc/></aaa>'
+            xml << "<aaa><bbb/><ccc/></aaa>"
           end
         end
 
-        assert_equal ["aaa"],       builder.doc.at_css("root").children.collect(&:name)
-        assert_equal ["bbb","ccc"], builder.doc.at_css("aaa").children.collect(&:name)
+        assert_equal ["aaa"], builder.doc.at_css("root").children.collect(&:name)
+        assert_equal ["bbb", "ccc"], builder.doc.at_css("aaa").children.collect(&:name)
       end
 
       def test_raw_xml_append_with_namespaces
@@ -264,10 +263,10 @@ def test_raw_xml_append_with_namespaces
           end
         end.doc
 
-        el = doc.at 'Element'
+        el = doc.at "Element"
         assert_not_nil el
 
-        assert_equal 'y', el.namespace.href
+        assert_equal "y", el.namespace.href
         assert_nil el.namespace.prefix
 
         attr = el.attributes["bar"]
@@ -283,7 +282,7 @@ def test_cdata
           }
         end
         assert_equal("<?xml version=\"1.0\"?><root><![CDATA[hello world]]></root>",
-          builder.to_xml.gsub(/\n/, ""))
+                     builder.to_xml.gsub(/\n/, ""))
       end
 
       def test_comment
@@ -302,7 +301,7 @@ def test_builder_no_block
           cdata string
         }
         assert_equal("<?xml version=\"1.0\"?><root><![CDATA[hello world]]></root>",
-          builder.to_xml.gsub(/\n/, ''))
+                     builder.to_xml.gsub(/\n/, ""))
       end
 
       def test_builder_can_inherit_parent_namespace
@@ -314,40 +313,55 @@ def test_builder_can_inherit_parent_namespace
           }
         }
         doc = builder.doc
-        ['product', 'products'].each do |n|
-          assert_equal doc.at_xpath("//*[local-name() = '#{n}']").namespace.href, 'foo'
+        ["product", "products"].each do |n|
+          assert_equal doc.at_xpath("//*[local-name() = '#{n}']").namespace.href, "foo"
         end
       end
 
       def test_builder_can_handle_namespace_override
         builder = Nokogiri::XML::Builder.new
-        builder.products('xmlns:foo' => 'bar') {
-          builder.product('xmlns:foo' => 'baz')
+        builder.products("xmlns:foo" => "bar") {
+          builder.product("xmlns:foo" => "baz")
         }
 
         doc = builder.doc
-        assert_equal doc.at_xpath("//*[local-name() = 'product']").namespaces['xmlns:foo'], 'baz'
-        assert_equal doc.at_xpath("//*[local-name() = 'products']").namespaces['xmlns:foo'], 'bar'
+        assert_equal doc.at_xpath("//*[local-name() = 'product']").namespaces["xmlns:foo"], "baz"
+        assert_equal doc.at_xpath("//*[local-name() = 'products']").namespaces["xmlns:foo"], "bar"
         assert_nil doc.at_xpath("//*[local-name() = 'products']").namespace
       end
 
       def test_builder_reuses_namespaces
         # see https://github.com/sparklemotion/nokogiri/issues/1810 for memory leak report
         builder = Nokogiri::XML::Builder.new
-        builder.send "envelope", {'xmlns' => 'http://schemas.xmlsoap.org/soap/envelope/'} do
-          builder.send "package", {'xmlns' => 'http://schemas.xmlsoap.org/soap/envelope/'}
+        builder.send "envelope", { "xmlns" => "http://schemas.xmlsoap.org/soap/envelope/" } do
+          builder.send "package", { "xmlns" => "http://schemas.xmlsoap.org/soap/envelope/" }
         end
         envelope = builder.doc.at_css("envelope")
         package = builder.doc.at_css("package")
         assert_equal envelope.namespace, package.namespace
         assert_equal envelope.namespace.object_id, package.namespace.object_id
       end
 
+      def test_builder_uses_proper_document_class
+        xml_builder = Nokogiri::XML::Builder.new
+        assert_instance_of Nokogiri::XML::Document, xml_builder.doc
+
+        html_builder = Nokogiri::HTML::Builder.new
+        assert_instance_of Nokogiri::HTML::Document, html_builder.doc
+
+        foo_builder = ThisIsATestBuilder.new
+        assert_instance_of Nokogiri::XML::Document, foo_builder.doc
+      end
+
       private
 
       def namespaces_defined_on(node)
-        Hash[*node.namespace_definitions.collect{|n| ["xmlns:" + n.prefix, n.href]}.flatten]
+        Hash[*node.namespace_definitions.collect { |n| ["xmlns:" + n.prefix, n.href] }.flatten]
       end
     end
   end
 end
+
+class ThisIsATestBuilder < Nokogiri::XML::Builder
+  # this exists for the test_builder_uses_proper_document_class and should be empty
+end

@@ -314,7 +314,7 @@ def scan_str(str)
 
       def load_file( filename )
         @filename = filename
-        open(filename, "r") do |f|
+        File.open(filename, "r") do |f|
           scan_setup(f.read)
         end
       end