From aca4e6d3651358b9e7cb6de68c7c7672340e9008 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Tue, 29 Mar 2016 16:54:25 +0000 Subject: [PATCH] mod_http2: fix for wrong handling of prefetched response bodies git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1737021 13f79535-47bb-0310-9956-ffa450edef68 --- modules/http2/h2_filter.c | 6 +++--- modules/http2/h2_filter.h | 4 ++-- modules/http2/h2_io.c | 2 +- modules/http2/h2_session.c | 19 ++++++++++--------- modules/http2/h2_stream.c | 35 +++++++++++++++++++---------------- modules/http2/h2_stream.h | 4 ++-- modules/http2/h2_util.c | 2 +- 7 files changed, 38 insertions(+), 34 deletions(-) diff --git a/modules/http2/h2_filter.c b/modules/http2/h2_filter.c index c036b02fd4..8bf7fbcb40 100644 --- a/modules/http2/h2_filter.c +++ b/modules/http2/h2_filter.c @@ -275,9 +275,9 @@ static apr_status_t h2_sos_h2_status_read_to(h2_sos *sos, apr_bucket_brigade *bb return sos->prev->read_to(sos->prev, bb, plen, peos); } -static apr_status_t h2_sos_h2_status_prep_read(h2_sos *sos, apr_off_t *plen, int *peos) +static apr_status_t h2_sos_h2_status_prepare(h2_sos *sos, apr_off_t *plen, int *peos) { - return sos->prev->prep_read(sos->prev, plen, peos); + return sos->prev->prepare(sos->prev, plen, peos); } static apr_status_t h2_sos_h2_status_readx(h2_sos *sos, h2_io_data_cb *cb, void *ctx, @@ -304,7 +304,7 @@ static h2_sos *h2_sos_h2_status_create(h2_sos *prev) sos->response = response; sos->stream = prev->stream; sos->buffer = h2_sos_h2_status_buffer; - sos->prep_read = h2_sos_h2_status_prep_read; + sos->prepare = h2_sos_h2_status_prepare; sos->readx = h2_sos_h2_status_readx; sos->read_to = h2_sos_h2_status_read_to; sos->get_trailers = h2_sos_h2_status_get_trailers; diff --git a/modules/http2/h2_filter.h b/modules/http2/h2_filter.h index 9a38a9b9cb..2f281f8be1 100644 --- a/modules/http2/h2_filter.h +++ b/modules/http2/h2_filter.h @@ -47,7 +47,7 @@ typedef struct h2_sos h2_sos; typedef apr_status_t h2_sos_data_cb(void *ctx, const char *data, apr_off_t len); typedef apr_status_t h2_sos_buffer(h2_sos *sos, apr_bucket_brigade *bb); -typedef apr_status_t h2_sos_prep_read(h2_sos *sos, apr_off_t *plen, int *peos); +typedef apr_status_t h2_sos_prepare(h2_sos *sos, apr_off_t *plen, int *peos); typedef apr_status_t h2_sos_readx(h2_sos *sos, h2_sos_data_cb *cb, void *ctx, apr_off_t *plen, int *peos); typedef apr_status_t h2_sos_read_to(h2_sos *sos, apr_bucket_brigade *bb, @@ -63,7 +63,7 @@ struct h2_sos { struct h2_response *response; void *ctx; h2_sos_buffer *buffer; - h2_sos_prep_read *prep_read; + h2_sos_prepare *prepare; h2_sos_readx *readx; h2_sos_read_to *read_to; h2_sos_get_trailers *get_trailers; diff --git a/modules/http2/h2_io.c b/modules/http2/h2_io.c index 64cb8b0178..d3c0e6c04d 100644 --- a/modules/http2/h2_io.c +++ b/modules/http2/h2_io.c @@ -355,7 +355,7 @@ apr_status_t h2_io_out_get_brigade(h2_io *io, apr_bucket_brigade *bb, if (io->eos_out_read) { return APR_EOF; } - else if (!io->bbout) { + else if (!io->bbout || APR_BRIGADE_EMPTY(io->bbout)) { return APR_EAGAIN; } else { diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 8111d93b66..fe4c843510 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -1109,7 +1109,12 @@ static int resume_on_data(void *ctx, void *val) AP_DEBUG_ASSERT(stream); if (h2_stream_is_suspended(stream)) { - if (h2_mplx_out_has_data_for(stream->session->mplx, stream->id)) { + apr_status_t status; + apr_off_t len = -1; + int eos; + + status = h2_stream_out_prepare(stream, &len, &eos); + if (status == APR_SUCCESS) { int rv; h2_stream_set_suspended(stream, 0); ++rctx->resume_count; @@ -1118,8 +1123,9 @@ static int resume_on_data(void *ctx, void *val) ap_log_cerror(APLOG_MARK, nghttp2_is_fatal(rv)? APLOG_ERR : APLOG_DEBUG, 0, session->c, APLOGNO(02936) - "h2_stream(%ld-%d): resuming %s", - session->id, stream->id, rv? nghttp2_strerror(rv) : ""); + "h2_stream(%ld-%d): resuming %s, len=%ld, eos=%d", + session->id, stream->id, + rv? nghttp2_strerror(rv) : "", (long)len, eos); } } return 1; @@ -1184,7 +1190,7 @@ static ssize_t stream_data_cb(nghttp2_session *ng2s, AP_DEBUG_ASSERT(!h2_stream_is_suspended(stream)); - status = h2_stream_prep_read(stream, &nread, &eos); + status = h2_stream_out_prepare(stream, &nread, &eos); if (nread) { *data_flags |= NGHTTP2_DATA_FLAG_NO_COPY; } @@ -1209,11 +1215,6 @@ static ssize_t stream_data_cb(nghttp2_session *ng2s, session->id, (int)stream_id); return NGHTTP2_ERR_DEFERRED; - case APR_EOF: - nread = 0; - eos = 1; - break; - default: nread = 0; ap_log_cerror(APLOG_MARK, APLOG_ERR, status, session->c, diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index fc41fe352d..0aaed8c42e 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -387,17 +387,17 @@ int h2_stream_is_suspended(const h2_stream *stream) return stream->suspended; } -apr_status_t h2_stream_prep_read(h2_stream *stream, - apr_off_t *plen, int *peos) +apr_status_t h2_stream_out_prepare(h2_stream *stream, + apr_off_t *plen, int *peos) { if (stream->rst_error) { + *plen = 0; + *peos = 1; return APR_ECONNRESET; } - if (!stream->sos) { - return APR_EGENERAL; - } - return stream->sos->prep_read(stream->sos, plen, peos); + AP_DEBUG_ASSERT(stream->sos); + return stream->sos->prepare(stream->sos, plen, peos); } apr_status_t h2_stream_readx(h2_stream *stream, @@ -516,8 +516,8 @@ static apr_status_t mplx_transfer(h2_sos_mplx *msos, int stream_id, } status = h2_mplx_out_get_brigade(msos->m, stream_id, msos->tmp, msos->buffer_size-1, &trailers); - if (status == APR_SUCCESS) { - status = h2_transfer_brigade(msos->bb, msos->tmp, pool); + if (!APR_BRIGADE_EMPTY(msos->tmp)) { + h2_transfer_brigade(msos->bb, msos->tmp, pool); } if (trailers) { msos->trailers = trailers; @@ -534,6 +534,9 @@ static apr_status_t h2_sos_mplx_read_to(h2_sos *sos, apr_bucket_brigade *bb, status = h2_append_brigade(bb, msos->bb, plen, peos); if (status == APR_SUCCESS && !*peos && !*plen) { status = APR_EAGAIN; + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, msos->m->c, + "h2_stream(%ld-%d): read_to, len=%ld eos=%d", + msos->m->id, sos->stream->id, (long)*plen, *peos); } ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, msos->m->c, "h2_stream(%ld-%d): read_to, len=%ld eos=%d", @@ -551,30 +554,30 @@ static apr_status_t h2_sos_mplx_readx(h2_sos *sos, h2_io_data_cb *cb, void *ctx, if (status == APR_SUCCESS && !*peos && !*plen) { status = APR_EAGAIN; } - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, msos->m->c, + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, msos->m->c, "h2_stream(%ld-%d): readx, len=%ld eos=%d", msos->m->id, sos->stream->id, (long)*plen, *peos); return status; } -static apr_status_t h2_sos_mplx_prep_read(h2_sos *sos, apr_off_t *plen, int *peos) +static apr_status_t h2_sos_mplx_prepare(h2_sos *sos, apr_off_t *plen, int *peos) { h2_sos_mplx *msos = sos->ctx; apr_status_t status = APR_SUCCESS; - H2_SOS_MPLX_OUT(APLOG_TRACE2, msos, "h2_sos_mplx prep_read_pre"); + H2_SOS_MPLX_OUT(APLOG_TRACE2, msos, "h2_sos_mplx prepare_pre"); if (APR_BRIGADE_EMPTY(msos->bb)) { status = mplx_transfer(msos, sos->stream->id, sos->stream->pool); } - status = h2_util_bb_avail(msos->bb, plen, peos); + h2_util_bb_avail(msos->bb, plen, peos); - H2_SOS_MPLX_OUT(APLOG_TRACE2, msos, "h2_sos_mplx prep_read_post"); + H2_SOS_MPLX_OUT(APLOG_TRACE2, msos, "h2_sos_mplx prepare_post"); ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, msos->m->c, - "h2_stream(%ld-%d): prep_read, len=%ld eos=%d, trailers=%s", + "h2_stream(%ld-%d): prepare, len=%ld eos=%d, trailers=%s", msos->m->id, sos->stream->id, (long)*plen, *peos, msos->trailers? "yes" : "no"); - if (status == APR_SUCCESS && !*peos && !*plen) { + if (!*peos && !*plen) { status = APR_EAGAIN; } @@ -617,7 +620,7 @@ static h2_sos *h2_sos_mplx_create(h2_stream *stream, h2_response *response) sos->ctx = msos; sos->buffer = h2_sos_mplx_buffer; - sos->prep_read = h2_sos_mplx_prep_read; + sos->prepare = h2_sos_mplx_prepare; sos->readx = h2_sos_mplx_readx; sos->read_to = h2_sos_mplx_read_to; sos->get_trailers = h2_sos_mplx_get_trailers; diff --git a/modules/http2/h2_stream.h b/modules/http2/h2_stream.h index b7df632502..a861b894e1 100644 --- a/modules/http2/h2_stream.h +++ b/modules/http2/h2_stream.h @@ -203,8 +203,8 @@ apr_status_t h2_stream_set_response(h2_stream *stream, * APR_EAGAIN if not data is available and end of stream has not been * reached yet. */ -apr_status_t h2_stream_prep_read(h2_stream *stream, - apr_off_t *plen, int *peos); +apr_status_t h2_stream_out_prepare(h2_stream *stream, + apr_off_t *plen, int *peos); /** * Read data from the stream output. diff --git a/modules/http2/h2_util.c b/modules/http2/h2_util.c index 002284a536..06472425f2 100644 --- a/modules/http2/h2_util.c +++ b/modules/http2/h2_util.c @@ -697,7 +697,7 @@ apr_status_t h2_util_bb_avail(apr_bucket_brigade *bb, return status; } else if (blen == 0) { - /* empty brigade, does it have an EOS bucket somwhere? */ + /* brigade without data, does it have an EOS bucket somwhere? */ *plen = 0; *peos = h2_util_has_eos(bb, -1); } -- 2.50.1