Date | Commit message (Collapse) |
|
...instead of tripping an assertion.
This fixes a potential denial-of-service for servers exposed directly
to untrusted clients.
This bug does not affect supported Unicorn deployments as Unicorn is
only supported with trusted clients (such as nginx) on a LAN. nginx is
known to reject clients that send invalid Content-Length headers, so any
deployments on a trusted LAN and/or behind nginx are safe.
Servers affected by this bug include (but are not limited to) Rainbows!
and Zbatery. This does not affect Thin nor Mongrel which never got
request body filtering treatment that the Unicorn HTTP parser got in
August 2009.
|
|
We'll use struct members exclusively from now on instead of
throwing ivars into the mix. This allows us to _unofficially_
support direct access to more members easily. Unofficial
extensions may include the ability to splice(2)/tee(2) for
better performance.
This also makes our object size smaller across all Ruby
implementations as well, too (helps Rainbows! out).
|
|
The temporary paths we create to mimic script/server-emulation
did not work when working_directory was used. Now we defer
path creation until after working_directory is bound.
|
|
First off, this memory leak DOES NOT affect Unicorn itself.
Unicorn allocates the HttpParser once and always reuses it
in every sequential request.
This leak affects applications which repeatedly allocate a new
HTTP parser. Thus this bug affects _all_ deployments of
Rainbows! and Zbatery. These servers allocate a new parser for
every client connection.
I misread the Data_Make_Struct/Data_Wrap_Struct documentation
and ended up passing NULL as the "free" argument instead of -1,
causing the memory to never be freed.
From README.EXT in the MRI source which I misread:
> The free argument is the function to free the pointer
> allocation. If this is -1, the pointer will be just freed.
> The functions mark and free will be called from garbage
> collector.
|
|
we've already got "-*- encoding: binary -*-" in everything
|
|
This prevents trigger-happy init scripts from reading the pid
file (and thus sending signals) to a not-fully initialized
master process to handle them.
This does NOT fix anything if other processes are sending
signals prematurely without relying on the presence of the pid
file. It's not possible to prevent all cases of this in one
process, even in a purely C application, so we won't bother
trying.
We continue to always defer signal handling to the main loop
anyways, and signals sent to the master process will be
deferred/ignored until Unicorn::HttpServer#join is run.
|
|
* rack-1.1:
http_response: disallow blank, multi-value headers
local.mk.sample: use rack-1.1.0
bump "rack.version" env to [1,1]
set env["rack.logger"] for applications
|
|
Rails 2.3.3.1 is ancient
|
|
This behavior change also means our grandparent (launched
from a controlling terminal or script) will wait until
the master process is ready before returning.
Thanks to IƱaki Baz Castillo for the initial implementations
and inspiration.
|
|
This is not explicitly specified or listed as an example in in
rfc2616. However, rfc2616 section 3.2.1 defers to rfc2396[1]
for the definition of absolute URIs, so the userinfo component
should be allowable, even if it does not make any sense.
In the real world, previous versions of Mongrel used URI.parse()
and thus allowed userinfo, so we also have precedence to allow
userinfo to be compatible *in case* our interpretation of the
RFCs is incorrect. This change is unfortunately needed because
*occasionally* real clients rely on them.
Reported-by: Scott Chacon
[1] rfc3986 obsoletes rfc2396, but also includes userinfo
|
|
rack.git upstream has it, so it will likely be in Rack 1.1
|
|
This is allowed according to RFC 2396, section 3.3 and matches
the behavior of URI.parse, as well.
|
|
On heavily loaded machines, this test can take a while,
fortunately our test suite is parallelization-friendly.
|
|
Shells already expand '~' before the executables see it, and
relative paths inside symlinks can get set incorrectly to the
actual directory name, and not the (usually desired) symlink
name for things like Capistrano.
Since our paths are now unexpanded, we must now check the
"working_directory" directive and raise an error if the user
specifies the config file in a way that makes the config file
unreloadable.
|
|
First move it to a separate method, this allows subclasses to
reuse our error handler. Additionally, capture HttpParserError
as well since backtraces are worthless when a client sends us
a bad request, too.
|
|
This works around a race condition caused by the server
closing the connection before writing out to stderr in
the ensure block. So to ensure we've waited on the server
to write to the log file, just send another HTTP request
since we know our test server only processes requests
serially.
|
|
Typically UNIX domain sockets are created with more liberal
file permissions than the rest of the application. By default,
we create UNIX domain sockets to be readable and writable by
all local users to give them the same accessibility as
locally-bound TCP listeners.
This only has an effect on UNIX domain sockets.
This was inspired by Suraj Kurapati in
cfbcd2f00911121536rd0582b8u961f7f2a8c6e546a@mail.gmail.com
|
|
Leaving the EOFError exception as-is bad because most
applications/frameworks run an application-wide exception
handler to pretty-print and/or log the exception with a huge
backtrace.
Since there's absolutely nothing we can do in the server-side
app to deal with clients prematurely shutting down, having a
backtrace does not make sense. Having a backtrace can even be
harmful since it creates unnecessary noise for application
engineers monitoring or tracking down real bugs.
|
|
Subclass off the core File class so we don't have to
worry about #size being defined. This will mainly
be useful to Rainbows! but allows us to simplify
our TeeInput implementation a little, too.
|
|
The relative working_directory test runs so quickly
that the master may not even have signal handlers
setup by the time we're done with it. The proper
way would be to not start workers until the master
is ready, but that breaks some test cases horribly.
|
|
Make sure we're completely resumable no matter how
idiotic clients are.
|
|
This allows clients to trickle headers and trailers. While
Unicorn itself does not support slow clients for many reasons,
this affects servers that depend on our parser like Rainbows!.
This actually does affect Unicorn when handling trailers, but
HTTP trailers are very ever rarely used in requests.
Fortunately this stupid bug does not seem able to trigger
out-of-bounds conditions.
|
|
Prevent ourselves from breaking things in case applications
start depending on this.
|
|
It makes more sense this way since users usually expect config
file directives to be order-independent.
|
|
This basically a prettier way of saying:
Dir.chdir(Unicorn::HttpServer::START_CTX[:cwd] = path)
In the config file. Unfortunately, this is configuration
directive where order matters and you should specify it
before any other path[1] directives if you're using relative
paths (relative paths are not recommended anyways)
[1] pid, stderr_path, stdout_path
|
|
Just write bytes to the file instead and track its
size increase instead of its mode. As of now all
the unit tests pass under FreeBSD 7.2.
|
|
Not documented on FreeBSD 7.2, but it seems to happen there
and searching around, it seems to happen on other systems,
too...
|
|
IPv4 addresses in the format of: "^[[:digit:]]+:[[:digit:]]+$"
isn't very portable..
|
|
We modified TeeInput to have read-in-full semantics in most
situations to suit existing apps and libraries (like Rails) that
don't check for and handle partial reads correctly.
The read-in-full semantics are needed by Rails because of this:
https://rails.lighthouseapp.com/projects/8994/tickets/3343
|
|
Checking for addr to match the DEFAULT_HOST constant
is wrong since having only 127.0.0.1:8080 will still
prevent 0.0.0.0:8080 from being bound.
|
|
:delay may be a Float to represent fractional seconds.
|
|
There's always been a small window of opportunity for a script
to do File.read(pid).to_i would cause File.read() to read an
empty file and return "". This closes that window while
hopefully retaining backwards compatibility...
We've always checked for dirname(pid) writability in
Configurator, so we can safely write to a temporary file in the
intended directory and then atomically rename() it to the
destination path.
|
|
Since Unicorn config files are written in Ruby, setting
RAILS_RELATIVE_URL_ROOT should be possible (and even encouraged)
in the config file if it is done at all.
|
|
We don't want to accidentally kill every process in the
process group.
|
|
When SIGHUP reloads the config, we didn't account for the case
where the listen socket was completely unspecified. Thus the
default listener (0.0.0.0:8080), did not get preserved and
re-injected into the config properly.
Note that relying on the default listen or specifying listeners
on the command-line means it's /practically/ impossible to
_unbind_ those listeners with a configuration file reload. We
also need to preserve the (unspecified) default listener across
upgrades that later result in SIGHUP, too; so the easiest way is
to inject the default listener into the command-line for
upgrades.
Many thanks to James Golick for reporting and helping me track
down the bug since this behavior is difficult to write reliable
automated tests for.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
|
|
Just to ensure we handle HUP correctly since preload_app
changes the behavior of HUP handling a bit.
|
|
This ensures any string literals that pop up in *our* code will
just be a bag of bytes. This shouldn't affect/fix/break
existing apps in most cases, but most constants will always have
the "correct" encoding (none!) to be consistent with HTTP/socket
expectations. Since this comment affects things only on a
per-source basis, it won't affect existing apps with the
exception of strings we pass to the Rack application.
This will eventually allow us to get rid of that Unicorn::Z
constant, too.
|
|
This probably doesn't affect anyone with HTTP/1.1, but
future versions of HTTP will use absolute URIs and maybe
we'll eventually get clients that (mistakenly) send us
Host: headers along with absolute URIs.
|
|
HTTP/0.9 GET requests expect responses without headers. Some
weird applications/tools still use the ancient HTTP/0.9
protocol for weird reasons, so we'll support them.
ref: rfc 1945, section 4.1
|
|
This method determines if there are headers in the request.
Simple HTTP/0.9 requests did not have headers in the request
(and our responses we make should not have them, either).
|
|
And it'll default to HTTP/0.9 if HTTP_VERSION is not specified
(as version-less HTTP requests imply HTTP/0.9.
|
|
Followup to commit 7f74a16406c92c4362ac20af4ccb8bc821cf978b,
we want to ensure error messages do not get swallowed up into
/dev/null when daemonizing; so we defer the default redirects
to "/dev/null" to as late as possible.
|
|
SERVER_PROTOCOL is actually defined as "HTTP/1.1 even though it
should not be for HTTP/0.9 responses.
|
|
HTTP/0.9 only supports GET requests and didn't require a
version number in the request line. Additionally, only
a single CRLF was required.
Note: we don't correctly generate HTTP/0.9 responses, yet.
|
|
While I still consider pound to be irrelevant, but I still
sometimes get hand-crafted HTTP requests that come in with
multiline headers. Since these are part of the HTTP specs and
not difficult to support, we might as well support them for
the sake of completeness.
|
|
Rack is autoload-based and so are we.
|
|
ab still sends this with HTTP/1.0 requests, which is
unfortunate, but synthetic benchmarks are good for marketing
purposes!
|
|
This lets clients can pass through newly-invented status codes
that Rack does not know about.
|
|
It's not worth the effort to keep the internal API consistent
between non-bugfix versions.
|
|
TeeInput being needed is now (once again) an uncommon code path
so there's no point in relying on global constants. While we're
at it, allow StringIO to be used in the presence of small
inputs; too.
|