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.

Sign up for GitHub

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

Jump to bottom

Rack 3 support #758

Merged
merged 7 commits into from
Sep 13, 2022
Merged

Rack 3 support #758

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
61dda4e
bump version of rack-test to allow for rack-3
lsylvester Sep 8, 2022
0f4783e
relax rack
lsylvester Sep 8, 2022
12e5fd9
require rack/lint
lsylvester Sep 8, 2022
0692518
lower case headers
lsylvester Sep 8, 2022
17d7723
Add Changelog entry
lsylvester Sep 8, 2022
8d2862e
unsure that we are using a version of rack that supports lower case h…
lsylvester Sep 10, 2022
a9f9ebb
update changelog entry to make it explict that the headers will change
lsylvester Sep 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
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 [#758](https://github.com/rails/sprockets/pull/758)

## 4.1.0

Expand Down
36 changes: 18 additions & 18 deletions lib/sprockets/server.rb
View file
Open in desktop
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
View file
Open in desktop
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", "> 1", "< 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
View file
Open in desktop
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
View file
Open in desktop
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