Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/webrick/httpservlet/cgihandler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def do_GET(req, res)
"Premature end of script headers: #{@script_filename}" if body.nil?

begin
header = HTTPUtils::parse_header(raw_header)
header = HTTPUtils::parse_header(raw_header, true)
if /^(\d+)/ =~ header['status'][0]
res.status = $1.to_i
header.delete('status')
Expand Down
11 changes: 8 additions & 3 deletions lib/webrick/httputils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,17 +168,22 @@ def join(separator = "; ")
"cookie" => CookieHeader,
})

def parse_header(raw)
def parse_header(raw, cgi_mode = false)
header = Hash.new([].freeze)
field = nil

line_break = cgi_mode ? "\\r?\\n" : "\\r\\n"
header_line = Regexp.new(/^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):([^\r\n\0]*?)#{line_break}\z/m)
continued_header_lines = Regexp.new(/^[ \t]+([^\r\n\0]*?)#{line_break}/m)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allocates 2 regexps per call. Can we instead add 4 Regexp constants (2 for CGI mode and 2 for non-CGI mode), and use those constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. That makes a lot of sense, especially for a frequently called function like this. I'll update it to use constants.


raw.each_line{|line|
case line
when /^([A-Za-z0-9!\#$%&'*+\-.^_`|~]+):([^\r\n\0]*?)\r\n\z/om
when header_line
field, value = $1, $2
field.downcase!
header[field] = HEADER_CLASSES[field].new unless header.has_key?(field)
header[field] << value
when /^[ \t]+([^\r\n\0]*?)\r\n/om
when continued_header_lines
unless field
raise HTTPStatus::BadRequest, "bad header '#{line}'."
end
Expand Down
2 changes: 1 addition & 1 deletion sig/httputils.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module WEBrick

HEADER_CLASSES: Hash[String, untyped]

def self?.parse_header: (String raw) -> Hash[String, Array[String]]
def self?.parse_header: (String raw, ?bool cgi_mode) -> Hash[String, Array[String]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also need updating for the keyword argument, but I've never written RBS before, so I'm not sure how.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have never written RBS either, but when I run rbs, it outputs the following:

 def self?.parse_header: (untyped raw, ?cgi_mode: bool) -> untyped

So I think it would be written like this:

 def self?.parse_header: (String raw, ?cgi_mode: bool) -> Hash[String, Array[String]]


def self?.split_header_value: (String str) -> Array[String]

Expand Down
11 changes: 11 additions & 0 deletions test/webrick/test_cgi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,15 @@ def test_bad_header
assert_not_match(CtrlPat, s)
}
end

def test_bare_lf_in_cgi_header
TestWEBrick.start_cgi_server do |server, addr, port, log|
http = Net::HTTP.new(addr, port)
req = Net::HTTP::Get.new("/webrick_bare_lf.cgi")
assert_nothing_raised do
res = http.request(req)
assert_equal res['Content-Type'], 'text/plain'
end
end
end
end
8 changes: 8 additions & 0 deletions test/webrick/webrick_bare_lf.cgi
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!ruby

body = "test for bare LF in cgi header"

print "Content-Type: text/plain\n"
print "Content-Length: #{body.size}\n"
print "\n"
print body
Loading