Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rack 3 support #758

Merged
merged 7 commits into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Get upgrade notes from Sprockets 3.x to 4.x at https://github.com/rails/sprockets/blob/master/UPGRADING.md

- Fix `Sprockets::Server` to return response headers to compatible with with Rack::Lint 2.0.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a reverting of the lower case headers which caused a performance issue - but this was resolved in rack 2.2.4 according to #746 (comment)

- Add support for Rack 3.0. Headers set by sprockets will now be lower case. [#758](https://github.com/rails/sprockets/pull/758)

## 4.1.0

Expand Down
36 changes: 18 additions & 18 deletions lib/sprockets/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,39 +148,39 @@ def not_modified_response(env, etag)
# Returns a 400 Forbidden response tuple
def bad_request_response(env)
if head_request?(env)
[ 400, { "Content-Type" => "text/plain", "Content-Length" => "0" }, [] ]
[ 400, { "content-type" => "text/plain", "content-length" => "0" }, [] ]
else
[ 400, { "Content-Type" => "text/plain", "Content-Length" => "11" }, [ "Bad Request" ] ]
[ 400, { "content-type" => "text/plain", "content-length" => "11" }, [ "Bad Request" ] ]
end
end

# Returns a 403 Forbidden response tuple
def forbidden_response(env)
if head_request?(env)
[ 403, { "Content-Type" => "text/plain", "Content-Length" => "0" }, [] ]
[ 403, { "content-type" => "text/plain", "content-length" => "0" }, [] ]
else
[ 403, { "Content-Type" => "text/plain", "Content-Length" => "9" }, [ "Forbidden" ] ]
[ 403, { "content-type" => "text/plain", "content-length" => "9" }, [ "Forbidden" ] ]
end
end

# Returns a 404 Not Found response tuple
def not_found_response(env)
if head_request?(env)
[ 404, { "Content-Type" => "text/plain", "Content-Length" => "0", "X-Cascade" => "pass" }, [] ]
[ 404, { "content-type" => "text/plain", "content-length" => "0", "x-cascade" => "pass" }, [] ]
else
[ 404, { "Content-Type" => "text/plain", "Content-Length" => "9", "X-Cascade" => "pass" }, [ "Not found" ] ]
[ 404, { "content-type" => "text/plain", "content-length" => "9", "x-cascade" => "pass" }, [ "Not found" ] ]
end
end

def method_not_allowed_response
[ 405, { "Content-Type" => "text/plain", "Content-Length" => "18" }, [ "Method Not Allowed" ] ]
[ 405, { "content-type" => "text/plain", "content-length" => "18" }, [ "Method Not Allowed" ] ]
end

def precondition_failed_response(env)
if head_request?(env)
[ 412, { "Content-Type" => "text/plain", "Content-Length" => "0", "X-Cascade" => "pass" }, [] ]
[ 412, { "content-type" => "text/plain", "content-length" => "0", "x-cascade" => "pass" }, [] ]
else
[ 412, { "Content-Type" => "text/plain", "Content-Length" => "19", "X-Cascade" => "pass" }, [ "Precondition Failed" ] ]
[ 412, { "content-type" => "text/plain", "content-length" => "19", "x-cascade" => "pass" }, [ "Precondition Failed" ] ]
end
end

Expand All @@ -189,7 +189,7 @@ def precondition_failed_response(env)
def javascript_exception_response(exception)
err = "#{exception.class.name}: #{exception.message}\n (in #{exception.backtrace[0]})"
body = "throw Error(#{err.inspect})"
[ 200, { "Content-Type" => "application/javascript", "Content-Length" => body.bytesize.to_s }, [ body ] ]
[ 200, { "content-type" => "application/javascript", "content-length" => body.bytesize.to_s }, [ body ] ]
end

# Returns a CSS response that hides all elements on the page and
Expand Down Expand Up @@ -242,7 +242,7 @@ def css_exception_response(exception)
}
CSS

[ 200, { "Content-Type" => "text/css; charset=utf-8", "Content-Length" => body.bytesize.to_s }, [ body ] ]
[ 200, { "content-type" => "text/css; charset=utf-8", "content-length" => body.bytesize.to_s }, [ body ] ]
end

# Escape special characters for use inside a CSS content("...") string
Expand All @@ -258,18 +258,18 @@ def cache_headers(env, etag)
headers = {}

# Set caching headers
headers["Cache-Control"] = +"public"
headers["ETag"] = %("#{etag}")
headers["cache-control"] = +"public"
headers["etag"] = %("#{etag}")

# If the request url contains a fingerprint, set a long
# expires on the response
if path_fingerprint(env["PATH_INFO"])
headers["Cache-Control"] << ", max-age=31536000, immutable"
headers["cache-control"] << ", max-age=31536000, immutable"

# Otherwise set `must-revalidate` since the asset could be modified.
else
headers["Cache-Control"] << ", must-revalidate"
headers["Vary"] = "Accept-Encoding"
headers["cache-control"] << ", must-revalidate"
headers["vary"] = "Accept-Encoding"
end

headers
Expand All @@ -279,15 +279,15 @@ def headers(env, asset, length)
headers = {}

# Set content length header
headers["Content-Length"] = length.to_s
headers["content-length"] = length.to_s

# Set content type header
if type = asset.content_type
# Set charset param for text/* mime types
if type.start_with?("text/") && asset.charset
type += "; charset=#{asset.charset}"
end
headers["Content-Type"] = type
headers["content-type"] = type
end

headers.merge(cache_headers(env, asset.etag))
Expand Down
4 changes: 2 additions & 2 deletions sprockets.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Gem::Specification.new do |s|
s.files = Dir["README.md", "CHANGELOG.md", "MIT-LICENSE", "lib/**/*.rb"]
s.executables = ["sprockets"]

s.add_dependency "rack", "> 1", "< 3"
s.add_dependency "rack", ">= 2.2.4", "< 4"
s.add_dependency "concurrent-ruby", "~> 1.0"

s.add_development_dependency "m", ">= 0"
Expand All @@ -28,7 +28,7 @@ Gem::Specification.new do |s|
s.add_development_dependency "timecop", "~> 0.9.1"
s.add_development_dependency "minitest", "~> 5.0"
s.add_development_dependency "nokogiri", "~> 1.3"
s.add_development_dependency "rack-test", "~> 0.6"
s.add_development_dependency "rack-test", "~> 2.0.0"
s.add_development_dependency "rake", "~> 12.0"
s.add_development_dependency "sass", "~> 3.4"
s.add_development_dependency "sassc", "~> 2.0"
Expand Down
1 change: 1 addition & 0 deletions test/sprockets_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "sprockets/environment"
require "fileutils"
require "timecop"
require "rack/lint"

old_verbose, $VERBOSE = $VERBOSE, false
Encoding.default_external = 'UTF-8'
Expand Down
76 changes: 38 additions & 38 deletions test/test_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ def app
test "serve single source file" do
get "/assets/foo.js"
assert_equal 200, last_response.status
assert_equal "9", last_response.headers['Content-Length']
assert_equal "Accept-Encoding", last_response.headers['Vary']
assert_equal "9", last_response.headers['content-length']
assert_equal "Accept-Encoding", last_response.headers['vary']
assert_equal "var foo;\n", last_response.body
end

test "serve single self file" do
get "/assets/foo.self.js"
assert_equal 200, last_response.status
assert_equal "9", last_response.headers['Content-Length']
assert_equal "9", last_response.headers['content-length']
assert_equal "var foo;\n", last_response.body
end

Expand All @@ -64,30 +64,30 @@ def app
assert_equal 200, last_response.status
assert_equal "\n(function() {\n application.boot();\n})();\n",
last_response.body
assert_equal "43", last_response.headers['Content-Length']
assert_equal "43", last_response.headers['content-length']
end

test "serve source with content type headers" do
get "/assets/application.js"
assert_equal "application/javascript", last_response.headers['Content-Type']
assert_equal "application/javascript", last_response.headers['content-type']

get "/assets/bootstrap.css"
assert_equal "text/css; charset=utf-8", last_response.headers['Content-Type']
assert_equal "text/css; charset=utf-8", last_response.headers['content-type']
end

test "serve source with etag headers" do
digest = @env['application.js'].etag

get "/assets/application.js"
assert_equal "\"#{digest}\"",
last_response.headers['ETag']
last_response.headers['etag']
end

test "not modified partial response when if-none-match etags match" do
get "/assets/application.js"
assert_equal 200, last_response.status
etag, cache_control, expires, vary = last_response.headers.values_at(
'ETag', 'Cache-Control', 'Expires', 'Vary'
'etag', 'cache-control', 'expires', 'vary'
)

assert_nil expires
Expand All @@ -97,31 +97,31 @@ def app
assert_equal 304, last_response.status

# Allow 304 headers
assert_equal cache_control, last_response.headers['Cache-Control']
assert_equal etag, last_response.headers['ETag']
assert_equal cache_control, last_response.headers['cache-control']
assert_equal etag, last_response.headers['etag']
assert_nil last_response.headers['Expires']
assert_equal vary, last_response.headers['Vary']
assert_equal vary, last_response.headers['vary']

# Disallowed 304 headers
refute last_response.headers['Content-Type']
refute last_response.headers['Content-Length']
refute last_response.headers['Content-Encoding']
refute last_response.headers['content-type']
refute last_response.headers['content-length']
refute last_response.headers['content-encoding']
end

test "response when if-none-match etags don't match" do
get "/assets/application.js", {},
'HTTP_IF_NONE_MATCH' => "nope"

assert_equal 200, last_response.status
assert_equal '"b452c9ae1d5c8d9246653e0d93bc83abce0ee09ef725c0f0a29a41269c217b83"', last_response.headers['ETag']
assert_equal '52', last_response.headers['Content-Length']
assert_equal '"b452c9ae1d5c8d9246653e0d93bc83abce0ee09ef725c0f0a29a41269c217b83"', last_response.headers['etag']
assert_equal '52', last_response.headers['content-length']
end

test "not modified partial response with fingerprint and if-none-match etags match" do
get "/assets/application.js"
assert_equal 200, last_response.status

etag = last_response.headers['ETag']
etag = last_response.headers['etag']
digest = etag[/"(.+)"/, 1]

get "/assets/application-#{digest}.js", {},
Expand All @@ -133,7 +133,7 @@ def app
get "/assets/prehashed-988881adc9fc3655077dc2d4d757d480b5ea0e11.js"
assert_equal 200, last_response.status

etag = last_response.headers['ETag']
etag = last_response.headers['etag']
digest = etag[/"(.+)"/, 1]

assert_equal 'edabfd0f1ac5fcdae82cc7d92d1c52abb671797a3948fa9040aec1db8e61c327', digest
Expand All @@ -143,7 +143,7 @@ def app
get "/assets/esbuild-TQDC3LZV.digested.js"
assert_equal 200, last_response.status

etag = last_response.headers['ETag']
etag = last_response.headers['etag']
digest = etag[/"(.+)"/, 1]

assert_equal '3ebac3dc00b383de6cbdfa470d105f5a9f22708fb72c63db917ad37f288ac708', digest
Expand All @@ -153,7 +153,7 @@ def app
get "/assets/application.js"
assert_equal 200, last_response.status

etag = last_response.headers['ETag']
etag = last_response.headers['etag']
digest = etag[/"(.+)"/, 1]

get "/assets/application-#{digest}.js", {},
Expand All @@ -177,7 +177,7 @@ def app
get "/assets/application.js"
assert_equal 200, last_response.status

etag = last_response.headers['ETag']
etag = last_response.headers['etag']

get "/assets/application-0000000000000000000000000000000000000000.js", {},
'HTTP_IF_NONE_MATCH' => etag
Expand All @@ -187,22 +187,22 @@ def app
test "ok partial response when if-match etags match" do
get "/assets/application.js"
assert_equal 200, last_response.status
etag = last_response.headers['ETag']
etag = last_response.headers['etag']

get "/assets/application.js", {},
'HTTP_IF_MATCH' => etag

assert_equal 200, last_response.status
assert_equal '"b452c9ae1d5c8d9246653e0d93bc83abce0ee09ef725c0f0a29a41269c217b83"', last_response.headers['ETag']
assert_equal '52', last_response.headers['Content-Length']
assert_equal '"b452c9ae1d5c8d9246653e0d93bc83abce0ee09ef725c0f0a29a41269c217b83"', last_response.headers['etag']
assert_equal '52', last_response.headers['content-length']
end

test "precondition failed with if-match is a mismatch" do
get "/assets/application.js", {},
'HTTP_IF_MATCH' => '"000"'
assert_equal 412, last_response.status

refute last_response.headers['ETag']
refute last_response.headers['etag']
end

test "not found with if-match" do
Expand All @@ -225,23 +225,23 @@ def app

test "fingerprint digest sets expiration to the future" do
get "/assets/application.js"
digest = last_response.headers['ETag'][/"(.+)"/, 1]
digest = last_response.headers['etag'][/"(.+)"/, 1]

get "/assets/application-#{digest}.js"
assert_equal 200, last_response.status
assert_match %r{max-age}, last_response.headers['Cache-Control']
assert_match %r{immutable}, last_response.headers['Cache-Control']
assert_match %r{max-age}, last_response.headers['cache-control']
assert_match %r{immutable}, last_response.headers['cache-control']
end

test "fingerprint digest of file self" do
get "/assets/application.self.js"
digest = last_response.headers['ETag'][/"(.+)"/, 1]
digest = last_response.headers['etag'][/"(.+)"/, 1]

get "/assets/application.self-#{digest}.js"
assert_equal 200, last_response.status
assert_equal "\n(function() {\n application.boot();\n})();\n", last_response.body
assert_equal "43", last_response.headers['Content-Length']
assert_match %r{max-age}, last_response.headers['Cache-Control']
assert_equal "43", last_response.headers['content-length']
assert_match %r{max-age}, last_response.headers['cache-control']
end

test "bad fingerprint digest returns a 404" do
Expand All @@ -250,14 +250,14 @@ def app

head "/assets/application-0000000000000000000000000000000000000000.js"
assert_equal 404, last_response.status
assert_equal "0", last_response.headers['Content-Length']
assert_equal "0", last_response.headers['content-length']
assert_equal "", last_response.body
end

test "missing source" do
get "/assets/none.js"
assert_equal 404, last_response.status
assert_equal "pass", last_response.headers['X-Cascade']
assert_equal "pass", last_response.headers['x-cascade']
end

test "re-throw JS exceptions in the browser" do
Expand Down Expand Up @@ -302,7 +302,7 @@ def app

head "/assets/.-0000000./etc/passwd"
assert_equal 403, last_response.status
assert_equal "0", last_response.headers['Content-Length']
assert_equal "0", last_response.headers['content-length']
assert_equal "", last_response.body
end

Expand Down Expand Up @@ -336,8 +336,8 @@ def app
test "serving static assets" do
get "/assets/logo.png"
assert_equal 200, last_response.status
assert_equal "image/png", last_response.headers['Content-Type']
refute last_response.headers['Content-Encoding']
assert_equal "image/png", last_response.headers['content-type']
refute last_response.headers['content-encoding']
assert_equal File.binread(fixture_path("server/app/images/logo.png")), last_response.body
end

Expand All @@ -347,8 +347,8 @@ def app

head "/assets/foo.js"
assert_equal 200, last_response.status
assert_equal "application/javascript", last_response.headers['Content-Type']
assert_equal "0", last_response.headers['Content-Length']
assert_equal "application/javascript", last_response.headers['content-type']
assert_equal "0", last_response.headers['content-length']
assert_equal "", last_response.body

post "/assets/foo.js"
Expand Down