Alex Rousskov wrote:
Dechunk incoming requests as needed and pipeline them to the server side.
The server side will eventually either chunk the request or fail. That
code is not ready yet and is not a part of this patch. This patch iis
not enough because the dechunked requests end up being sent without
chunking and without Content-Length. However, these client-side changes
are ready and seem to be working. It may be easier to review them now,
without the server-side code.
Details are below.
Removed clientIsRequestBodyValid() as unused. It was called with a
content-length>0 precondition that made the function always return true.
Removed old dechunking hack that was trying to buffering the entire
request body, pretending that we are still reading the headers. Adjusted
related code. More work may be needed to identify client-side code that
assumes the request size is always known.
Removed ConnStateData::bodySizeLeft() because we do not always know how
much body is left to read -- chunked requests do not have known sizes
until we read the last-chunk. Moreover, it was possibly used wrong
because sometimes we want to know whether we want to comm_read more body
bytes and sometimes we want to know whether we want to "produce" more
body bytes (i.e., copy already read bytes into the BodyPipe buffer,
which can get full).
Added ConnStateData::mayNeedToReadMoreBody() to replace
conn->bodySizeLeft() with something more usable and precise.
Removed my wrong XXX related to closing after initiateClose.
Removed my(?) XXX related to endless chunked requests. There is nothing
special about them, I guess, as a non-chunked request can be virtually
endless as well if it has a huge Content-Length value.
Use commIsHalfClosed() instead of fd_table[fd].flags.socket_eof for
consistency with other client-side code and to improve readability. I
think these should return the same value in our context but I am not sure.
Correctly handle identity encoding. TODO: double check that it is still
in the HTTP standard.
Fixed HttpStateData::doneSendingRequestBody to call its parent. I am not
sure it helps with correctly processing transactions, but the parent
method was designed to be called, and calling it make the transaction
state more clear.
Thank you,
Alex.
If your comment is correct and connKeepReadingIncompleteRequest() only
refer to the headers. Could the function name be updated too say that?
In connCancelIncompleteRequests the debugs look useless for level 1.
There is no info about which requests they refer. If you wish to retain
them at that level please use DBG_IMPORTANT, reference something to
track the request and add a WARNING or NOTICE etc tag to highlight why
they are at that level.
Looks good so far.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.7
Beta testers wanted for 3.2.0.1
