Commit 7716c7e8 authored by Robert Speicher's avatar Robert Speicher Committed by Robert Speicher

Merge branch 'fix-out-of-bounds-markdown-refs' into 'master'

Fix RangeError exceptions when referring to issues or merge requests outside of max database values

When using #XYZ in Markdown text, if XYZ exceeds the maximum value of a signed 32-bit integer, we get an exception when the Markdown render attempts to run `where(iids: XYZ)`. Introduce a method that will throw out out-of-bounds values.
    
Closes #18777

See merge request !4777
parent fc6fd97c
...@@ -49,6 +49,10 @@ module Referable ...@@ -49,6 +49,10 @@ module Referable
raise NotImplementedError, "#{self} does not implement #{__method__}" raise NotImplementedError, "#{self} does not implement #{__method__}"
end end
def reference_valid?(reference)
true
end
def link_reference_pattern(route, pattern) def link_reference_pattern(route, pattern)
%r{ %r{
(?<url> (?<url>
......
...@@ -83,6 +83,10 @@ class Issue < ActiveRecord::Base ...@@ -83,6 +83,10 @@ class Issue < ActiveRecord::Base
@link_reference_pattern ||= super("issues", /(?<issue>\d+)/) @link_reference_pattern ||= super("issues", /(?<issue>\d+)/)
end end
def self.reference_valid?(reference)
reference.to_i > 0 && reference.to_i <= Gitlab::Database::MAX_INT_VALUE
end
def self.sort(method, excluded_labels: []) def self.sort(method, excluded_labels: [])
case method.to_s case method.to_s
when 'due_date_asc' then order_due_date_asc when 'due_date_asc' then order_due_date_asc
......
...@@ -133,6 +133,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -133,6 +133,10 @@ class MergeRequest < ActiveRecord::Base
@link_reference_pattern ||= super("merge_requests", /(?<merge_request>\d+)/) @link_reference_pattern ||= super("merge_requests", /(?<merge_request>\d+)/)
end end
def self.reference_valid?(reference)
reference.to_i > 0 && reference.to_i <= Gitlab::Database::MAX_INT_VALUE
end
# Returns all the merge requests from an ActiveRecord:Relation. # Returns all the merge requests from an ActiveRecord:Relation.
# #
# This method uses a UNION as it usually operates on the result of # This method uses a UNION as it usually operates on the result of
......
...@@ -218,8 +218,9 @@ module Banzai ...@@ -218,8 +218,9 @@ module Banzai
nodes.each do |node| nodes.each do |node|
node.to_html.scan(regex) do node.to_html.scan(regex) do
project = $~[:project] || current_project_path project = $~[:project] || current_project_path
symbol = $~[object_sym]
refs[project] << $~[object_sym] refs[project] << symbol if object_class.reference_valid?(symbol)
end end
end end
......
module Gitlab module Gitlab
module Database module Database
# The max value of INTEGER type is the same between MySQL and PostgreSQL:
# https://www.postgresql.org/docs/9.2/static/datatype-numeric.html
# http://dev.mysql.com/doc/refman/5.7/en/integer-types.html
MAX_INT_VALUE = 2147483647
def self.adapter_name def self.adapter_name
connection.adapter_name connection.adapter_name
end end
......
...@@ -8,7 +8,7 @@ describe Banzai::Filter::AbstractReferenceFilter do ...@@ -8,7 +8,7 @@ describe Banzai::Filter::AbstractReferenceFilter do
doc = Nokogiri::HTML.fragment("#1 #{project.to_reference}#2") doc = Nokogiri::HTML.fragment("#1 #{project.to_reference}#2")
filter = described_class.new(doc, project: project) filter = described_class.new(doc, project: project)
expect(filter).to receive(:object_class).twice.and_return(Issue) expect(filter).to receive(:object_class).exactly(4).times.and_return(Issue)
expect(filter).to receive(:object_sym).twice.and_return(:issue) expect(filter).to receive(:object_sym).twice.and_return(:issue)
refs = filter.references_per_project refs = filter.references_per_project
......
...@@ -134,6 +134,12 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do ...@@ -134,6 +134,12 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
expect(reference_filter(act).to_html).to eq exp expect(reference_filter(act).to_html).to eq exp
end end
it 'ignores out-of-bounds issue IDs on the referenced project' do
exp = act = "Fixed ##{Gitlab::Database::MAX_INT_VALUE + 1}"
expect(reference_filter(act).to_html).to eq exp
end
end end
context 'cross-project URL reference' do context 'cross-project URL reference' do
......
...@@ -38,6 +38,12 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do ...@@ -38,6 +38,12 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do
expect(reference_filter(act).to_html).to eq exp expect(reference_filter(act).to_html).to eq exp
end end
it 'ignores out-of-bounds merge request IDs on the referenced project' do
exp = act = "Merge !#{Gitlab::Database::MAX_INT_VALUE + 1}"
expect(reference_filter(act).to_html).to eq exp
end
it 'includes a title attribute' do it 'includes a title attribute' do
doc = reference_filter("Merge #{reference}") doc = reference_filter("Merge #{reference}")
expect(doc.css('a').first.attr('title')).to eq "Merge Request: #{merge.title}" expect(doc.css('a').first.attr('title')).to eq "Merge Request: #{merge.title}"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment