Skip to content

Commit

Permalink
Rack 3 no longer required environments (#437)
Browse files Browse the repository at this point in the history
* when using Rack 3, don't add no longer required environments (rack.multithread/rack.multiprocess/rack.run_once/rack.version)
  • Loading branch information
and9000 authored Feb 16, 2025
1 parent dbf793a commit de6b618
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 25 deletions.
33 changes: 33 additions & 0 deletions lib/thin/env.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@

module Thin
module Env
def self.with_defaults(env)
if ::Rack.release >= "3"
rack_env_class = Rack3
else
rack_env_class = Rack2
end

rack_env_class.env.merge(env)
end
end

private

class Rack2
def self.env
{
::Thin::Request::RACK_VERSION => ::Thin::VERSION::RACK,
::Thin::Request::RACK_MULTITHREAD => false,
::Thin::Request::RACK_MULTIPROCESS => false,
::Thin::Request::RACK_RUN_ONCE => false
}
end
end

class Rack3
def self.env
{}
end
end
end
11 changes: 3 additions & 8 deletions lib/thin/request.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'tempfile'
require_relative './env'

module Thin
# Raised when an incoming request is not valid
Expand Down Expand Up @@ -55,20 +56,14 @@ def initialize
@data = String.new
@nparsed = 0
@body = StringIO.new(INITIAL_BODY.dup)
@env = {
@env = Env.with_defaults({
SERVER_SOFTWARE => SERVER,
SERVER_NAME => LOCALHOST,

# Rack stuff
RACK_INPUT => @body,

RACK_VERSION => VERSION::RACK,
RACK_ERRORS => STDERR,

RACK_MULTITHREAD => false,
RACK_MULTIPROCESS => false,
RACK_RUN_ONCE => false
}
})
end

# Parse a chunk of data into the request environment
Expand Down
48 changes: 31 additions & 17 deletions spec/request/parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,48 +19,48 @@
expect(request.env["rack.url_scheme"]).to eq('http')
expect(request.env['FRAGMENT'].to_s).to be_empty
expect(request.env['QUERY_STRING'].to_s).to be_empty

expect(request).to validate_with_lint
end

it 'should upcase headers' do
request = R("GET / HTTP/1.1\r\nX-Invisible: yo\r\n\r\n")
expect(request.env['HTTP_X_INVISIBLE']).to eq('yo')
end

it 'should not prepend HTTP_ to Content-Type and Content-Length' do
request = R("POST / HTTP/1.1\r\nHost: localhost\r\nContent-Type: text/html\r\nContent-Length: 2\r\n\r\naa")
expect(request.env.keys).not_to include('HTTP_CONTENT_TYPE', 'HTTP_CONTENT_LENGTH')
expect(request.env.keys).to include('CONTENT_TYPE', 'CONTENT_LENGTH')

expect(request).to validate_with_lint
end

it 'should raise error on invalid request line' do
expect { R("GET / SsUTF/1.1") }.to raise_error(InvalidRequest)
expect { R("GET / HTTP/1.1yousmelllikecheeze") }.to raise_error(InvalidRequest)
end

it 'should support fragment in uri' do
request = R("GET /forums/1/topics/2375?page=1#posts-17408 HTTP/1.1\r\nHost: localhost\r\n\r\n")

expect(request.env['REQUEST_URI']).to eq('/forums/1/topics/2375?page=1')
expect(request.env['PATH_INFO']).to eq('/forums/1/topics/2375')
expect(request.env['QUERY_STRING']).to eq('page=1')
expect(request.env['FRAGMENT']).to eq('posts-17408')

expect(request).to validate_with_lint
end

it 'should parse path with query string' do
request = R("GET /index.html?234235 HTTP/1.1\r\nHost: localhost\r\n\r\n")
expect(request.env['REQUEST_PATH']).to eq('/index.html')
expect(request.env['QUERY_STRING']).to eq('234235')
expect(request.env['FRAGMENT']).to be_nil

expect(request).to validate_with_lint
end

it 'should parse headers from GET request' do
request = R(<<-EOS, true)
GET / HTTP/1.1
Expand Down Expand Up @@ -151,7 +151,7 @@

expect(request).to validate_with_lint
end

it 'should parse even with stupid Content-Length' do
body = <<-EOS.chomp
POST / HTTP/1.1
Expand All @@ -165,7 +165,7 @@
request.body.rewind
expect(request.body.read).to eq('aye')
end

it "should parse ie6 urls" do
%w(/some/random/path"
/some/random/path>
Expand All @@ -184,14 +184,14 @@
expect(parser).not_to be_error
end
end

it "should parse absolute request URI" do
request = R(<<-EOS, true)
GET http://localhost:3000/hi?qs#f HTTP/1.1
Host: localhost:3000
EOS

expect(request.env['PATH_INFO']).to eq('/hi')
expect(request.env['REQUEST_PATH']).to eq('/hi')
expect(request.env['REQUEST_URI']).to eq('/hi?qs')
Expand Down Expand Up @@ -236,7 +236,7 @@
expect(req).to have_key('HTTP_HOS_T')
}
end

it "should parse PATH_INFO with semicolon" do
qs = "QUERY_STRING"
pi = "PATH_INFO"
Expand All @@ -257,14 +257,14 @@
expect(env["REQUEST_URI"]).to eq(uri)

next if uri == "*"

# Validate w/ Ruby's URI.parse
uri = URI.parse("http://example.com#{uri}")
expect(env[qs]).to eq(uri.query.to_s)
expect(env[pi]).to eq(uri.path)
end
end

it "should parse IE7 badly encoded URL" do
body = <<-EOS.chomp
GET /H%uFFFDhnchenbrustfilet HTTP/1.1
Expand All @@ -275,4 +275,18 @@

expect(request.env['REQUEST_URI']).to eq("/H%uFFFDhnchenbrustfilet")
end

describe "with Rack < 3", unless: ::Rack.release >= "3" do
it "should add required env" do
request = R("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n")
expect(request.env).to include("rack.version", "rack.multithread", "rack.multiprocess", "rack.run_once")
end
end

describe "with Rack >= 3", if: ::Rack.release >= "3" do
it "should not add not required env" do
request = R("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n")
expect(request.env).not_to include("rack.version", "rack.multithread", "rack.multiprocess", "rack.run_once")
end
end
end

0 comments on commit de6b618

Please sign in to comment.