I have reverse proxy with backend webserver enabled with both SSL and gzip compression. For certain application backend webserver is sending only 1 byte of data for first time, and then it sends around 4k of data. Below is gdb output from mod_deflate in function "inflate_out_filter" ============================== (gdb) p data $6 = 0x7f3e559ef700 "\037" (gdb) p len $7 = 1 Second time 4495 byte comes to deflate (gdb) p len $10 = 4495 (gdb) p data $11 = 0x7f3e559ef700 "\213\b" =============================== As gzip header is 10 bytes, and first data is only 1 byte, mod_deflate is not able to handle this, even-though next time it is sending remaining data, and giving error as "Insufficient data for inflate"( Full log is attached) and for second data which 4495 bytes its giving error as "Validation bytes not present". Application on Backend webserver works fine if disabled with SSL, and also works fine if disabled with gzip.
Created attachment 30936 [details] Error log file of apache(reverse proxy), log is stripped with encrypted data.
Created attachment 30937 [details] mod_deflate inflate header buffering (2.2.x) Can you verify this patch works for you ?
Created attachment 30938 [details] mod_deflate inflate header buffering (trunk/2.4.x) Or maybe on trunk/2.4.x ?
Created attachment 30939 [details] mod_deflate inflate header buffering (trunk/2.4.x) This the good (corresponding) trunk/2.4.x patch.
Created attachment 30959 [details] modified diff Thank you Yann Ylavic for providing a fix. I tried your fix, but did not work straight forward, I modified little bit to make it work. attaching the modified version of diff. Thank you once again.
Created attachment 30960 [details] mod_deflate inflate header+flags buffering (2.2.x) Here is a more complete patch for the inflate_out_filter (including your fix) which also takes care of flags truncation (ie. EXTRA_FIELD, ORIG_NAME, COMMENT, and HEAD_CRC), in addition to the header truncation. It also fixes header truncated (and reentrance in non-blocking mode) for the deflate_in_filter (which in fact inflates the input). Can you test that one on your side for 2.2.x? trunk/2.4.x is coming next, if all is good.
Created attachment 30961 [details] mod_deflate inflate header+flags+crc buffering (2.2.x) A new patch that also handles CRC verification reentrance. Also, the parsing/stripping of the zlib flags is now done by a function which can be used both by inflate_out_filter (as existing) and deflate_in_filter (change the #if 0 to 1 to use it there too, for now it is disabled to preserve the existing behaviour). Don"t use the previous patch since it may cause an infinite loop (fixed here).
(In reply to Yann Ylavic from comment #7) > Created attachment 30961 [details] > mod_deflate inflate header+flags+crc buffering (2.2.x) > > A new patch that also handles CRC verification reentrance. > > Also, the parsing/stripping of the zlib flags is now done by a function > which can be used both by inflate_out_filter (as existing) and > deflate_in_filter (change the #if 0 to 1 to use it there too, for now it is > disabled to preserve the existing behaviour). > > Don"t use the previous patch since it may cause an infinite loop (fixed > here). Thanks on this issue too. Would it be possible to stage these patches in some way for review purposes? I know it's painful to go backwards.
Created attachment 30962 [details] mod_deflate inflate header+flags+crc buffering (2.2.x) Fix test APR_BRIGADE_EMPTY(bb) => APR_BRIGADE_EMPTY(ctx->bb) in the non-blocking case.
(In reply to Eric Covener from comment #8) > (In reply to Yann Ylavic from comment #7) > > Created attachment 30961 [details] > > mod_deflate inflate header+flags+crc buffering (2.2.x) > > > > A new patch that also handles CRC verification reentrance. > > > > Also, the parsing/stripping of the zlib flags is now done by a function > > which can be used both by inflate_out_filter (as existing) and > > deflate_in_filter (change the #if 0 to 1 to use it there too, for now it is > > disabled to preserve the existing behaviour). > > > > Don"t use the previous patch since it may cause an infinite loop (fixed > > here). > > Thanks on this issue too. > > Would it be possible to stage these patches in some way for review purposes? > I know it's painful to go backwards. I'm front-porting to trunk now and I'll provide test cases. Is that what you mean?
> > > > Would it be possible to stage these patches in some way for review purposes? > > I know it's painful to go backwards. > > I'm front-porting to trunk now and I'll provide test cases. > Is that what you mean? I meant in terms of several small patches vs. a comprehensive one (where possible) so they can be individually digested/reviewed/committed withou thaving to absorb the entire thing in aggregate.
Created attachment 30973 [details] inflate output filter header buffering (trunk 1/6) First of six patches to handle inflate reentrance in the mod_deflate's output and input filters. Patch 1. Handle Zlib header buffering in the inflate output filter : - adds the new deflate_ctx_t fields needed to re-enter the Zlib header parsing, - declares the new consume_zlib_flags() function to parse/consume the ZLib flags (will be used for the input filter too in a following patch), - fixes a (possible) size_t underflow when parsing/consuming ZLib nul-terminated string flags (eg. "while (len-- && *data++);" could underflow len if the nul is not reached in the bucket). This patch fixes the current PR, but the issue still remains for the inflate input filter (hence the following patches).
Created attachment 30974 [details] inflate input filter header buffering (trunk 2/6) Patch 2. Handle Zlib header buffering in the inflate input filter : - uses the new deflate_ctx_t fields from the previous patch 1, - fixes the double ap_get_brigade() when an EOS brigade is encountered while reading the header, eg : /* zero length body? step aside */ bkt = APR_BRIGADE_FIRST(ctx->bb); if (APR_BUCKET_IS_EOS(bkt)) { + APR_BUCKET_REMOVE(bkt); + APR_BRIGADE_INSERT_TAIL(bb, bkt); ap_remove_input_filter(f); - return ap_get_brigade(f->next, bb, mode, block, readbytes); + return APR_SUCCESS; }
Created attachment 30975 [details] inflate input filter validation bytes buffering (trunk 3/6) Patch 3. Handle Zlib validation bytes buffering (CRC + length, eg. footer).
Created attachment 30976 [details] inflate input filter with non-blocking read (trunk 4/6) Patch 4. In the inflate input filter, return immediatly when a non-blocking read would block. Note that a non-blocking read which would block while reading the ZLib header is handled by patch 2 with : + rv = ap_get_brigade(f->next, ctx->bb, AP_MODE_READBYTES, block, + len); + + /* ap_get_brigade may return success with an empty brigade for + * a non-blocking read which would block (an empty brigade for + * a blocking read is an issue which is simply forwarded here). + */ + if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(ctx->bb)) { return rv; }
Created attachment 30977 [details] inflate input filter with Zlib flags (trunk 5/6) Patch 5. Handle Zlib flags in the input filter as already existing in the output filter, using the common consume_zlib_flags().
Created attachment 30978 [details] mod_deflate safety splits (trunk 6/6) This latest patch 6 ensures that bucket don't exceed INT_MAX length (Zlib lengths are ints), or split the bucket (should one ask to inflate a 3gig tarball, the CPU would now burn before mod_deflate :p ). This patch also takes the fast path if the bucket is empty.
Created attachment 30979 [details] mod_deflate all in one patch (trunk) Finally the full patch, with the correct indentation (which has sometimes been muted in the previous patches, that was to show that unchanged code have simply been put into lower blocks). Thanks for reviewing these (smaller) patches. Regards.
Created attachment 30983 [details] mod_deflate all in one patch (2.2.x) At last, the full patch for 2.2.x (should this be backported). Note I also merged the error (failure) logs from trunk which did not exist in 2.2.x (where most Zlib errors are not logged). That may be a behaviour change per se, so let me know if you want me to revert them.
Sorry,Yann Ylavic i don't have proper setup now to test this.
Bug 46146 is older. I plan to commit the patches from here and report there. *** This bug has been marked as a duplicate of bug 46146 ***