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

Rework host/hostname/authority implementation. #1561

Merged
merged 11 commits into from Feb 7, 2020
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file. For info on
- `rack.session` request environment entry must respond to `to_hash` and return unfrozen Hash. ([@jeremyevans](https://github.com/jeremyevans))
- Request environment cannot be frozen. ([@jeremyevans](https://github.com/jeremyevans))
- CGI values in the request environment with non-ASCII characters must use ASCII-8BIT encoding. ([@jeremyevans](https://github.com/jeremyevans))
- Improve SPEC/lint relating to SERVER_NAME, SERVER_PORT and HTTP_HOST. ([#1561](https://github.com/rack/rack/pull/1561), [@ioquatix](https://github.com/ioquatix))

### Added

Expand Down Expand Up @@ -37,6 +38,7 @@ All notable changes to this project will be documented in this file. For info on
- `Rack::HeaderHash` is memoized by default. ([#1549](https://github.com/rack/rack/pull/1549), [@ioquatix](https://github.com/ioquatix))
- `Rack::Directory` allow directory traversal inside root directory. ([#1417](https://github.com/rack/rack/pull/1417), [@ThomasSevestre](https://github.com/ThomasSevestre))
- Sort encodings by server preference. ([#1184](https://github.com/rack/rack/pull/1184), [@ioquatix](https://github.com/ioquatix), [@wjordan](https://github.com/wjordan))
- Rework host/hostname/authority implementation in `Rack::Request`. `#host` and `#host_with_port` have been changed to correctly return IPv6 addresses formatted with square brackets, as defined by [RFC3986](https://tools.ietf.org/html/rfc3986#section-3.2.2). ([#1561](https://github.com/rack/rack/pull/1561), [@ioquatix](https://github.com/ioquatix))

### Removed

Expand Down
1 change: 1 addition & 0 deletions lib/rack.rb
Expand Up @@ -15,6 +15,7 @@

module Rack
HTTP_HOST = 'HTTP_HOST'
HTTP_PORT = 'HTTP_PORT'
HTTP_VERSION = 'HTTP_VERSION'
HTTPS = 'HTTPS'
PATH_INFO = 'PATH_INFO'
Expand Down
31 changes: 24 additions & 7 deletions lib/rack/lint.rb
Expand Up @@ -117,17 +117,19 @@ def check_env(env)
## follows the <tt>?</tt>, if any. May be
## empty, but is always required!

## <tt>SERVER_NAME</tt>, <tt>SERVER_PORT</tt>::
## When combined with <tt>SCRIPT_NAME</tt> and
## <tt>SERVER_NAME</tt>:: When combined with <tt>SCRIPT_NAME</tt> and
## <tt>PATH_INFO</tt>, these variables can be
## used to complete the URL. Note, however,
## that <tt>HTTP_HOST</tt>, if present,
## should be used in preference to
## <tt>SERVER_NAME</tt> for reconstructing
## the request URL.
## <tt>SERVER_NAME</tt> and <tt>SERVER_PORT</tt>
## can never be empty strings, and so
## are always required.
## <tt>SERVER_NAME</tt> can never be an empty
## string, and so is always required.

## <tt>SERVER_PORT</tt>:: An optional +Integer+ which is the port the
## server is running on. Should be specified if
## the server is running on a non-standard port.

## <tt>HTTP_</tt> Variables:: Variables corresponding to the
## client-supplied HTTP request
Expand Down Expand Up @@ -271,13 +273,28 @@ def check_env(env)
## accepted specifications and must not be used otherwise.
##

%w[REQUEST_METHOD SERVER_NAME SERVER_PORT
QUERY_STRING
%w[REQUEST_METHOD SERVER_NAME QUERY_STRING
rack.version rack.input rack.errors
rack.multithread rack.multiprocess rack.run_once].each { |header|
assert("env missing required key #{header}") { env.include? header }
}

## The <tt>SERVER_PORT</tt> must be an Integer if set.
assert("env[SERVER_PORT] is not an Integer") do
server_port = env["SERVER_PORT"]
server_port.nil? || (Integer(server_port) rescue false)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we state that SERVER_PORT must be an integer, we should do env["SERVER_PORT"].is_a?(Integer). I don't think we need to check that it is convertable to an integer, unless we want that to be the spec.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, makes sense, I'll update the SPEC.

end

## The <tt>SERVER_NAME</tt> must be a valid authority as defined by RFC7540.
assert("#{env[SERVER_NAME]} must be a valid authority") do
URI.parse("http://#{env[SERVER_NAME]}/") rescue false
end

## The <tt>HTTP_HOST</tt> must be a valid authority as defined by RFC7540.
ioquatix marked this conversation as resolved.
Show resolved Hide resolved
assert("#{env[HTTP_HOST]} must be a valid authority") do
URI.parse("http://#{env[HTTP_HOST]}/") rescue false
end

## The environment must not contain the keys
## <tt>HTTP_CONTENT_TYPE</tt> or <tt>HTTP_CONTENT_LENGTH</tt>
## (use the versions without <tt>HTTP_</tt>).
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/recursive.rb
Expand Up @@ -19,7 +19,7 @@ def initialize(url, env = {})
@env[PATH_INFO] = @url.path
@env[QUERY_STRING] = @url.query if @url.query
@env[HTTP_HOST] = @url.host if @url.host
@env["HTTP_PORT"] = @url.port if @url.port
@env[HTTP_PORT] = @url.port if @url.port
@env[RACK_URL_SCHEME] = @url.scheme if @url.scheme

super "forwarding to #{url}"
Expand Down