| Date | Commit message (Collapse) |
|---|
|
Remove the use of BOUNDARY_REGEX, as well as the @boundary, @full_boundary,
and @end_boundary instance variables. Replace them with the existing
@body_regex, which already contains the boundary. Tweak body_regex slightly
so the preceding EOF is required if we aren't at the start of the buffer,
since boundaries must be on their own lines.
When looking for the boundary, scan until using @body_regex (i.e. until you
found the boundary). Differentiate the full_boundary from the end_boundary
by seeing if the scanned data ends with \r\n (full boundary will,
end_boundary must end with --). If no boundary is found, clear the buffer
and try again, for behavior similar to previous.
For the fast forward state, if the first boundary found is the end boundary,
ignore it and keep scanning. We could alternatively raise an exception, not
sure if that is better. The previous behavior of treating the end boundary
as the starting boundary was definitely wrong, though.
Add some comments describing this boundary parsing behavior (which was my
initial intent when working on this).
Speeds up test/spec_multipart.rb more than 6x in my testing.
|
|
This fixes a shell escape issue
[CVE-2022-30123]
|
|
This commit restricts broken mime parsing to deal with a ReDOS
vulnerability.
[CVE-2022-30122]
|
|
It's already skipped on Ruby 2.4 and 2.5, so it's not like we
expect this to pass in all cases anyway. If it is fixed on JRuby,
we can renable. But it's better to not generate false negatives
in CI for unrelated changes.
|
|
This limit comes from RFC 1521 Section 7.2.1. Clients generally
do not use boundaries longer than 55 characters.
|
|
|
|
Coverage after this commit:
3273 relevant lines, 3273 lines covered and 0 lines missed. ( 100.0% )
1083 total branches, 1083 branches covered and 0 branches missed. ( 100.0% )
|
|
This is simpler and faster.
|
|
Mime.mime_type always returns a value. The only way it wouldn't
is if MIME_TYPES was modified to have an entry with a nil/false
value, which would seem to violate the expectations of
Mime.mime_type, which is documented to return a string if the
mime type is found.
|
|
We no longer support Ruby 1.8.
|
|
Previously, this would raise an error for invalid charsets, but that
is unlikely to be desired behavior.
While here, inline the CHARSET constant, since it is only used in
a single case, and we are using frozen string literals.
|
|
This is the only remaining parser state. It's impossible to
hit the else condition unless manually mutating the parser state.
|
|
This is in a private method, and the private method is only called
if file.respond_to?(:original_filename).
|
|
Rack::Multipart was always already defined at the point these
were called, since they are nested inside a Rack::Multipart module
opening.
|
|
uri.path is always set to a non-empty value starting with /
earlier in this method.
|
|
Conditional immediately preceding this checks for ranges.empty?,
and if it isn't empty, it definitely has a size greater than or
equal to 1.
|
|
|
|
The conditional with the dead code is:
```ruby
raise EOFError, "bad content body" if @sbuf.rest_size >= @bufsize
```
The dead code would be executed when consume_boundary fails in
handle_fast_forward, if the remaining data in the string scanner
is larger than the buffer size.
This branch appears to be impossible to hit. However, we want to
be very sure of that before removing it.
As some background, FAST_FORWARD is the initial parser state, and
is never reentered. The processing of the initial state in the
state machine is basically:
```ruby
read_data
read_data until handle_fast_forward == :MIME_HEAD
```
handle_fast_forward calls consume_boundary, which does
scan_until(BOUNDARY_REGEX) on the read data. BOUNDARY_REGEX is
`/\A([^\n]*(?:\n|\Z))/`, which matches all strings. It matches
up to the first newline if there is a newline, or the entire
string if there is not a newline.
After the scan, it strips the read data, and then checks if it
matches the boundary. If it matches the boundary, then there
is a state transition and the conditional is not reached.
If it doesn't match the boundary, if there is data left in the
string scanner, it loops and rescans using BOUNDARY_REGEX, which
again, matches all strings. The loop in consume_boundary basically
makes sure that either the boundary is matched, or there is
no data left in the string scanner.
If there is no data left in the string scanner, then the loop
exits. In that case, the branch is reached, but `@sbuf.rest_size`
is always 0.
To confirm that `@sbuf.rest_size` is always 0 before the conditional,
I added the following code before the conditional:
```ruby
abort("non-0 after consume_boundary") unless @sbuf.rest_size == 0
```
All tests still passed with that code. Note that the only test
where the conditional was even reached was the insanely long
boundary test.
If you try to force hitting the dead code by setting the buffer
size to 0, you get an EmptyContentError raised in read_data before
starting the state machine.
Looking at the history, this code initially comes from
f95113402b7239f225282806673e1b6424522b18, where it was possible to
hit the code because the equivalent of BOUNDARY_REGEX didn't use
\Z (the regex used was was `/\A([^\n]*\n)/`). The \Z wasn't added
until 360d374ab1a263fcf81a18d6e5ac74cccccbc7e9, and the dead branch
has probably not been possible to hit since.
|
|
Change error message for .ru file with embedded options, since it's not
just deprecated, the support has been fully removed.
Coverage after this commit:
3282 relevant lines, 3282 lines covered and 0 lines missed. ( 100.0% )
1110 total branches, 1068 branches covered and 42 branches missed. ( 96.22% )
|
|
Remove dead code in _normalize_params. There are two different types of
dead code. First, directly before this dead code, you have
`v ||= String.new`, so `!v.nil?` is always true and could be removed.
The remaining conditions for the dead branch are `k.empty?` and
`name = '[]'`. Looking at the conditional above, it's never possible
for these two conditions to be simultaneously true:
```ruby
if !name
# name != '[]'
elsif depth == 0
if start = name.index('[', 1)
k = name[0, start]
# !k.empty?
else
k = name
# !k.empty? || name != '[]'
end
elsif name.start_with?('[]')
k = '[]'
# !k.empty?
else # all remaining branches
# name != '[]', otherwise previous branch taken
end
```
Coverage after this commit:
3283 relevant lines, 3282 lines covered and 1 lines missed. ( 99.97% )
1112 total branches, 1068 branches covered and 44 branches missed. ( 96.04% )
|
|
|
|
Only handlers for WEBrick and CGI are included with Rack,
the other handlers were removed.
See #1584, commit 98d9cf5834d4e27e34bbaa017cdfc68795763b55.
|
|
|
|
The comment says this should be removed in Rack 3. This was added
in Rack 2.2, so it should be safe to remove now.
|
|
This avoids potentially installing unnecessary dependencies.
Also, avoid gem update --system and bundler config/install.
|
|
|
|
because String#% takes the arguments as an Array, which ends up in allocating an extra Array object
|
|
Simplify #server_port now that it is required to be an integer.
Simplify #port.
Remove nested conditional in #ip.
Remove private #extract_proto_header, unused since
b87d1828bd90b24eb0fa4a99abf580d9ddde4a0e (added in
6f349e1d2d1f528c486417d3421609be6e033e31, so only available since
2.1).
Coverage after this commit:
3286 relevant lines, 3282 lines covered and 4 lines missed. ( 99.88% )
1114 total branches, 1067 branches covered and 47 branches missed. ( 95.78% )
|
|
Raise for HeadersHash.allocate, don't define HeadersHash#allocate.
Remove unnecessary character class in regexp for parsing cookies.
Coverage after this commit:
3301 relevant lines, 3287 lines covered and 14 lines missed. ( 99.58% )
1130 total branches, 1067 branches covered and 63 branches missed. ( 94.42% )
|
|
Fix obviously broken code in respond_to? implementation.
Coverage after this commit:
3305 relevant lines, 3280 lines covered and 25 lines missed. ( 99.24% )
1133 total branches, 1061 branches covered and 72 branches missed. ( 93.65% )
|
|
Coverage after this commit:
3305 relevant lines, 3266 lines covered and 39 lines missed. ( 98.82% )
1133 total branches, 1039 branches covered and 94 branches missed. ( 91.7% )
|
|
Fix handling of accept-encoding in vary header, since Array#include?
when called with an array doesn't check if any element in the array
matches.
Coverage after this commit:
3305 relevant lines, 3257 lines covered and 48 lines missed. ( 98.55% )
1133 total branches, 1036 branches covered and 97 branches missed. ( 91.44% )
|
|
Remove unnecessary conditional in -D option handling.
Simplify code in --profile-mode option handling.
Remove unnecessary begin clause in handler_opts, since it
covers the whole method.
Remove use of SPEC_ARGV, just use ARGV and set Rack::Server::ARGV
in the specs, relying on normal constant lookup.
Simplify server method now that Rack::Handler::FastCGI is no longer
present.
Coverage after this change:
3305 relevant lines, 3254 lines covered and 51 lines missed. ( 98.46% )
1134 total branches, 1028 branches covered and 106 branches missed. ( 90.65% )
|
|
Simplify coverage testing code while here.
Current coverage:
3311 relevant lines, 3243 lines covered and 68 lines missed. ( 97.95% )
1140 total branches, 1021 branches covered and 119 branches missed. ( 89.56% )
|
|
Add rack-session gem when testing rack-attack
|
|
Rename the security policy to the recognised name by GitHub.
|
|
SPEC currently does not currently specify a way to get the HTTP
version in use. However, both Chunked and CommonLogger need access
to the http version for correct functioning, and other users in
the rack ecosystem need it as well (Roda needs it, and I've just
identified a need for it in rack-test).
Unicorn, Webrick, and Puma all currently set SERVER_PROTOCOL.
However, Puma currently sets SERVER_PROTOCOL statically to
HTTP/1.1, unlike Unicorn and Webrick, which set it to the
protocol used by the client. Unicorn and Puma set HTTP_VERSION
to the protocol used by the client.
This specifies that SERVER_PROTOCOL should match the protocol
used by the client, that it should be a valid protocol matching
HTTP/\d(\.\d)?, and that if HTTP_VERSION is provided, it must
match SERVER_PROTOCOL. This will require minor changes to Puma
to be compliant with the new SPEC.
Set SERVER_PROTOCOL to HTTP/1.1 by default in Rack::MockRequest,
allowing it to be set by the :http_version option. Update
CommonLogger specs to include the version.
This removes a spec in Chunked for usage without SERVER_PROTOCOL.
A comment in the removed lines indicate unicorn will not set
SERVER_PROTOCOL for HTTP/0.9 requests, but that is incorrect, as
unicorn has set SERVER_PROTOCOL to HTTP/0.9 since 2009 (see unicorn
commit bd0599c4ac91d95cae1f34df3ae99c92f3225391). The related
comment was correct when added in 2009 (rack commit
895beec0622d3cafdc5fbae20d665c6d5f6c8e7c), but has been incorrect
since the code was changed from HTTP_VERSION to SERVER_PROTOCOL in
2015 (rack commit e702d31335c1a820e99c3acdd9d3368ac25da010).
|
|
This commit adds two new predicate methods to the `Rack::Response`
class:
- `not_acceptable?` which returns true on HTTP 406 Not Acceptable
- `request_timeout?` which returns true on HTTP 408 Request Timeout
Links to MDN documentation for each status code:
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/406
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408
|
|
|
|
|
|
|
|
At least on OpenBSD, this test occasionally hangs, because the
`wr.write` does not return/raise after the `rd.close` in the other
thread. Switch to `write_nonblock` with `exception: false`, using
Thread.pass if the write would block.
With this change, the test takes less than two seconds and does
not hang.
|
|
With this and the webrick handler autoload removal, rack is autoload
free if you do not require rack itself, and only require the parts
of rack you are using.
Take care to avoid the circular requires when switching from
autoload to require_relative.
Move Multipart::Parser specific constants into multipart/parser.rb.
MULTIPART_BOUNDARY is used also in Multipart::Generator, so leave
that in multipart.rb.
Move require_relative calls to avoid circular require warnings.
|
|
|
|
There isn't a reason the constant needs to be autoloaded. If puma or
falcon is not installed, the try_require will still load the webrick
handler file, so register the the handler in the handler file, similar
to how puma and falcon handle it. This makes webrick support less
special.
|
|
|
|
|
|
|
|
|
|
|