On 02/04/2017 09:14 PM, [email protected] wrote: > Author: druggeri > Date: Sat Feb 4 20:14:18 2017 > New Revision: 1781701 > > URL: http://svn.apache.org/viewvc?rev=1781701&view=rev > Log: > Change tactic for PROXY processing in Optional case > > Modified: > httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml > httpd/httpd/trunk/modules/metadata/mod_remoteip.c >
> Modified: httpd/httpd/trunk/modules/metadata/mod_remoteip.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_remoteip.c?rev=1781701&r1=1781700&r2=1781701&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original) > +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Sat Feb 4 20:14:18 2017 > @@ -1085,28 +1091,53 @@ static apr_status_t remoteip_input_filte > memcpy(ctx->header + ctx->rcvd, ptr, len); > ctx->rcvd += len; > > - /* Put this bucket into our temporary storage brigade because > - we may permit a request without a header. In that case, the > - data in the temporary storage brigade just gets copied > - to bb_out */ > - APR_BUCKET_REMOVE(b); > - apr_bucket_setaside(b, f->c->pool); > - APR_BRIGADE_INSERT_TAIL(ctx->store_bb, b); > + apr_bucket_delete(b); > psts = HDR_NEED_MORE; > > if (ctx->version == 0) { > /* reading initial chunk */ > if (ctx->rcvd >= MIN_HDR_LEN) { > ctx->version = remoteip_determine_version(f->c, > ctx->header); > - if (ctx->version < 0) { > - psts = HDR_MISSING; > - } > - else if (ctx->version == 1) { > - ctx->mode = AP_MODE_GETLINE; > - ctx->need = sizeof(proxy_v1); > + > + /* We've read enough to know that a header is present. > In peek mode > + we purge the bb and can decide to step aside or > switch to > + non-speculative read to consume the data */ > + if (ctx->peeking) { > + apr_brigade_destroy(ctx->bb); apr_brigade_cleanup is better instead of destroy and create afterwards. > + > + if (ctx->version < 0) { > + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c, > APLOGNO(03512) > + "RemoteIPProxyProtocol: PROXY > header is missing from " > + "request. Stepping aside."); > + ctx->bb = NULL; > + ctx->done = 1; > + return ap_get_brigade(f->next, bb_out, mode, > block, readbytes); > + } > + else { > + /* Rest ctx to initial values */ > + ctx->rcvd = 0; > + ctx->need = MIN_HDR_LEN; > + ctx->version = 0; > + ctx->bb = apr_brigade_create(f->c->pool, > f->c->bucket_alloc); > + ctx->done = 0; > + ctx->mode = AP_MODE_READBYTES; > + ctx->peeking = 0; > + > + ap_get_brigade(f->next, ctx->bb, ctx->mode, > block, > + ctx->need - ctx->rcvd); > + } > } > - else if (ctx->version == 2) { > - ctx->need = MIN_V2_HDR_LEN; > + else { > + if (ctx->version < 0) { > + psts = HDR_MISSING; > + } > + else if (ctx->version == 1) { > + ctx->mode = AP_MODE_GETLINE; > + ctx->need = sizeof(proxy_v1); > + } > + else if (ctx->version == 2) { > + ctx->need = MIN_V2_HDR_LEN; > + } > } > } > } Other comments: If you are reading in SPECULATIVE mode and renter the filter (e.g. MIN_HDR_LEN bytes were not available and you were reading non blocking) or if you just do a second ap_get_brigade in the outer loop, the returned brigade will contain all the data you had already seen plus potential new data. So you don't need to tuck it away in ctx->header. You always need to examine the whole returned brigade for the header and the brigade should become longer with a second ap_get_brigade. If not and you are in non blocking mode no new data was available and you should leave. Also take care to handle meta buckets correctly. You could probably hit an EOS bucket which tells you that no more data will be delivered. Regards Rüdiger
