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

Add rack.early_hints to SPEC #1831

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -23,6 +23,7 @@ All notable changes to this project will be documented in this file. For info on
- `rack.hijack?` (partial hijack) and `rack.hijack` (full hijack) are now independently optional.
- `rack.hijack_io` has been removed completely.
- `rack.response_finished` is an optional environment key which contains an array of callable objects that must accept `#call(env, status, headers, error)` and are invoked after the response is finished (either successfully or unsucessfully).
- rack.early_hints is now officially supported as an optional feature (already implemented by Unicorn, Puma, and Falcon).
jeremyevans marked this conversation as resolved.
Show resolved Hide resolved

### Removed

Expand Down
11 changes: 11 additions & 0 deletions SPEC.rdoc
Expand Up @@ -124,6 +124,7 @@ There are the following restrictions:
* There must be a valid input stream in <tt>rack.input</tt>.
* There must be a valid error stream in <tt>rack.errors</tt>.
* There may be a valid hijack callback in <tt>rack.hijack</tt>
* There may be a valid early hints callback in <tt>rack.early_hints</tt>
* The <tt>REQUEST_METHOD</tt> must be a valid token.
* The <tt>SCRIPT_NAME</tt>, if non-empty, must start with <tt>/</tt>
* The <tt>PATH_INFO</tt>, if non-empty, must start with <tt>/</tt>
Expand Down Expand Up @@ -227,6 +228,16 @@ instance is recommended.

The special response header +rack.hijack+ must only be set
if the request +env+ has a truthy +rack.hijack?+.

=== Early Hints

The application or any middleware may call the <tt>rack.early_hints</tt>
with an object which would be valid as the headers of a Rack response.

If <tt>rack.early_hints</tt> is present, it must respond to #call.
If <tt>rack.early_hints</tt> is called, it must be called with
valid Rack response headers.

== The Response

=== The Status
Expand Down
1 change: 1 addition & 0 deletions lib/rack/constants.rb
Expand Up @@ -39,6 +39,7 @@ module Rack
# Rack environment variables
RACK_VERSION = 'rack.version'
RACK_TEMPFILES = 'rack.tempfiles'
RACK_EARLY_HINTS = 'rack.early_hints'
RACK_ERRORS = 'rack.errors'
RACK_LOGGER = 'rack.logger'
RACK_INPUT = 'rack.input'
Expand Down
26 changes: 26 additions & 0 deletions lib/rack/lint.rb
Expand Up @@ -334,6 +334,8 @@ def check_environment(env)
check_error env[RACK_ERRORS]
## * There may be a valid hijack callback in <tt>rack.hijack</tt>
check_hijack env
## * There may be a valid early hints callback in <tt>rack.early_hints</tt>
check_early_hints env

## * The <tt>REQUEST_METHOD</tt> must be a valid token.
unless env[REQUEST_METHOD] =~ /\A[0-9A-Za-z!\#$%&'*+.^_`|~-]+\z/
Expand Down Expand Up @@ -608,6 +610,30 @@ def check_hijack_response(headers, env)
nil
end

##
## === Early Hints
##
## The application or any middleware may call the <tt>rack.early_hints</tt>
## with an object which would be valid as the headers of a Rack response.
def check_early_hints(env)
if env[RACK_EARLY_HINTS]
##
## If <tt>rack.early_hints</tt> is present, it must respond to #call.
unless env[RACK_EARLY_HINTS].respond_to?(:call)
raise LintError, "rack.early_hints must respond to call"
end

original_callback = env[RACK_EARLY_HINTS]
env[RACK_EARLY_HINTS] = lambda do |headers|
## If <tt>rack.early_hints</tt> is called, it must be called with
## valid Rack response headers.
check_headers(headers)
original_callback.call(headers)
end
end
end

##
## == The Response
##
## === The Status
Expand Down
52 changes: 52 additions & 0 deletions test/spec_lint.rb
Expand Up @@ -437,6 +437,58 @@ def obj.each; end

end

it "notice rack.early_hints errors" do
def self.env(arg={})
super(arg.merge("rack.early_hints" => proc{}))
end
def self.app(value)
app = Rack::Lint.new(lambda { |env|
env['rack.early_hints'].call(value)
[200, {}, []]
})
lambda { app.call(env) }
end

app(Object.new).must_raise(Rack::Lint::LintError).
message.must_equal "headers object should be a hash, but isn't (got Object as headers)"

app({}.freeze).must_raise(Rack::Lint::LintError).
message.must_equal "headers object should not be frozen, but is"

app(true => false).must_raise(Rack::Lint::LintError).
message.must_equal "header key must be a string, was TrueClass"

app("status" => "404").must_raise(Rack::Lint::LintError).
message.must_match(/must not contain status/)

invalid_headers = 0.upto(31).map(&:chr) + %W<( ) , / : ; < = > ? @ [ \\ ] { } \x7F>
invalid_headers.each do |invalid_header|
app(invalid_header => "text/plain").
must_raise(Rack::Lint::LintError, "on invalid header: #{invalid_header}").
message.must_equal("invalid header name: #{invalid_header}")
end

('A'..'Z').each do |invalid_header|
app(invalid_header => "text/plain").
must_raise(Rack::Lint::LintError, "on invalid header: #{invalid_header}").
message.must_equal("uppercase character in header name: #{invalid_header}")
end

valid_headers = 0.upto(127).map(&:chr) - invalid_headers - ('A'..'Z').to_a
valid_headers.each do |valid_header|
app(valid_header => "text/plain").call.first.must_equal 200
end

app("foo" => Object.new).must_raise(Rack::Lint::LintError).
message.must_equal "a header value must be a String or Array of Strings, but the value of 'foo' is a Object"

app("foo-bar" => "text\000plain").must_raise(Rack::Lint::LintError).
message.must_match(/invalid header/)

app([%w(content-type text/plain), %w(content-length 0)]).must_raise(Rack::Lint::LintError).
message.must_equal "headers object should be a hash, but isn't (got Array as headers)"
end

it "notice content-type errors" do
# lambda {
# Rack::Lint.new(lambda { |env|
Expand Down