Skip to content

Commit 5723105

Browse files
committed
properly parse query string
1 parent 34cc41a commit 5723105

File tree

2 files changed

+46
-8
lines changed

2 files changed

+46
-8
lines changed

lib/bullet/rack.rb

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require 'rack/request'
44
require 'json'
5+
require 'cgi'
56

67
module Bullet
78
class Rack
@@ -85,17 +86,20 @@ def skip_html_injection?(request)
8586
query_string = request.env['QUERY_STRING']
8687
return false if query_string.nil? || query_string.empty?
8788

88-
if defined?(::Rack::QueryParser)
89-
parser = ::Rack::QueryParser.new
90-
params = parser.parse_nested_query(query_string)
91-
else
92-
# compatible with rack 1.x,
93-
# remove it after dropping rails 4.2 suppport
94-
params = ::Rack::Utils.parse_nested_query(query_string)
95-
end
89+
params = simple_parse_query_string(query_string)
9690
params['skip_html_injection'] == 'true'
9791
end
9892

93+
# Simple query string parser
94+
def simple_parse_query_string(query_string)
95+
params = {}
96+
query_string.split('&').each do |pair|
97+
key, value = pair.split('=', 2).map { |s| CGI.unescape(s) }
98+
params[key] = value if key && !key.empty?
99+
end
100+
params
101+
end
102+
99103
def file?(headers)
100104
headers['Content-Transfer-Encoding'] == 'binary' || headers['Content-Disposition']
101105
end

spec/bullet/rack_spec.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,40 @@ module Bullet
3333
end
3434
end
3535

36+
context '#skip_html_injection?' do
37+
let(:request) { double('request') }
38+
39+
it 'should return false if query_string is nil' do
40+
allow(request).to receive(:env).and_return({ 'QUERY_STRING' => nil })
41+
expect(middleware.skip_html_injection?(request)).to be_falsey
42+
end
43+
44+
it 'should return false if query_string is empty' do
45+
allow(request).to receive(:env).and_return({ 'QUERY_STRING' => '' })
46+
expect(middleware.skip_html_injection?(request)).to be_falsey
47+
end
48+
49+
it 'should return true if skip_html_injection parameter is true' do
50+
allow(request).to receive(:env).and_return({ 'QUERY_STRING' => 'skip_html_injection=true' })
51+
expect(middleware.skip_html_injection?(request)).to be_truthy
52+
end
53+
54+
it 'should return false if skip_html_injection parameter is not true' do
55+
allow(request).to receive(:env).and_return({ 'QUERY_STRING' => 'skip_html_injection=false' })
56+
expect(middleware.skip_html_injection?(request)).to be_falsey
57+
end
58+
59+
it 'should return false if skip_html_injection parameter is not present' do
60+
allow(request).to receive(:env).and_return({ 'QUERY_STRING' => 'other_param=value' })
61+
expect(middleware.skip_html_injection?(request)).to be_falsey
62+
end
63+
64+
it 'should handle complex query strings' do
65+
allow(request).to receive(:env).and_return({ 'QUERY_STRING' => 'param1=value1&skip_html_injection=true&param2=value2' })
66+
expect(middleware.skip_html_injection?(request)).to be_truthy
67+
end
68+
end
69+
3670
context 'empty?' do
3771
it 'should be false if response is a string and not empty' do
3872
response = double(body: '<html><head></head><body></body></html>')

0 commit comments

Comments
 (0)