Skip to content

Commit

Permalink
Fix header casing compatibility with Rails 7
Browse files Browse the repository at this point in the history
To support Rack 3, response headers in `Sprockets::Server` were all
downcased. However, this has led to issues with Rack 2 applications (ex.
Rails 7) since they still expect mixed case (ex. `Content-Type`)
headers.

To ensure compatibility with both Rack 2 and Rack 3 applications, this
commit makes the casing of the headers conditional on the Rack version.
Rack itself provides constants to do this easily for most of the headers
used (`Content-Type`, `Content-Length`, `Cache-Control`, and `ETag`) and
the rest are added as constants under `Rack::Server`.

As an alternative to this, the responses could instead be wrapped using
`Rack::Headers` (and `Rack::Utils::HeaderHash` in Rack 2), but making
the header casing conditional seems better to me because it is
relatively easier to implement and there will be less churn if/when Rack
2 support is eventually removed.
  • Loading branch information
skipkayhil committed Aug 10, 2023
1 parent 6554b6d commit 5d795a7
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Get upgrade notes from Sprockets 3.x to 4.x at https://github.com/rails/sprockets/blob/master/UPGRADING.md

- Fix for precompile issues when multiple extensions map to the same MIME type (eg. `.jpeg` / `.jpg`). [#781](https://github.com/rails/sprockets/pull/781)
- Fix compatibility with Rack 2 applications. [#790](https://github.com/rails/sprockets/pull/790)

## 4.2.0

Expand Down
48 changes: 29 additions & 19 deletions lib/sprockets/server.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true
require 'set'
require 'time'
require 'rack/utils'
require 'rack'

module Sprockets
# `Server` is a concern mixed into `Environment` and
Expand All @@ -11,6 +11,16 @@ module Server
# Supported HTTP request methods.
ALLOWED_REQUEST_METHODS = ['GET', 'HEAD'].to_set.freeze

# :stopdoc:
if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
X_CASCADE = "X-Cascade"
VARY = "Vary"
else
X_CASCADE = "x-cascade"
VARY = "vary"
end
# :startdoc:

# `call` implements the Rack 1.x specification which accepts an
# `env` Hash and returns a three item tuple with the status code,
# headers, and body.
Expand Down Expand Up @@ -148,39 +158,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, { Rack::CONTENT_TYPE => "text/plain", Rack::CONTENT_LENGTH => "0" }, [] ]
else
[ 400, { "content-type" => "text/plain", "content-length" => "11" }, [ "Bad Request" ] ]
[ 400, { Rack::CONTENT_TYPE => "text/plain", Rack::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, { Rack::CONTENT_TYPE => "text/plain", Rack::CONTENT_LENGTH => "0" }, [] ]
else
[ 403, { "content-type" => "text/plain", "content-length" => "9" }, [ "Forbidden" ] ]
[ 403, { Rack::CONTENT_TYPE => "text/plain", Rack::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, { Rack::CONTENT_TYPE => "text/plain", Rack::CONTENT_LENGTH => "0", X_CASCADE => "pass" }, [] ]
else
[ 404, { "content-type" => "text/plain", "content-length" => "9", "x-cascade" => "pass" }, [ "Not found" ] ]
[ 404, { Rack::CONTENT_TYPE => "text/plain", Rack::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, { Rack::CONTENT_TYPE => "text/plain", Rack::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, { Rack::CONTENT_TYPE => "text/plain", Rack::CONTENT_LENGTH => "0", X_CASCADE => "pass" }, [] ]
else
[ 412, { "content-type" => "text/plain", "content-length" => "19", "x-cascade" => "pass" }, [ "Precondition Failed" ] ]
[ 412, { Rack::CONTENT_TYPE => "text/plain", Rack::CONTENT_LENGTH => "19", X_CASCADE => "pass" }, [ "Precondition Failed" ] ]
end
end

Expand All @@ -189,7 +199,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, { Rack::CONTENT_TYPE => "application/javascript", Rack::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 +252,7 @@ def css_exception_response(exception)
}
CSS

[ 200, { "content-type" => "text/css; charset=utf-8", "content-length" => body.bytesize.to_s }, [ body ] ]
[ 200, { Rack::CONTENT_TYPE => "text/css; charset=utf-8", Rack::CONTENT_LENGTH => body.bytesize.to_s }, [ body ] ]
end

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

# Set caching headers
headers["cache-control"] = +"public"
headers["etag"] = %("#{etag}")
headers[Rack::CACHE_CONTROL] = +"public"
headers[Rack::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[Rack::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[Rack::CACHE_CONTROL] << ", must-revalidate"
headers[VARY] = "Accept-Encoding"
end

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

# Set content length header
headers["content-length"] = length.to_s
headers[Rack::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[Rack::CONTENT_TYPE] = type
end

headers.merge(cache_headers(env, asset.etag))
Expand Down

0 comments on commit 5d795a7

Please sign in to comment.