Commit af9b0bfb authored by Grzegorz Bizon's avatar Grzegorz Bizon

Simplify untrusted regexp factory method

parent 61d55b56
......@@ -55,7 +55,7 @@ module Gitlab
end
def self.valid?(pattern)
self.fabricate(pattern)
!!self.fabricate(pattern)
rescue RegexpError
false
end
......@@ -63,16 +63,13 @@ module Gitlab
def self.fabricate(pattern)
matches = pattern.match(%r{^/(?<regexp>.+)/(?<flags>[ismU]*)$})
if matches
raise RegexpError, 'Invalid regular expression!' if matches.nil?
expression = matches[:regexp]
flags = matches[:flags]
expression.prepend("(?#{flags})") if flags.present?
self.new(expression, multiline: false)
else
self.new(pattern, multiline: false)
end
end
private
......
......@@ -79,7 +79,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
describe '#evaluate' do
it 'returns a regular expression' do
regexp = described_class.new('abc')
regexp = described_class.new('/abc/')
expect(regexp.evaluate).to eq Gitlab::UntrustedRegexp.new('abc')
end
......@@ -87,7 +87,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
it 'raises error if evaluated regexp is not valid' do
allow(Gitlab::UntrustedRegexp).to receive(:valid?).and_return(true)
regexp = described_class.new('invalid ( .*')
regexp = described_class.new('/invalid ( .*/')
expect { regexp.evaluate }
.to raise_error(Gitlab::Ci::Pipeline::Expression::RuntimeError)
......
......@@ -4,9 +4,13 @@ require 'support/shared_examples/malicious_regexp_shared_examples'
describe Gitlab::UntrustedRegexp do
describe '.valid?' do
it 'returns true if regexp is valid' do
expect(described_class.valid?('/some ( thing/'))
.to be false
end
it 'returns true if regexp is invalid' do
expect(described_class.valid?('/some .* thing/'))
.to be true
end
end
......@@ -32,17 +36,9 @@ describe Gitlab::UntrustedRegexp do
end
end
context 'when regexp is not plain pattern' do
it 'fabricates regexp without flags' do
regexp = described_class.fabricate('something')
expect(regexp).to eq described_class.new('something')
end
end
context 'when regexp is invalid' do
context 'when regexp is a raw pattern' do
it 'raises an error' do
expect { described_class.fabricate('/some ( thing/') }
expect { described_class.fabricate('some .* thing') }
.to raise_error(RegexpError)
end
end
......
require 'timeout'
shared_examples 'malicious regexp' do
let(:malicious_text) { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' }
let(:malicious_regexp) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' }
......
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