diff --git a/config/feature_flags/gitlab_com_derisk/allow_nokogiri_parse_huge_xml.yml b/config/feature_flags/gitlab_com_derisk/allow_nokogiri_parse_huge_xml.yml
new file mode 100644
index 0000000000000000000000000000000000000000..00034cefcd7a694081a07391064153f4ad8634d2
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/allow_nokogiri_parse_huge_xml.yml
@@ -0,0 +1,8 @@
+---
+name: allow_nokogiri_parse_huge_xml
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148779
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/454797
+milestone: '17.0'
+group: group::pipeline execution
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/lib/gitlab/ci/parsers/test/junit.rb b/lib/gitlab/ci/parsers/test/junit.rb
index 04481b10b3949a24eff395431f8f98b62f9c2b04..fc91bff0e181b11881ca97f949ffb159c4a85dc8 100644
--- a/lib/gitlab/ci/parsers/test/junit.rb
+++ b/lib/gitlab/ci/parsers/test/junit.rb
@@ -10,8 +10,12 @@ class Junit
def parse!(xml_data, test_report, job:)
test_suite = test_report.get_suite(job.test_suite_name)
+ root = if Feature.enabled?(:allow_nokogiri_parse_huge_xml, job.project)
+ XmlConverter.new(xml_data).to_h
+ else
+ Hash.from_xml(xml_data)
+ end
- root = Hash.from_xml(xml_data)
total_parsed = 0
max_test_cases = job.max_test_cases_per_report
diff --git a/lib/gitlab/xml_converter.rb b/lib/gitlab/xml_converter.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2c089ed13426e28995fc0395ba27c853ca12fe62
--- /dev/null
+++ b/lib/gitlab/xml_converter.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+# rubocop:disable Gitlab/NamespacedClass -- Base method live in the global namespace
+module Gitlab
+ MAX_XML_SIZE = 30.megabytes
+
+ class XmlConverter < ActiveSupport::XMLConverter
+ # Override the default Nokogiri parser in to allow parsing huge XML files
+ def initialize(xml, disallowed_types = nil)
+ if xml.size > MAX_XML_SIZE
+ raise ArgumentError, format(_("The XML file must be less than %{max_size} MB."),
+ max_size: MAX_XML_SIZE / 1.megabyte)
+ end
+
+ doc = Nokogiri::XML(xml, &:huge)
+ raise doc.errors.first unless doc.errors.empty?
+
+ # These two variables are internally required by `ActiveSupport::XMLConverter`
+ @xml = normalize_keys(doc.to_hash)
+ @disallowed_types = disallowed_types || DISALLOWED_TYPES
+ end
+ end
+end
+# rubocop:enable Gitlab/NamespacedClass
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 29eb0a159b5f89517da017277e86be197a6c2bb9..9762cbfffc5349fcd89df0aaaf59bb418453af30 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -51402,6 +51402,9 @@ msgstr ""
msgid "The URLs for connecting to Elasticsearch. For clustering, add the URLs separated by commas."
msgstr ""
+msgid "The XML file must be less than %{max_size} MB."
+msgstr ""
+
msgid "The `/merge` quick action requires the SHA of the head of the branch."
msgstr ""
diff --git a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
index 64db92a1d890be9c48783bb5f375eccb55142a6d..77a693f1d0da627f85d5d7e21c4d42bb9da1ef59 100644
--- a/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
+++ b/spec/lib/gitlab/ci/parsers/test/junit_spec.rb
@@ -1,17 +1,19 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+require 'spec_helper'
RSpec.describe Gitlab::Ci::Parsers::Test::Junit do
describe '#parse!' do
subject { described_class.new.parse!(junit, test_report, job: job) }
- let(:job) { double(test_suite_name: 'rspec', max_test_cases_per_report: max_test_cases) }
+ let(:project) { build(:project) }
+ let(:job) { double(test_suite_name: 'rspec', max_test_cases_per_report: max_test_cases, project: project) }
let(:test_report) { Gitlab::Ci::Reports::TestReport.new }
let(:test_suite) { test_report.get_suite(job.test_suite_name) }
let(:test_cases) { flattened_test_cases(test_suite) }
let(:max_test_cases) { 0 }
+ let(:junit) { '<testsuites></testsuites>' }
context 'when data is JUnit style XML' do
context 'when there are no <testcases> in <testsuite>' do
@@ -379,7 +381,7 @@
end
context 'when data is not XML' do
- let(:junit) { double(:random_trash) }
+ let(:junit) { double(:random_trash, size: 1) }
it 'attaches an error to the TestSuite object' do
expect { subject }.not_to raise_error
@@ -521,6 +523,26 @@
end
end
+ it 'parses XML using XmlConverter instead of Hash.from_xml' do
+ expect(Gitlab::XmlConverter).to receive(:new).with(junit).once.and_call_original
+ expect(Hash).not_to receive(:from_xml)
+
+ subject
+ end
+
+ context 'when feature flag `allow_nokogiri_parse_huge_xml` is disabled' do
+ before do
+ stub_feature_flags(allow_nokogiri_parse_huge_xml: false)
+ end
+
+ it 'parses XML using Hash.from_xml instead of XmlConverter' do
+ expect(Gitlab::XmlConverter).not_to receive(:new)
+ expect(Hash).to receive(:from_xml).with(junit).once.and_call_original
+
+ subject
+ end
+ end
+
private
def flattened_test_cases(test_suite)
diff --git a/spec/lib/gitlab/xml_converter_spec.rb b/spec/lib/gitlab/xml_converter_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..de5130127580dee058d1ac670abeb8613c4d6a79
--- /dev/null
+++ b/spec/lib/gitlab/xml_converter_spec.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Gitlab::XmlConverter, feature_category: :pipeline_composition do
+ describe '#to_h' do
+ let(:xml_data) { '<root><key>value</key></root>' }
+
+ subject(:to_h) { described_class.new(xml_data).to_h }
+
+ context "when the xml is valid" do
+ let(:xml_data) { '<root><key>value</key></root>' }
+
+ it "parses the xml with huge option" do
+ expect(Nokogiri).to receive(:XML).and_wrap_original do |original_method, *args, &block|
+ expect(block).to be(Proc.new(&:huge))
+ original_method.call(*args, &block)
+ end
+
+ expect(to_h).to eq('root' => { 'key' => 'value' })
+ end
+ end
+
+ context "when the xml is invalid" do
+ let(:xml_data) { '<root><key>value</key>' }
+
+ it "raises an error" do
+ expect { to_h }.to raise_error(Nokogiri::XML::SyntaxError)
+ end
+ end
+
+ context "when the xml is too large" do
+ let(:xml_data) { instance_double(String, size: Gitlab::MAX_XML_SIZE + 1) }
+
+ it "raises an error" do
+ expect { to_h }.to raise_error(ArgumentError, "The XML file must be less than 30 MB.")
+ end
+ end
+ end
+end