Re: Crash with SSL renegotiations in 2.4.x branch
Backported in 1844223, will be part of 2.4.37. Thanks again! Rainer Great! Thanks a lot for proposing & backporting. Regards, Michael
Crash with SSL renegotiations in 2.4.x branch
Hi, there's a bug in the current 2.4.x branch of httpd which leads to crashes for SSL renegotiations. The variable "ctx" is always NULL in ssl_engine_kernel.c, ssl_hook_Access_classic(), and it's used here: if (!(cert_store || (cert_store = SSL_CTX_get_cert_store(ctx ... In trunk, this bug has been fixed in r1828793. Please backport this for 2.4.37. Regards, Michael
Re: Regression: mod_http2 continues to process abandoned connection
Hi Stefan,
Yes, the patch solves the problem for me :-) Thanks a bunch!
Regards,
Michael
Zitat von Stefan Eissing :
Michael, I can reproduce the problem and habe a fix. Can you test if
the following patch also solves the problem for you? Thanks!
Index: modules/http2/h2_mplx.c
===
--- modules/http2/h2_mplx.c (revision 1748955)
+++ modules/http2/h2_mplx.c (working copy)
@@ -436,6 +436,9 @@
if (stream->input) {
m->tx_handles_reserved += h2_beam_get_files_beamed(stream->input);
h2_beam_on_consumed(stream->input, NULL, NULL);
+/* Let anyone blocked reading know that there is no more to come */
+h2_beam_abort(stream->input);
+/* Remove mutex after, so that abort still finds cond to signal */
h2_beam_mutex_set(stream->input, NULL, NULL, NULL);
}
h2_stream_cleanup(stream);
Regression: mod_http2 continues to process abandoned connection
Hi, I have found a regression in mod_http2. When the client stops sending data and closes the connection, mod_http2 doesn't detect that the client has left and continues to "read" request data (until the request times out because of the server's TimeOut value). The bug has been introduced with mod_http2 version 1.5.8 (SVN 1747532). It is also present in the httpd 2.4.21 release candidate. mod_http2 version 1.5.7 (SVN 1747194) works. How to reproduce: curl --http2 -k -v --data-binary @bigfile.dat --limit-rate 1 https://http2-enabled-apache-server/ ... then kill the curl process with "kill " Log messages: h2_session.c(1827): h2_session(66): NO_IO event, 1 streams open h2_session.c(1691): AH03078: h2_session(66): transit [BUSY] -- no io --> [WAIT] h2_session.c(2260): h2_session: wait for data, 20 micros h2_mplx.c(775): h2_mplx(66): trywait on data for 200.00 ms) h2_session.c(1691): AH03078: h2_session(66): transit [WAIT] -- wait cycle --> [BUSY] h2_filter.c(113): core_input(66): read, NONBLOCK_READ, mode=0, readbytes=8000 h2_filter.c(164): (104)Connection reset by peer: AH03046: h2_conn_io: error reading h2_session.c(1576): (104)Connection reset by peer: h2_session(66): input gone h2_session.c(1777): h2_session(66): conn error -> shutdown h2_session.c(789): h2_session(66): malloc(120) h2_session.c(643): AH03068: h2_session(66): sent FRAME[GOAWAY[error=0, reason='', last_stream=1]], frames=3/3 (r/s) h2_session.c(799): h2_session(66): free() h2_session.c(799): h2_session(66): free() h2_conn_io.c(289): h2_conn_io: pass_output h2_conn_io.c(124): bb_dump(66-0)-master conn pass: heap[17] flush h2_conn_io.c(309): (32)Broken pipe: AH03044: h2_conn_io(66): pass_out brigade 17 bytes h2_session.c(740): AH03069: session(66): sent GOAWAY, err=0, msg= h2_session.c(1691): AH03078: h2_session(66): transit [BUSY] -- local goaway --> [LSHUTDOWN] h2_mplx.c(1356): h2_mplx(66): dispatch events h2_session.c(1827): h2_session(66): NO_IO event, 1 streams open h2_session.c(1691): AH03078: h2_session(66): transit [LSHUTDOWN] -- no io --> [WAIT] h2_session.c(2260): h2_session: wait for data, 20 micros h2_mplx.c(775): h2_mplx(66): trywait on data for 200.00 ms) h2_session.c(1691): AH03078: h2_session(66): transit [WAIT] -- wait cycle --> [LSHUTDOWN] h2_filter.c(113): core_input(66): read, NONBLOCK_READ, mode=0, readbytes=8000 h2_filter.c(164): (103)Software caused connection abort: AH03046: h2_conn_io: error reading h2_session.c(1576): (103)Software caused connection abort: h2_session(66): input gone h2_session.c(1691): AH03078: h2_session(66): transit [LSHUTDOWN] -- conn error --> [DONE] h2_mplx.c(1356): h2_mplx(66): dispatch events h2_session.c(2312): (70014)End of file found: h2_session(66): [DONE] process returns h2_conn_io.c(289): h2_conn_io: pass_output h2_conn_io.c(124): bb_dump(66-0)-master conn pass: h2eoc flush h2_session.c(963): session(66): cleanup and destroy h2_mplx.c(539): h2_mplx(66): release_join with 1 streams open, 0 streams resume, 0 streams ready, 1 tasks h2_mplx.c(518): h2_mplx(66-1): exists, started=1, scheduled=1, submitted=0, suspended=0 h2_mplx.c(402): h2_stream(66-1): done h2_mplx.c(567): h2_mplx(66): 2. release_join with 1 streams in hold AH03198: h2_mplx(66): release, waiting for 5 seconds now for 1 h2_workers to return, have still 1 tasks outstanding ->03198: h2_stream(66-1): POST server /myurl -> ? 0[orph=1/started=1/done=0] AH03198: h2_mplx(66): release, waiting for 10 seconds now for 1 h2_workers to return, have still 1 tasks outstanding AH03198: h2_mplx(66): release, waiting for 15 seconds now for 1 h2_workers to return, have still 1 tasks outstanding AH03198: h2_mplx(66): release, waiting for 20 seconds now for 1 h2_workers to return, have still 1 tasks outstanding [...] AH03198: h2_mplx(66): release, waiting for 270 seconds now for 1 h2_workers to return, have still 1 tasks outstanding AH03198: h2_mplx(66): release, waiting for 275 seconds now for 1 h2_workers to return, have still 1 tasks outstanding AH03198: h2_mplx(66): release, waiting for 280 seconds now for 1 h2_workers to return, have still 1 tasks outstanding AH03198: h2_mplx(66): release, waiting for 285 seconds now for 1 h2_workers to return, have still 1 tasks outstanding h2_task.c(194): (70007)The timeout specified has expired: h2_task(66-1): read returned mod_airlock.c(1307): Error reading body data from client (errno = 0) h2_from_h1.c(488): h2_from_h1(1): output_filter called h2_from_h1.c(551): h2_from_h1(1): removed header filter, passing brigade len=0 h2_task.c(355): h2_task(66-1): write response body (0 bytes) h2_task.c(355): h2_task(66-1): write response body (0 bytes) h2_task.c(355): h2_task(66-1): write response body (0 bytes) h2_task.c(343): AH03348: h2_task(66-1): open response to POST iaves60 /capi/TestServlet h2_task.c(753): h2_task(66-1): process_request done h2_task.c(725): h2_task(66-1): processing done h2_mplx.c(949): h2_
Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
Zitat von Stefan Eissing : Withdrawn the proposal in r1747668 after reading the comment from Roy again. Apache is the only HTTP/2 server in this world that sends this strange header. So omitting it would be the right thing to do. Michael, since you know more about this: is there a specific UA string where httpd can detect the broken NodeJS client from and suppress "Update: XXX" response headers? I believe NodeJS is broken on *any* Update response header, right? So, if we fix it for this client, we need to fix any protocol announcement, not just 'h2'. That sounds reasonable. I think the GitHub user that created the original bug report may know which NodeJS versions are affected. Regards, Michael
Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
Zitat von William A Rowe Jr : Skimming the responses, they just keep getting more and more amusing, and shining a candle to the absurdity of keeping this non-sequitur request response. Could you go ahead and add that conditional? To all developers who participated in this discussion: Please review the backport proposal and vote +1 if you think that it's not completely wrong :-) Note that "Suppress 'h2' announcements in Upgrade: header" has been inserted at the top of the STATUS file; it should probably be moved to the bottom of the 'Patches proposed' section. Regards, Michael
Re: Detecting client aborts and stream resets
Zitat von Stefan Eissing : Thanks for the patch! I applied it to trunk in r1743335, will be part of next 1.5.4 release. I only omitted the last change as I do not want to set aborted on the main connection every time the session closes. Ok, that's fine for me. Thanks a lot Stefan! Regards, Michael
Re: Detecting client aborts and stream resets
Zitat von William A Rowe Jr :
On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann
wrote:
William is right, this is not a good idea. The ->aborted flag should serve
this purpose of telling anyone interested that this connection is not
longer delivering. I will make a github release soon where that is working
and you can test.
Thank you Stefan! It is now working for stream resets, but it's not yet
working if the client closes the whole TCP connection.
As expected... this is why I pointed out in my first reply that you don't
want a single-protocol solution to this puzzle.
Of course I'd also prefer a general solution.
I have created a patch for mod_http2: With the patch, it sets
c->aborted on the master connection and also on the "dummy"
connections (streams) when the client closes the TCP connection.
@Stefan: It would be great if you could check this code and add it to
mod_http2 :-)
Index: modules/http2/h2_mplx.c
===
--- modules/http2/h2_mplx.c (revision 1743146)
+++ modules/http2/h2_mplx.c (working copy)
@@ -488,6 +488,15 @@
return 1;
}
+static int task_abort_connection(void *ctx, void *val)
+{
+h2_task *task = val;
+if (task->c) {
+task->c->aborted = 1;
+}
+return 1;
+}
+
apr_status_t h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
{
apr_status_t status;
@@ -537,6 +546,8 @@
* and workers *should* return in a timely fashion.
*/
for (i = 0; m->workers_busy > 0; ++i) {
+h2_ihash_iter(m->tasks, task_abort_connection, m);
+
m->join_wait = wait;
status = apr_thread_cond_timedwait(wait, m->lock,
apr_time_from_sec(wait_secs));
@@ -591,6 +602,7 @@
AP_DEBUG_ASSERT(m);
if (!m->aborted && enter_mutex(m, &acquired) == APR_SUCCESS) {
m->aborted = 1;
+m->c->aborted = 1;
h2_ngn_shed_abort(m->ngn_shed);
leave_mutex(m, acquired);
}
See my later reply about detecting connection tear-down, patches welcome.
Sounds good! If someone sends a patch, I will help to test it.
Re: Detecting client aborts and stream resets
William is right, this is not a good idea. The ->aborted flag should serve this purpose of telling anyone interested that this connection is not longer delivering. I will make a github release soon where that is working and you can test. Thank you Stefan! It is now working for stream resets, but it's not yet working if the client closes the whole TCP connection. Regards, Michael
Re: Detecting client aborts and stream resets
Zitat von William A Rowe Jr : On Tue, May 3, 2016 at 9:31 AM, Michael Kaufmann wrote: Hi all, a content generator module can detect client aborts and stream resets while it reads the request body. But how can it detect this afterwards, while the response is being generated? This is important for HTTP/2, because the client may reset a stream, and mod_http2 needs to wait for the content generator to finish. Therefore the content generator should stop generating the response when it is no longer needed. Is there any API for this? The "conn_rec->aborted" flag exists, but which Apache function sets this flag? If there is no API, maybe an optional function for mod_http2 would be a solution. Nope - an optional function in mod_http2 is too special case, generators must remain protocol (socket or other transport) agnostic. Sure, official Apache modules should not call protocol-dependent hooks, but it could be a solution for 3rd party modules. In the case of mod_cache'd content, the generator can't quit, it is already populating a cache, which means you'll generate an invalid cached object. But if you knew that the cache module wasn't collecting info, r->c->aborted tells you if anyone is still listening, right? Unfortunately not. While the content generator is running (just preparing the response, it is not reading from the input filters anymore and not writing to the output filters yet), Apache does not check the connection's state. I'm not sure how mod_proxy deals with this - does it abort the backend request when the client closes the connection?
[PATCH 54255] mod_deflate adjusts the headers "too late"
Hi, It would be great if somebody could have a look at the proposed patch. The problem: Request headers are adjusted too late (in the input filter). Proposed solution: Adjust the request headers in the filter init function. https://bz.apache.org/bugzilla/show_bug.cgi?id=54255 Regards, Michael
Detecting client aborts and stream resets
Hi all, a content generator module can detect client aborts and stream resets while it reads the request body. But how can it detect this afterwards, while the response is being generated? This is important for HTTP/2, because the client may reset a stream, and mod_http2 needs to wait for the content generator to finish. Therefore the content generator should stop generating the response when it is no longer needed. Is there any API for this? The "conn_rec->aborted" flag exists, but which Apache function sets this flag? If there is no API, maybe an optional function for mod_http2 would be a solution. Regards, Michael
Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
Done in r1740075.
I think that commit introduced a small bug, because the "for" loop is
left when "h2" is seen and "report_all" is false. There may be other
protocols that are more preferred than the current one.
Suggested change:
Index: server/protocol.c
===
--- server/protocol.c (revision 1740112)
+++ server/protocol.c (working copy)
@@ -2017,15 +2017,18 @@
* existing. (TODO: maybe 426 and Upgrade then?) */
upgrades = apr_array_make(pool, conf->protocols->nelts + 1,
sizeof(char *));
for (i = 0; i < conf->protocols->nelts; i++) {
const char *p = APR_ARRAY_IDX(conf->protocols, i, char *);
/* special quirk for HTTP/2 which does not allow 'h2' to
* be part of an Upgrade: header */
-if (strcmp(existing, p) && strcmp("h2", p)) {
+if (!strcmp("h2", p)) {
+continue;
+}
+if (strcmp(existing, p)) {
/* not the one we have and possible, add in this order */
APR_ARRAY_PUSH(upgrades, const char*) = p;
}
else if (!report_all) {
break;
}
}
Regards,
Michael
Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
Zitat von Stefan Eissing : Done in r1740075. I was thinking of a nicer solution, but that involved inventing new hooks which seems not worth it. Since this area of protocol negotiation has already been talked about in regard to TLS upgrades and websockets, I do not want to invest in the current way of handling this too much time. -Stefan Thank you Stefan. Also thanks to everyone who took part in the discussion. This topic is way more complex than I thought. Regards, Michael
Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
On Tue, Apr 19, 2016 at 8:57 AM, Michael Kaufmann wrote: Yes, you are right. But with the response header "Upgrade: h2", Apache is telling the client "you may send such a header" when in fact this is not allowed. Devils advocate: Apache is telling the client at the application layer its protocol support and preferences. Something it couldn't actually do with ALPN. If the client knows HTTP/2 it has knows the particulars of the Upgrade. I'd suggest taking it up with the HTTP/2 workgroup mailing list. Good idea. I have sent a mail to the HTTP/2 workgroup mailing list, so let's discuss this issue there: https://lists.w3.org/Archives/Public/ietf-http-wg/
Re: "Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
On Tue, Apr 19, 2016 at 8:47 AM, Michael Kaufmann wrote: I think that this is wrong, because of this sentence in RFC 7540: A server MUST ignore an "h2" token in an Upgrade header field. Presence of a token with "h2" implies HTTP/2 over TLS, which is instead negotiated as described in Section 3.3. Isn't that referring to inbound Upgrade headers? Yes, you are right. But with the response header "Upgrade: h2", Apache is telling the client "you may send such a header" when in fact this is not allowed.
"Upgrade: h2" header for HTTP/1.1 via TLS (Bug 59311)
Hi, you may already know that HTTP/2 clients use ALPN to advertise support for HTTP/2 when TLS is used. The issue is this: When mod_http2 is enabled, Apache sends an "Upgrade: h2" response header to clients that have *not* advertised support for HTTP/2 (clients that speak only HTTP/1.x). I think that this is wrong, because of this sentence in RFC 7540: A server MUST ignore an "h2" token in an Upgrade header field. Presence of a token with "h2" implies HTTP/2 over TLS, which is instead negotiated as described in Section 3.3. What do you think about this issue, and what do you think about the attached patch in bug 59311? Regards, Michael
Re: Bug with "SetHandler None"
Eric Covener wrote: On Sat, Mar 19, 2016 at 11:31 AM, Michael Kaufmann wrote: I have found a bug in the current 2.4.x branch: "SetHandler None" does not work anymore (note the capital letter "N"). This worked with Apache 2.4.18. Probably this commit has changed the behavior: http://svn.apache.org/r1729876 Thanks Michael! Thank you for fixing the bug! :-)
Bug with "SetHandler None"
Hi, I have found a bug in the current 2.4.x branch: "SetHandler None" does not work anymore (note the capital letter "N"). This worked with Apache 2.4.18. Probably this commit has changed the behavior: http://svn.apache.org/r1729876 The documentation at https://httpd.apache.org/docs/2.4/en/mod/core.html#sethandler is inconsistent: In the syntax definition, the value "none" is used, but there is also this sentence: "You can override an earlier defined SetHandler directive by using the value None." So I expect that both "none" and "None" should work. Example configuration that shows the problem (the send-as-is handler is not used anymore for asis files): SetHandler None AddType text/html .html .asis AddHandler send-as-is html asis Regards, Michael
[PATCH 57044] Use base64url in UNIQUE_ID
Hi, in bug 57044, I proposed to use base64url for UNIQUE_ID. This means that the character '_' shall be used instead of '@', because '@' is not URL-safe. '_' is, and it may be used without percent-encoding in URLs, HTTP headers, or cookie values. What do you think? Does anyone dare to commit the proposed patch (in trunk only) ? Regards, Michael
Re: LimitRequestBody is broken in 2.4.13-2.4.15
Thanks for reporting this before the testing/release. Fixed in r1688274 (will now propose a backport), and since this is a showstopper, it will be merged (once reviewed) before 2.4.16/2.2.30. Proposed patch (for backport) is http://people.apache.org/~ylavic/httpd-2.4.x-fix_LimitRequestBody.patch Thanks (again) for testing if that's possible. I have tested the patch, it works :-) Thank you very much! Regards, Michael
LimitRequestBody is broken in 2.4.13-2.4.15
Hi, LimitRequestBody is broken in the (unreleased) Apache versions 2.4.13-2.4.15 because of this change: http://svn.apache.org/r1684515 In http_filters.c, ap_http_filter(): The variable "totalread" is uninitialized if readbytes is 0. Messages similar to this one are logged: "AH01591: Read content-length of 140067070814864 is larger than the configured limit of 104857600", and then Apache closes the connection. I hope that it's possible to fix this for Apache 2.4.16. Regards, Michael
Re: [PATCH 57100] "SSLProtocol ALL" is ignored for virtual hosts
On Thu, Jan 22, 2015 at 8:27 AM, Michael Kaufmann wrote: Hi, It would be great if somebody finds time to review the proposed patch for bug 57100 (and maybe commit it to trunk). Any feedback would be greatly appreciated. -> https://issues.apache.org/bugzilla/show_bug.cgi?id=57100 Thanks, committed to trunk and proposed for 2.4.x. That was quick! Thank you very much for reviewing and committing :-) Michael
[PATCH 57100] "SSLProtocol ALL" is ignored for virtual hosts
Hi, It would be great if somebody finds time to review the proposed patch for bug 57100 (and maybe commit it to trunk). Any feedback would be greatly appreciated. -> https://issues.apache.org/bugzilla/show_bug.cgi?id=57100 Regards, Michael
