Bug #14620 for POE-Component-Client-HTTP: apache 1.3 may include spaces before the optional "chunk-extension"

Fri Sep 16 13:29:17 2005 Guest - Ticket created

Subject:

apache 1.3 may include spaces before the optional "chunk-extension"

POE::Filter::HTTPChunk seems to follow RFC2616 3.6.1 fairly closely (snippet below). Unfortunately, apache 1.3 seems to include trailing spaces after the chunk-size and before the CRLF when there's no chunk-extension (and possibly when there is, although I can't force that condition). This results in the following debug statements: Finding chunk length marker at /home/y/lib/perl5/site_perl/5.6.1/POE/Filter/HTTPChunk.pm line 93. DIDN'T FIND CHUNK LENGTH 5b The enclosed patch which adds \s* to the chunk-length search regexp fixes it for me. Please apply the patch and re-release. Chunked-Body = *chunk last-chunk trailer CRLF chunk = chunk-size [ chunk-extension ] CRLF chunk-data CRLF chunk-size = 1*HEX last-chunk = 1*("0") [ chunk-extension ] CRLF chunk-extension= *( ";" chunk-ext-name [ "=" chunk-ext-val ] ) chunk-ext-name = token chunk-ext-val = token | quoted-string chunk-data = chunk-size(OCTET) trailer = *(entity-header CRLF)

Download patch
application/octet-stream 450b

Message body not shown because it is not plain text.

Sat Sep 17 13:46:15 2005 RCAPUTO [...] cpan.org - Correspondence added

Thanks for the patch. It turns out that \s* is a little too indiscriminate about the whitespace it eats. Thankfully we have a regression test that caught the problem, or I'd never have noticed. Your patch, as applied, uses [^\S\015\012]* instead. Thanks again.

Sat Sep 17 13:46:16 2005 RCAPUTO [...] cpan.org - Status changed from 'new' to 'resolved'

Sun Sep 18 14:11:59 2005 Guest - Correspondence added

From:

scr14 [...] cornell.edu

[RCAPUTO - Sat Sep 17 13:46:15 2005]: Show quoted text

> Thanks for the patch. It turns out that \s* is a little too > indiscriminate about the whitespace it eats. Thankfully we have a > regression test that caught the problem, or I'd never have noticed. > > Your patch, as applied, uses [^\S\015\012]* instead. Thanks again.

I'm curious -- would \s*? have worked?? Adding the ? says non-greedy, which means it would stop as soon as the rest of the match string matched rather than continuing to eat... Just a thought -- technically your match string might short-circuit if there was a \r in the middle of the string which was not followed by \n... Either way, when do you expect to package a new version for CPAN consumption?

Sun Sep 18 14:12:00 2005 The RT System itself - Status changed from 'resolved' to 'open'

Sun Sep 18 14:27:52 2005 RCAPUTO [...] cpan.org - Correspondence added

[guest - Sun Sep 18 14:11:59 2005]: Show quoted text

> [RCAPUTO - Sat Sep 17 13:46:15 2005]: >

> > Thanks for the patch. It turns out that \s* is a little too > > indiscriminate about the whitespace it eats. Thankfully we have a > > regression test that caught the problem, or I'd never have noticed. > > > > Your patch, as applied, uses [^\S\015\012]* instead. Thanks again.

> > > I'm curious -- would \s*? have worked?? > > Adding the ? says non-greedy, which means it would stop as soon as the > rest of the match string matched rather than continuing to eat... Just > a thought -- technically your match string might short-circuit if there > was a \r in the middle of the string which was not followed by \n...

The full match is $chunk =~ s/^($HEX+)[^\S\015\012]*(?:;.*?)?[^\S\015\012]*\015?\012//s . The other [^\S\015\012]* was added to fix a bug where \s* was eating up the newline. I think \s*? would also work, but it would require the match to bactrack more than necessary. Show quoted text

> Either way, when do you expect to package a new version for CPAN > consumption?

Within the next day or two, I think. I've been applying a lot of patches lately, and things are nearly stable enough to push another release. I have one active issue to go.

Sun Sep 18 14:27:53 2005 RCAPUTO [...] cpan.org - Status changed from 'open' to 'resolved'