Merge pull request #5847 from coolo/be_strict_in_minitest

Be strict with XML documents

src/api/app/controllers/announcements_controller.rb

@@ -60,7 +60,7 @@ def set_announcement
 
   # Only allow a trusted parameter "white list" through.
   def announcement_params
-    xml = Nokogiri::XML(request.raw_post)
+    xml = Nokogiri::XML(request.raw_post, &:strict)
     title = xml.xpath('//announcement/title').text
     content = xml.xpath('//announcement/content').text
 

src/api/app/controllers/application_controller.rb

@@ -221,7 +221,7 @@ def pass_to_backend(path = nil)
 
   rescue_from Backend::Error do |exception|
     text = exception.message
-    xml = Nokogiri::XML(text).root
+    xml = Nokogiri::XML(text, &:strict).root
     http_status = xml['code'] || 500
     xml['origin'] ||= 'backend'
     text = xml.to_xml

src/api/app/controllers/issue_trackers_controller.rb

@@ -41,7 +41,7 @@ def create
       @issue_tracker = IssueTracker.new(params)
     rescue
       # User didn't really upload www-form-urlencoded data but raw XML, try to parse that
-      xml = Nokogiri::XML(request.raw_post).root
+      xml = Nokogiri::XML(request.raw_post, &:strict).root
       @issue_tracker = IssueTracker.create(name: xml.xpath('name[1]/text()').to_s,
                                            kind: xml.xpath('kind[1]/text()').to_s,
                                            description: xml.xpath('description[1]/text()').to_s,
@@ -78,7 +78,7 @@ def update
         ret = @issue_tracker.update_attributes(request.request_parameters)
       rescue ActiveRecord::UnknownAttributeError, ActiveModel::MassAssignmentSecurity::Error
         # User didn't really upload www-form-urlencoded data but raw XML, try to parse that
-        xml = Nokogiri::XML(request.raw_post).root
+        xml = Nokogiri::XML(request.raw_post, &:strict).root
         attribs = {}
         attribs[:name] = xml.xpath('name[1]/text()').to_s unless xml.xpath('name[1]/text()').empty?
         attribs[:kind] = xml.xpath('kind[1]/text()').to_s unless xml.xpath('kind[1]/text()').empty?

src/api/app/controllers/message_controller.rb

@@ -36,7 +36,7 @@ def delete
   end
 
   def update
-    new_msg = Nokogiri::XML(request.raw_post).root
+    new_msg = Nokogiri::XML(request.raw_post, &:strict).root
     begin
       msg = Message.new
       msg.text = new_msg.content

src/api/app/controllers/request_controller.rb

@@ -35,7 +35,7 @@ def render_request_collection
     rel = BsRequest.find_for(params).includes(bs_request_actions: :bs_request_action_accept_info)
     rel = rel.limit(params[:limit].to_i) if params[:limit].to_i > 0
 
-    xml = Nokogiri::XML('<collection/>').root
+    xml = Nokogiri::XML('<collection/>', &:strict).root
     matches = 0
     rel.each do |r|
       matches += 1
@@ -170,7 +170,7 @@ def request_command_diff
 
     diff_text = ''
     if params[:view] == 'xml'
-      xml_request = Nokogiri::XML("<request id='#{req.number}'/>").root
+      xml_request = Nokogiri::XML("<request id='#{req.number}'/>", &:strict).root
     end
 
     req.bs_request_actions.each do |action|

src/api/app/controllers/status_messages_controller.rb

@@ -22,7 +22,7 @@ def create
       raise PermissionDeniedError, 'message(s) cannot be created, you have not sufficient permissions'
     end
 
-    new_messages = Nokogiri::XML(request.raw_post).root
+    new_messages = Nokogiri::XML(request.raw_post, &:strict).root
     @messages = []
     if new_messages.css('message').present?
       # message(s) are wrapped in outer xml tag 'status_messages'

src/api/app/helpers/maintenance_helper.rb

@@ -26,7 +26,7 @@ def _release_package(source_package, target_project, target_package_name, action
       # detect local links
       begin
         link = source_package.source_file('_link')
-        link = Nokogiri::XML(link).root
+        link = Nokogiri::XML(link, &:strict).root
         links_to_source = link['project'].nil? || link['project'] == source_package.project.name
       rescue Backend::Error
       end
@@ -398,7 +398,7 @@ def instantiate_container(project, opackage, opts = {})
                                                                    :oproject, :opackage])
       Backend::Connection.post path
       # and fix the link
-      link_xml = Nokogiri::XML(lpkg.source_file('_link')).root
+      link_xml = Nokogiri::XML(lpkg.source_file('_link'), &:strict).root
       link_xml.remove_attribute('project') # its a local link, project name not needed
       link_xml['package'] = pkg.name
       Backend::Connection.put lpkg.source_path('_link', user: User.current.login), link_xml.to_xml

src/api/app/models/branch_package.rb

@@ -134,7 +134,7 @@ def create_branch_packages(tprj)
         # copy project local linked packages
         Backend::Api::Sources::Package.copy(tpkg.project.name, tpkg.name, p[:link_target_project].name, p[:package].name, User.current.login)
         # and fix the link
-        ret = Nokogiri::XML(tpkg.source_file('_link')).root
+        ret = Nokogiri::XML(tpkg.source_file('_link'), &:strict).root
         ret.remove_attribute('project') # its a local link, project name not needed
         linked_package = p[:link_target_package]
         # user enforce a rename of base package

src/api/app/models/patchinfo.rb

@@ -44,7 +44,7 @@ def hashed
 
   # patchinfo has two roles
   def initialize(data = '<patchinfo/>')
-    @document = Nokogiri::XML(data)
+    @document = Nokogiri::XML(data, &:strict)
   end
 
   def is_repository_matching?(repo, rt)

src/api/app/models/project.rb

@@ -1006,8 +1006,8 @@ def branch_local_repositories(project, pkg_to_enable, opts = {})
 
   def branch_remote_repositories(project)
     remote_project = Project.new(name: project)
-    remote_project_meta = Nokogiri::XML(remote_project.meta.content)
-    local_project_meta = Nokogiri::XML(render_xml)
+    remote_project_meta = Nokogiri::XML(remote_project.meta.content, &:strict)
+    local_project_meta = Nokogiri::XML(render_xml, &:strict)
 
     remote_repositories = remote_project.repositories_from_meta
     remote_repositories -= repositories.where(name: remote_repositories).pluck(:name)
@@ -1048,7 +1048,7 @@ def meta
 
   def repositories_from_meta
     result = []
-    Nokogiri::XML(meta.content).xpath('//repository').each do |repo|
+    Nokogiri::XML(meta.content, &:strict).xpath('//repository').each do |repo|
       result.push(repo.attributes.values.first.to_s)
     end
     result

src/api/app/models/service.rb

@@ -12,7 +12,7 @@ def document
     return @document if @document
     xml = Backend::Api::Sources::Package.service(project.name, package.name)
     xml ||= '<services/>'
-    @document = Nokogiri::XML(xml)
+    @document = Nokogiri::XML(xml, &:strict)
   end
 
   def self.valid_name?(name)

src/api/lib/opensuse/validator.rb

@@ -97,7 +97,7 @@ def validate(opt, content)
           raise ValidationError, "Document is empty, not allowed for #{schema_file}"
         end
         begin
-          doc = Nokogiri::XML(content, nil, nil, Nokogiri::XML::ParseOptions::STRICT)
+          doc = Nokogiri::XML(content, &:strict)
           schema.validate(doc).each do |error|
             logger.error "validation error: #{error}"
             logger.debug "Schema #{schema_file} for: #{content}"

src/api/spec/controllers/public_controller_spec.rb

@@ -185,7 +185,7 @@
     context 'with history unlimited' do
       before do
         get :source_file, params: { project: project.name, package: package.name, filename: '_history' }
-        @revisions = Nokogiri::XML(response.body).xpath('//revision')
+        @revisions = Nokogiri::XML(response.body, &:strict).xpath('//revision')
       end
 
       it { is_expected.to respond_with(:success) }
@@ -195,7 +195,7 @@
     context 'with history limited to 1' do
       before do
         get :source_file, params: { project: project.name, package: package.name, filename: '_history', limit: 1 }
-        @revisions = Nokogiri::XML(response.body).xpath('//revision')
+        @revisions = Nokogiri::XML(response.body, &:strict).xpath('//revision')
       end
 
       it { is_expected.to respond_with(:success) }

src/api/spec/controllers/webui/sitemaps_controller_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe Webui::SitemapsController do
-  let(:paths) { Nokogiri::XML(response.body).xpath('//xmlns:loc').map { |url| URI.parse(url.content).path } }
+  let(:paths) { Nokogiri::XML(response.body, &:strict).xpath('//xmlns:loc').map { |url| URI.parse(url.content).path } }
 
   describe 'GET #index' do
     render_views

src/api/spec/models/bs_request_spec.rb

@@ -50,7 +50,7 @@
              source_project: source_package.project.name,
              source_package: source_package.name)
     end
-    let(:doc) { Nokogiri::XML(review_request.to_axml) }
+    let(:doc) { Nokogiri::XML(review_request.to_axml, &:strict) }
 
     context "'when' attribute provided" do
       let!(:updated_when) { 10.years.ago }

src/api/test/functional/channel_maintenance_test.rb

@@ -144,7 +144,7 @@ def test_large_channel_test
     # add an old style patch name, only used via %N (in BaseDistro3Channel at the end of this test)
     get "/source/#{incident_project}/patchinfo/_patchinfo"
     assert_response :success
-    pi = Nokogiri::XML(@response.body).root
+    pi = Nokogiri::XML(@response.body, &:strict).root
     pi.add_child('<name>patch_name</name>')
     pi.add_child('<message>During reboot a popup with a question will appear</message>')
     put "/source/#{incident_project}/patchinfo/_patchinfo", params: pi.to_xml
@@ -298,7 +298,7 @@ def test_large_channel_test
     assert_response :success
     get '/source/My:Maintenance/_meta'
     assert_response :success
-    meta = Nokogiri::XML(@response.body).root
+    meta = Nokogiri::XML(@response.body, &:strict).root
     meta.at_xpath('maintenance').add_child('<maintains project="Channel"/>')
     put '/source/My:Maintenance/_meta', params: meta.to_xml
     assert_response :success
@@ -519,7 +519,7 @@ def test_large_channel_test
     assert_response 404
     get "/source/#{incident_project}/patchinfo/_patchinfo"
     assert_response :success
-    pi = Nokogiri::XML(@response.body).root
+    pi = Nokogiri::XML(@response.body, &:strict).root
     pi.add_child('<issue id="0815" tracker="bnc"/>')
     put "/source/#{incident_project}/patchinfo/_patchinfo", params: pi.to_xml
     assert_response :success

src/api/test/functional/maintenance_test.rb

@@ -1023,7 +1023,7 @@ def test_create_maintenance_project_and_release_packages
     # add reader role for adrian
     get '/source/' + incident_project + '/_meta'
     assert_response :success
-    meta = Nokogiri::XML(@response.body).root
+    meta = Nokogiri::XML(@response.body, &:strict).root
     meta.add_child('<person userid="adrian" role="reader"/>')
     Timecop.freeze(1)
     put '/source/' + incident_project + '/_meta', params: meta.to_xml
@@ -1033,7 +1033,7 @@ def test_create_maintenance_project_and_release_packages
     assert_xml_tag(tag: 'patchinfo', attributes: { incident: incident_id })
     # FIXME: add another patchinfo pointing to a third place
     # add required informations about the update
-    pi = Nokogiri::XML(@response.body).root
+    pi = Nokogiri::XML(@response.body, &:strict).root
     pi.at_xpath('.//summary').content = 'if you are bored'
     pi.at_xpath('.//description').content = 'if you are bored and really want fixes'
     pi.at_xpath('.//rating').content = 'important'
@@ -1159,7 +1159,7 @@ def test_create_maintenance_project_and_release_packages
     # block patchinfo build
     get "/source/#{incident_project}/patchinfo/_patchinfo"
     assert_response :success
-    pi = Nokogiri::XML(@response.body).root
+    pi = Nokogiri::XML(@response.body, &:strict).root
     pi.add_child('<stopped>The issue is not fixed for real yet</stopped>')
     put "/source/#{incident_project}/patchinfo/_patchinfo", params: pi.to_xml
     assert_response :success
@@ -2057,7 +2057,7 @@ def test_validate_evergreen_reviewers
     login_king
     get '/source/BaseDistro:Update/_meta'
     assert_response :success
-    meta = originmeta = Nokogiri::XML(@response.body).root
+    meta = originmeta = Nokogiri::XML(@response.body, &:strict).root
     meta.add_child('<person userid="adrian" role="reviewer"/>')
     put '/source/BaseDistro:Update/_meta', params: meta.to_xml
     assert_response :success

src/api/test/functional/source_controller_test.rb

@@ -391,7 +391,7 @@ def test_put_project_meta_with_invalid_permissions
     # Change description
     xml = @response.body
     new_desc = 'Changed description 1'
-    doc = Nokogiri::XML(xml).root
+    doc = Nokogiri::XML(xml, &:strict).root
     d = doc.at_xpath('//description')
     d.content = new_desc
 
@@ -407,7 +407,7 @@ def test_put_project_meta_with_invalid_permissions
     assert_response 403
     assert_match(/admin rights are required to change projects using remote resources/, @response.body)
     # DoD remote repository
-    doc = Nokogiri::XML(xml).root
+    doc = Nokogiri::XML(xml, &:strict).root
     r = doc.add_child('<repository name="download_on_demand"/>')
     r.first.add_child('<download arch="i586" url="http://somewhere" repotype="rpmmd"/>')
     put url_for(controller: :source_project_meta, action: :update, project: 'kde4'), params: doc.to_xml

src/api/test/test_helper.rb

@@ -299,7 +299,8 @@ def load_backend_file(path)
     end
 
     def check_xml_tag(data, conds)
-      NodeMatcher.new(conds).find_matching(Nokogiri::XML(data).root)
+      xml = Nokogiri::XML(data, &:strict)
+      NodeMatcher.new(conds).find_matching(xml.root)
     end
 
     def assert_xml_tag(conds)
@@ -355,7 +356,8 @@ class ActiveSupport::TestCase
   set_fixture_class history_elements: HistoryElement::Base
 
   def check_xml_tag(data, conds)
-    NodeMatcher.new(conds).find_matching(Nokogiri::XML(data).root)
+    xml = Nokogiri::XML(data, &:strict)
+    NodeMatcher.new(conds).find_matching(xml.root)
   end
 
   def assert_xml_tag(data, conds)