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

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

> 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...

No, since I set chr to -1.
It stays in the loop only if chr is CR or LF.

> 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)

Well, ok.

> 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)

Remy