mpm_winnt: Remove an unnecessary Sleep() in the winnt_accept() function.
This sleep occured in a situation when:
- We don't have a free completion context in the queue
- We can't add one, as doing so would exceed the max_num_completion_contexts
limit (all worker threads are busy)
- We have exceeded a 1 second timeout while waiting for it
In this case, the Sleep() call is unnecessary, as there is no intermittent
failure that can be waited out, but rather than that, it's an ordinary
situation with all workers being busy. Presumably, calling Sleep() here
can be even considered harmful, as it affects the fairness between the
listeners that are blocked waiting for the completion context.
So, instead of calling Sleep() just check for the possible shutdown and
immediately retry acquiring a completion context. If all worker threads
are still busy, the retry will block in the same WaitForSingleObject() call,
which is fine.
mpm_winnt: Simplify the shutdown code that was waiting for multiple worker
thread handles in batches.
Starting from r1801636, there is no difference between ending the wait with
one or multiple remaining threads. This is because we terminate the process
if at least one thread is still active when we hit a timeout.
Therefore, instead of making an effort to evenly distribute and batch the
handles with WaitForMultipleObjects(), we could just start from one end,
and wait for one thread handle at a time.
mpm_winnt: Avoid using TerminateThread() in case the shutdown routine
hits a timeout while waiting for the worker threads to exit.
Using TerminateThread() can have dangerous consequences such as deadlocks —
say, if the the thread is terminated while holding a lock or a heap lock
in the middle of HeapAlloc(), as these locks would not be released.
Or it can corrupt the application state and cause a crash.
(See https://msdn.microsoft.com/en-us/library/windows/desktop/ms686717)
Rework the code to call TerminateProcess() in the described circumstances
and leave the cleanup to the operating system.
mpm_winnt: Make the shutdown faster by avoiding unnecessary Sleep()'s
when shutting down the worker threads.
Previously, the shutdown code was posting an amount of I/O completion
packets equal to the amount of the threads blocked on the I/O completion
port. Then it would Sleep() until all these threads "acknowledge" the
completion packets by decrementing the global amount of blocked threads.
A better way would be to send the number of IOCP_SHUTDOWN completion
packets equal to the total amount of threads and immediately proceed to
the next step. There is no need to block until the threads actually receive
the completion, as the shutdown process includes a separate step that waits
until the threads exit, and the new approach avoids an unnecessary delay.
Add logic to read the Upgrade header and use it in the response.
Use we you are proxying to a server that has multiple upgrade on the same IP/Port.
PR 61142
mpm_winnt: Refactor the mpm_get_completion_context() function so that it
would return a proper apr_status_t instead of yielding the result via the
*timeout out variable.
This makes the calling side easier to follow by avoiding an additional
layer of if's.
mpm_winnt: Remove an unnecessary retry after receiving a non-timeout failure
from the mpm_get_completion_context() function.
Currently, the only possible reasons why mpm_get_completion_context() could
fail are real errors such as being unable to WaitForSingleObject(), allocate
memory or create an event. Retrying under such circumstances doesn't make
sense, and could be as well considered harmful.
mpm_winnt: Factor out a helper function to parse the type of an accept
filter and use an appropriate enum for it.
This makes the code in winnt_accept() a bit easier to follow. As a minor
side effect, it also fixes a small bug where the "unrecognized AcceptFilter
'%s'" log entry would always contain "none" instead of the actually
unrecognized kind of the accept filter.
Fix another mod_lua compile error.
Compilation now tested against Lus 5.1, 5.2
and 5.3. The libs were compiled with COMPAT
flags set, but mod_lua no longer sets them
so it should compile and run with non-COMPAT
libs as well.
* Do not apply the strict permissions of the temporary file to a possibly
existing passwd file.
This long standing bug was triggered by fixing a bug in APR in r1791029.
Jacob Champion [Thu, 29 Jun 2017 17:43:48 +0000 (17:43 +0000)]
proxy_fcgi: remove FPM-specific logic
Reverts r1780328, r1780329, and their associated followups, which
incorrectly manipulated SCRIPT_NAME by default. All proxy_fcgi.t
regression tests now pass.
Discussion on-list, but any occurance of a platform-specific behavior in this
code path will alter the behavior of the core code and introduce the very
fingerprintable behavior this patch pretended to obscuficate.
Returning 404 for /CON for example may lead to a module such as mod_speling
revealing the existance of a real file named similar to /.conf, which makes
this an unwise behavior.
Further discussion of returning 404 for all CHR files encountered in the
filepath (not URI path), which currently return 403 on all platforms,
belongs on the dev list.
Since r1753257, "HEAD" method is registered into the registry hash with the M_GET ID.
(r1757672 in 2.4.x)
We iterate over all the values of the registery, so there is no need anymore to have a special case for "HEAD" in 'make_allow()'. It has its own entry now.
With the current code, we have "HEAD" 3 times in the Allow Header field.
This is because we find M_GET 2 times in the registry hash. The first one gives "GET" and "HEAD" (as the special handling), and the second "HEAD" and "HEAD" (as the special handling).
BTW, use APR_ARRAY_PUSH instead of hand coding it, in oder to have the code more readable.
Send a 404 response like other OSs do instead of 403 on Windows when
a path segment or file requested uses a reserved word so Windows
cannot be fingerprinted. PR55887
Jacob Champion [Tue, 20 Jun 2017 23:55:20 +0000 (23:55 +0000)]
util.c: add a strict Base64 decoding function
ap_pbase64decode_strict() adds to the functionality of
ap_pbase64decode() in two ways:
- the length of the decoded buffer is returned, allowing embedded NULLs
to be retained by the caller
- the input string is strictly checked for Base64 validity, including
correct zero-padding at the end of the string
(This was originally added to the httpdunit feature/backport branch in
r1796208, then reverted in r1799376, since it's currently intended for
trunk only.)
Jacob Champion [Tue, 20 Jun 2017 23:08:19 +0000 (23:08 +0000)]
util.c: ensure all TEST_CHAR loops stop at the null terminator
In the aftermath of CVE-2017-7668, decouple the business logic ("is NULL
a T_HTTP_CTRL") from the postcondition ("must not go past the end of the
string"). The NULL-byte classification in the TEST_CHAR table may change
in the future.
Eric Covener [Fri, 16 Jun 2017 15:13:03 +0000 (15:13 +0000)]
add RewriteOptions LongURLOptimization
Variable expansion in RewriteCond causes strings to be duplicated
out of r->pool. If the variables are long and many conditions
are evaluated, r->pool can get seriously bloated.
Clear the memory used for variable expansion for each set of conditons
once the set of conditions fails to match.
Stefan Eissing [Tue, 6 Jun 2017 07:58:57 +0000 (07:58 +0000)]
On the trunk:
mod_http2: Fix for possible CPU busy loop introduced in v1.10.3 where a stream may keep
the session in continuous check for state changes that never happen.
projects / apache / log