Jetty

Bug in HttpGenerator.OutputWriter.write()

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 6.0.0beta17
  • Fix Version/s: 6.0.0beta18, 6.0.0RC0
  • Component/s: None
  • Labels:
    None
  • Environment:
    Linux, special characters in HTML data
  • Number of attachments :
    0

Description

There is a bug in jetty-6.0.0beta17\modules\jetty\src\main\java\org\mortbay\jetty\HttpGenerator.java at line 1244.
This is within the method OutputWriter.write().

The faulty line 1244 is:
_converter.write(_chars,i0,i-i0);
It should be:
_converter.write(_chars,i0,n-i0);
(n must be used instead of i)

And line 1241:
n+=_writeChunk/2;
should be changed to:
n+=(_writeChunk-1)/2;
because the _bytes buffer is only _writeChunk*2 long and n has already been increased by 1 at the previous line.

The stack trace for this error is:

java.lang.IndexOutOfBoundsException
at sun.nio.cs.StreamEncoder.write(Unknown Source)
at java.io.OutputStreamWriter.write(Unknown Source)
at org.mortbay.jetty.HttpGenerator$OutputWriter.write(HttpGenerator.java:1244)
at java.io.PrintWriter.write(Unknown Source)
at java.io.PrintWriter.write(Unknown Source)
at ApplServletResponseSender.sendTextResponse(Unknown Source)
at ApplServletResponseSender.sendHttpResponse(Unknown Source)
...

Is there a guarantee that _converter only produces at most twice as much bytes as the number of characters feed? (UTF8 may produce up to 4 bytes per character, but the UTF8 case seems to be already handled separately on line 1198.)

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Christian d'Heureuse added a comment -

Correction: The change at line 1241 is probably not necessary, because the term _writeChunk/2 is not to prevent a buffer overflow. The outer loop continues to fill the buffer.
But line 1244 is a real bug.

Show
Christian d'Heureuse added a comment - Correction: The change at line 1241 is probably not necessary, because the term _writeChunk/2 is not to prevent a buffer overflow. The outer loop continues to fill the buffer. But line 1244 is a real bug.
Hide
Permalink
Greg Wilkins added a comment -

Thanks for the bug report.
Fix going into svn now.

Note that I have reduced the chunklet size to be _writeChunk/6 because aUTF8
character may be 6 bytes.

Show
Greg Wilkins added a comment - Thanks for the bug report. Fix going into svn now. Note that I have reduced the chunklet size to be _writeChunk/6 because aUTF8 character may be 6 bytes.
Hide
Permalink
Christian d'Heureuse added a comment -

Thanks for fixing this bug.

> Note that I have reduced the chunklet size to be _writeChunk/6 because
> aUTF8 character may be 6 bytes.

I think that change on line 1246 (current cvs) was not necessary.
old version:
n+=_writeChunk/2;
new version:
n+=(_writeChunk+5)/6;
The buffer is filled in the next outer loop, independent of the chunklet size.
_converter.flush() does not flush the buffer _buf.
And the ByteArrayOutputStream _buf grows automatically if it is full.

The purpose of line 1246 is only to optimize speed, so that the faster _bytes.writeUnchecked() will be used after a chunklet has been processed, instead of the slower _converter.write().

Show
Christian d'Heureuse added a comment - Thanks for fixing this bug. > Note that I have reduced the chunklet size to be _writeChunk/6 because > aUTF8 character may be 6 bytes. I think that change on line 1246 (current cvs) was not necessary. old version: n+=_writeChunk/2; new version: n+=(_writeChunk+5)/6; The buffer is filled in the next outer loop, independent of the chunklet size. _converter.flush() does not flush the buffer _buf. And the ByteArrayOutputStream _buf grows automatically if it is full. The purpose of line 1246 is only to optimize speed, so that the faster _bytes.writeUnchecked() will be used after a chunklet has been processed, instead of the slower _converter.write().

People

  • Assignee:
    Greg Wilkins
    Reporter:
    Christian d'Heureuse
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: