RE: [PATCH] Tomcat4.0: org.apache.catalina.connector.http.SocketI nput Stream

["Vanlerberghe, Luc" <lu...@bvdep.com>]

Oops, apparently I tend to misread do {} while like a pascal repeat...until
statement (not the first time).
I thought the loop was skipping the CR and LF characters...

Please ignore my previous mail...

The only remark that still holds is that the following:
>         if ((chr != CR) || (chr != LF)) {
>             pos--;
>         }
will always execute pos--; so either the test is obsolate or the writer
meant it differently.

Luc Vanlerberghe

> -----Original Message-----
> From: Vanlerberghe, Luc [mailto:luc.vanlerberghe@bvdep.com]
> Sent: Monday, January 08, 2001 12:02 PM
> To: 'tomcat-dev@jakarta.apache.org'
> Subject: [PATCH] Tomcat4.0:
> org.apache.catalina.connector.http.SocketInput Stream
> 
> 
> I noticed the possibility for an infinite loop in
> org.apache.catalina.connector.http.SocketInputStream
> after the commit on thursday...
> 
> I've attached a patch, but the diff is pretty unreadable...
> 
> I changed the following piece of code:
> 
>         do { // Skipping CR or LF
>             try {
>                 chr = read();
>             } catch (IOException e) {
>                 chr = -1;
>             }
>         } while ((chr == CR) || (chr == LF));
>         if (chr == -1)
>             throw new EOFException
>                 (sm.getString("requestStream.readline.error"));
>         if ((chr != CR) || (chr != LF)) {
>             pos--;
>         }
> 
> 1. When there's a IOException, there's no way out of the loop...
> 2. After the loop chr is neither CR nor LF, so the test looks 
> obsolete. pos
> should always be decremented.  (I didn't check why.  I only 
> looked at this
> piece of code, not the reasons behind the logic)
> 3. I moved the try {} catch outside the loop to make it a bit 
> clearer (a
> matter of taste, I guess but also I'm not so sure 
> entering/leaving a try {}
> catch doesn't involve a slight overhead)
> 
> This is the result:
> 
>         try {
>             do { // Skipping CR or LF
>                 chr = read();
>             } while ((chr == CR) || (chr == LF));
> 
>             pos--;
>         } catch (IOException e) {
>             throw new EOFException
>                 (sm.getString("requestStream.readline.error"));
>         }
> 
> I compiled the code and tested some of the example pages, so 
> I don't think I
> did any damage...
> 
> Luc Vanlerberghe
>  <<SocketInputStream.java.diff>> 
> 

Re: [PATCH] Tomcat4.0: org.apache.catalina.connector.http.SocketInput Stream

[Remy Maucherat <re...@apache.org>]

> Oops, apparently I tend to misread do {} while like a pascal
repeat...until
> statement (not the first time).
> I thought the loop was skipping the CR and LF characters...
>
> Please ignore my previous mail...
>
> The only remark that still holds is that the following:
> >         if ((chr != CR) || (chr != LF)) {
> >             pos--;
> >         }
> will always execute pos--; so either the test is obsolate or the writer
> meant it differently.

Yes I agree.

Remy