You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ma...@locus.apache.org on 2000/03/02 22:23:40 UTC

cvs commit: apache-1.3/src/main buff.c

martin      00/03/02 13:23:40

  Modified:    src      CHANGES
               src/main buff.c
  Log:
  Recent global changes in CRLF handling broke chunked encoding. Until
  now, I hadn't noticed until now because I always use a HTTP/1.0 proxy in
  front of the origin servers. Now the trailing CRLF after a chunk is sent
  as a constant "pre-encoded ASCII CRLF".
  
  Revision  Changes    Path
  1.1523    +3 -0      apache-1.3/src/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/CHANGES,v
  retrieving revision 1.1522
  retrieving revision 1.1523
  diff -u -r1.1522 -r1.1523
  --- CHANGES	2000/03/01 09:18:55	1.1522
  +++ CHANGES	2000/03/02 21:23:36	1.1523
  @@ -1,5 +1,8 @@
   Changes with Apache 1.3.13
   
  +  *) [EBCDIC] Make chunked encoding work again; it was broken by the
  +     recent CRLF macro changes. An oversight. [Martin Kraemer]
  +
     *) Work around a popular restriction of some sed(1)'s in APACI where
        "1,/<pattern>/" commands start searching for <pattern> at line 2 only.
        [Ralf S. Engelschall]
  
  
  
  1.95      +8 -3      apache-1.3/src/main/buff.c
  
  Index: buff.c
  ===================================================================
  RCS file: /home/cvs/apache-1.3/src/main/buff.c,v
  retrieving revision 1.94
  retrieving revision 1.95
  diff -u -r1.94 -r1.95
  --- buff.c	2000/01/11 15:51:32	1.94
  +++ buff.c	2000/03/02 21:23:39	1.95
  @@ -82,6 +82,11 @@
   #define CHUNK_HEADER_SIZE (5)
   #endif
   
  +#ifdef CHARSET_EBCDIC
  +#define ascii_CRLF "\015\012" /* A CRLF which won't pass the conversion machinery */
  +#else
  +#define ascii_CRLF CRLF
  +#endif
   
   /* bwrite()s of greater than this size can result in a large_write() call,
    * which can result in a writev().  It's a little more work to set up the
  @@ -1157,7 +1162,7 @@
   	return -1;
       if (write_it_all(fb, buf, nbyte) == -1)
   	return -1;
  -    if (write_it_all(fb, CRLF, 2) == -1)
  +    if (write_it_all(fb, ascii_CRLF, 2) == -1)
   	return -1;
       return nbyte;
   #else
  @@ -1170,7 +1175,7 @@
   #endif /*CHARSET_EBCDIC*/
       vec[1].iov_base = (void *) buf;	/* cast is to avoid const warning */
       vec[1].iov_len = nbyte;
  -    vec[2].iov_base = CRLF;
  +    vec[2].iov_base = ascii_CRLF;
       vec[2].iov_len = 2;
   
       return writev_it_all(fb, vec, (sizeof(vec) / sizeof(vec[0]))) ? -1 : nbyte;
  @@ -1211,7 +1216,7 @@
   	vec[nvec].iov_base = (void *) buf;
   	vec[nvec].iov_len = nbyte;
   	++nvec;
  -	vec[nvec].iov_base = CRLF;
  +	vec[nvec].iov_base = ascii_CRLF;
   	vec[nvec].iov_len = 2;
   	++nvec;
       }
  
  
  

Re: cvs commit: apache-1.3/src/main buff.c

Posted by Greg Ames <gr...@raleigh.ibm.com>.

Martin Kraemer wrote:

> I used netcat on the output of mod_info, like this:

mod_info did the trick for me.  It fails every time on OS/390 using IE5 without your patch, and works every
time with your patch.  I owe you another beer, Martin!

I did run mod_rndchunk using netcat | check_chunked manually but didn't capture any failures that way,
which is kind of puzzling.  Jeff Trawick gave me a Perl script to automate this (thanks!).  I'll try it
after ApacheCon - see you all there.

Greg Ames


Re: cvs commit: apache-1.3/src/main buff.c

Posted by Martin Kraemer <Ma...@Mch.SNI.De>.
On Fri, Mar 03, 2000 at 05:45:04PM -0500, Greg Ames wrote:
> I assume it's broken on OS/390 also, so I'm trying to test it to demonstrate the
> brokenness.  I'm using mod_rndchunk on 390 with IE, see a bunch of somewhat random data
> similar to what I see from Linux, and IE isn't complaining.  However IE could be tolerating
> garbage for all I know.

Ha! Yes, the "similar" is the key. It must be *identical*! At one
tiny place, you'll notice that the mainframe returns a ^U character in
place of a newline. But the HTTP/1.1 protocol requires that this be
a '\n'!

> next step: learn how to use netcat, and feed netcat's output into check_chunked.  Is there
> an easier way to test chunking?

I used netcat on the output of mod_info, like this:

echo "GET /server-info HTTP/1.1
Host: $DESTHOST:$DESTPORT
Connection: close
" | netcat $DESTHOST $DESTPORT | tee test.tmp | ../../check_chunked || Fail Chunking error in $TESTNAME

(extracted from my script, substitute the obvious variables and
shell functions). It uses Dean's check_chunked Perl script, look in
the CVS repository.

The effect of the error was that IE5 refused to connect to the site
completely (unless a HTTP/1.0 proxy was in between, or MSIE was
configured to force-downgrade-1.0 by configuration). The effect
should be identical on OS/390 (or any other machine where '\n' != '\012')

    Martin
-- 
  <Ma...@MchP.Siemens.De>      |       Fujitsu Siemens
       <ma...@apache.org>              |   81730  Munich,  Germany
((See you at ApacheCon 2000 in Orlanda, Florida, March 8-10, 2000!))
		   <URL:http://ApacheCon.Com/>

Re: cvs commit: apache-1.3/src/main buff.c

Posted by Greg Ames <gr...@raleigh.ibm.com>.

martin@locus.apache.org wrote:

>   Modified:    src      CHANGES
>                src/main buff.c
>   Log:
>   Recent global changes in CRLF handling broke chunked encoding.

I assume it's broken on OS/390 also, so I'm trying to test it to demonstrate the
brokenness.  I'm using mod_rndchunk on 390 with IE, see a bunch of somewhat random data
similar to what I see from Linux, and IE isn't complaining.  However IE could be tolerating
garbage for all I know.

next step: learn how to use netcat, and feed netcat's output into check_chunked.  Is there
an easier way to test chunking?

Thanks,

Greg Ames


Re: cvs commit: apache-1.3/src/main buff.c

Posted by Martin Kraemer <Ma...@Mch.SNI.De>.
On Thu, Mar 02, 2000 at 03:13:34PM -0700, pg@sweng.stortek.com wrote:
> In a recent note, Greg Stein said:
> 
> I agree.  Both path through this are equivalent to the unconditional:
> 
> > >   +#define ascii_CRLF "\015\012" /* A CRLF which won't pass the conversion machinery */
> 

Good point. I'll commit it like this if it makes the intention
clearer.
(Of course they are identical. What I wanted to do is retain the
mnemonic name while indicating that -in contrast to the normal CRFL
macro- this has already been "converted to" ASCII and must therefore
not be submitted to the translation machinery again).

    Martin
-- 
  <Ma...@MchP.Siemens.De>      |       Fujitsu Siemens
       <ma...@apache.org>              |   81730  Munich,  Germany
((See you at ApacheCon 2000 in Orlanda, Florida, March 8-10, 2000!))
		   <URL:http://ApacheCon.Com/>

Re: cvs commit: apache-1.3/src/main buff.c

Posted by Greg Stein <gs...@lyra.org>.
Why should the ascii_CRLF macro even exist? The protocol is explicit that
those characters must be \015\012. That constant should be used rather
than a macro.

If you *do* want to retain a macro (for symbolic purposes), then please
force it to always be "\015\012" rather than using an #ifdef and possibly
depending on CRLF.

Cheers,
-g

On 2 Mar 2000 martin@locus.apache.org wrote:
> martin      00/03/02 13:23:40
> 
>   Modified:    src      CHANGES
>                src/main buff.c
>   Log:
>   Recent global changes in CRLF handling broke chunked encoding. Until
>   now, I hadn't noticed until now because I always use a HTTP/1.0 proxy in
>   front of the origin servers. Now the trailing CRLF after a chunk is sent
>   as a constant "pre-encoded ASCII CRLF".
>   
>   Revision  Changes    Path
>   1.1523    +3 -0      apache-1.3/src/CHANGES
>   
>   Index: CHANGES
>   ===================================================================
>   RCS file: /home/cvs/apache-1.3/src/CHANGES,v
>   retrieving revision 1.1522
>   retrieving revision 1.1523
>   diff -u -r1.1522 -r1.1523
>   --- CHANGES	2000/03/01 09:18:55	1.1522
>   +++ CHANGES	2000/03/02 21:23:36	1.1523
>   @@ -1,5 +1,8 @@
>    Changes with Apache 1.3.13
>    
>   +  *) [EBCDIC] Make chunked encoding work again; it was broken by the
>   +     recent CRLF macro changes. An oversight. [Martin Kraemer]
>   +
>      *) Work around a popular restriction of some sed(1)'s in APACI where
>         "1,/<pattern>/" commands start searching for <pattern> at line 2 only.
>         [Ralf S. Engelschall]
>   
>   
>   
>   1.95      +8 -3      apache-1.3/src/main/buff.c
>   
>   Index: buff.c
>   ===================================================================
>   RCS file: /home/cvs/apache-1.3/src/main/buff.c,v
>   retrieving revision 1.94
>   retrieving revision 1.95
>   diff -u -r1.94 -r1.95
>   --- buff.c	2000/01/11 15:51:32	1.94
>   +++ buff.c	2000/03/02 21:23:39	1.95
>   @@ -82,6 +82,11 @@
>    #define CHUNK_HEADER_SIZE (5)
>    #endif
>    
>   +#ifdef CHARSET_EBCDIC
>   +#define ascii_CRLF "\015\012" /* A CRLF which won't pass the conversion machinery */
>   +#else
>   +#define ascii_CRLF CRLF
>   +#endif
>    
>    /* bwrite()s of greater than this size can result in a large_write() call,
>     * which can result in a writev().  It's a little more work to set up the
>   @@ -1157,7 +1162,7 @@
>    	return -1;
>        if (write_it_all(fb, buf, nbyte) == -1)
>    	return -1;
>   -    if (write_it_all(fb, CRLF, 2) == -1)
>   +    if (write_it_all(fb, ascii_CRLF, 2) == -1)
>    	return -1;
>        return nbyte;
>    #else
>   @@ -1170,7 +1175,7 @@
>    #endif /*CHARSET_EBCDIC*/
>        vec[1].iov_base = (void *) buf;	/* cast is to avoid const warning */
>        vec[1].iov_len = nbyte;
>   -    vec[2].iov_base = CRLF;
>   +    vec[2].iov_base = ascii_CRLF;
>        vec[2].iov_len = 2;
>    
>        return writev_it_all(fb, vec, (sizeof(vec) / sizeof(vec[0]))) ? -1 : nbyte;
>   @@ -1211,7 +1216,7 @@
>    	vec[nvec].iov_base = (void *) buf;
>    	vec[nvec].iov_len = nbyte;
>    	++nvec;
>   -	vec[nvec].iov_base = CRLF;
>   +	vec[nvec].iov_base = ascii_CRLF;
>    	vec[nvec].iov_len = 2;
>    	++nvec;
>        }
>   
>   
>   
> 

-- 
Greg Stein, http://www.lyra.org/