On Thu, 2006-09-28 at 05:18 -0400, Tsantilas Christos wrote: > 3) File ICAP/ICAPModXact.cc function > ICAPModXact::handleCommWroteHeaders() > > Sometimes, we have read the headers but not any part of the body yet > so in this case we must not call writeMore() function. > > - writeMore(); > + if(virgin->data->body && virgin->data->body->contentSize()) > + writeMore();
As far as I can see, virgin->data->body is never NULL at this stage and the rest of the code assumes it is not NULL. Thus, I assume the first part of the above if-statement can be safely removed. This leaves us with the zero-size content part. I believe zero size content is valid. If writeMore() cannot handle it, writeMore() should be fixed and still called unconditionally in the above code. One place where zero-sized content is useful is for zero-size previews, which are common (please see below on that). > 7) File ICAP/ICAPModXact.cc function ICAPModXact::estimateVirginBody > > if function header->expectingBody(method, size) returns size==0 then > we do not expect a body. Can you explain why zero-size body should be treated differently here? I am especially worried about the case were Squid may tell the ICAP server that there was no HTTP entity body where in fact there was a body of zero length. The difference is likely to affect the adaptation logic on the ICAP server if the server adapts (or preserves) HTTP bodies. Again, if zero-size bodies cause problems elsewhere in the ICAP code, we should fix those problems, but hopefully without completely ignoring such bodies. > 8) File ICAP/ICAPModXact.cc function ICAPPreview::enable > > The theAd is the expected preview size so if theAd==0 do not > enable preview mode. > > - theState = stWriting; > + if(theAd >0) > + theState = stWriting; > + else > + theState = stDisabled; Zero-size Preview is very useful because it allows an ICAP server to receive the HTTP headers without the body before making any decisions. Why do you want to disable it? Do we have zero-preview handling bugs elsewhere? I believe I have applied all other changes you wrote about. The changes currently live on the squid3-icap branch in the SourceForge Squid repository. There are other ICAP bugs that must be fixed before ICAP code can be declared stable. I hope to nail most in the next week or so. I would appreciate if you can answer the above questions and/or provide your test cases so that we can "close" the three pending changes one way or the other. Thank you, Alex.
