Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 370285

Summary:

Socket back pressure at the start of a request can lead to missing chunk header

Product:

[RT] Jetty

Reporter:

Jeff Bergan <bergan>

Component:

client

Assignee:

Greg Wilkins <gregw>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

jetty-inbox

Version:

unspecified

Flags:

gregw: iplog+

Target Milestone:

7.5.x

Hardware:

OS:

Linux

Whiteboard:

Attachments:

Description	Flags
Proposed patch with test case.	none

Description Jeff Bergan

2012-01-31 17:56:52 EST

Build Identifier: 8.0.4.v20111024

Similar to Bug 363757 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=363757), but a different cause, and still the case in trunk, if there is back pressure on the socket writing the request, it seems that the HttpGenerator can end up missing a chunk header (which results in a 'bad chunk char' on the server).

I'm attaching a patch which includes a test case that reproduces the issue, along with a possible (though a tad clunky) patch.  The problem occurs when the client calls addContent twice, but is unable to write out the first full buffer.

If the first buffer has a length of greater than 1024 characters, then it enters "_bypass", so _buffer stays null.  When the second addContent is called, it will attempt to flushBuffer() to clear up the _content buffer.  The initial chunk header is written to _header, and then _header and some amount of _content are written out.

If flushBuffer() does not manage to flush all of _content, then it concatenates _content and the new content buffer together into _content, without adding a chunk header for the new content that it is adding.


Reproducible: Always

Steps to Reproduce:
Run testChunkedWithBackPressure() from the attached patch.

Comment 1 Jeff Bergan

2012-01-31 17:57:41 EST

Created attachment 210341 [details]
Proposed patch with test case.

Comment 2 Greg Wilkins

2012-03-01 19:20:37 EST

Sorry for the slow response looking at this one. Analysing now.

Comment 3 Greg Wilkins

2012-03-02 00:51:43 EST

I've applied and committed your patch more or less as it is.  I agree it is a tad clunky... but mostly because the whole HttpGenerator is a tad clunky.  I've not spent any time cleaning it up, as this is going through a total refactor currently in the jetty-9 branch, to avoid exactly that kind of situation.

thanks for the contribution.

Comment 4 Greg Wilkins

2012-03-02 00:52:40 EST

74da51a8e61f227ccf13b0850d0791594e90a711