Re: squid-3 icap related patch

2006-10-03 Thread Tsantilas Christos

Hi Alex,
  
Alex Rousskov wrote:



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.
 


Yes that the  virgin-data-body is never NULL. I will check it more...


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


I believe that the problem here is not the zero sized content.
Squid calls this function when read all headers.
There are cases in which  squids did not read any part of body yet
(and there is not Content-Lenght header so preview is disabled).
In this case squid sends to the icap server the 0\r\n\r\n string
(0\r\n definition + \r\n after chunk)
The icap server believes that this is the end of body and does not
expect  more data.


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?

 


You are right about zero-preview and zero size content.
Give me some time to check again if there is any zero-preview
handling bug.


I believe I have applied all other changes you wrote about. The changes
currently live on the squid3-icap branch in the SourceForge Squid
...
 


OK, I will use the squid3-icap branch for my tests.

Regards,
 Christos




Re: proposal to remove port 563 from default ACLs

2006-10-03 Thread Henrik Nordstrom
tis 2006-10-03 klockan 00:35 +0200 skrev Henrik Nordstrom:

 I have actually used it in in real life. Was a some vendor support forum
 NNTP server requiring authentication and encryption to protect the
 passwords. But I have no problem with removing the port from the
 suggested default configuration as it's not at all common and easy to
 add back if needed. Most have switched to using web forums anyway.. This
 vendor actually provided both nntps and https access methods to the same
 forums. Unfortunately I don't remember the vendor, but it was one of the
 large commercial software vendors with active user communities.

Looking around I saw that gmane also has nttps support in addition to
their nntp and http interfaces..

Regards
Henrik


signature.asc
Description: Detta är en digitalt signerad	meddelandedel


Re: squid-3 icap related patch

2006-10-03 Thread Alex Rousskov
On Tue, 2006-10-03 at 11:36 +0300, Tsantilas Christos wrote:
   
 Alex Rousskov wrote:
 I believe zero size content is valid. If writeMore() cannot handle it,
 writeMore() should be fixed and still called unconditionally in the
 above code.

 I believe that the problem here is not the zero sized content.
 Squid calls this function when read all headers.
 There are cases in which  squids did not read any part of body yet
 (and there is not Content-Lenght header so preview is disabled).
 In this case squid sends to the icap server the 0\r\n\r\n string
 (0\r\n definition + \r\n after chunk)
 The icap server believes that this is the end of body and does not
 expect  more data.

I see! Writing last-chunk when the body is [still] expected is obviously
wrong and should be fixed. I will try to reproduce and fix this.

In general, I am worried about adding a guard for writeMore() outside
of writeMore() because writeMore() is called from several places, on
several events.

Thank you,

Alex.




Re: squid-3 icap related patch

2006-10-03 Thread Alex Rousskov
On Tue, 2006-10-03 at 08:48 -0600, Alex Rousskov wrote:
 On Tue, 2006-10-03 at 11:36 +0300, Tsantilas Christos wrote:

  I believe that the problem here is not the zero sized content.
  Squid calls this function when read all headers.
  There are cases in which  squids did not read any part of body yet
  (and there is not Content-Lenght header so preview is disabled).
  In this case squid sends to the icap server the 0\r\n\r\n string
  (0\r\n definition + \r\n after chunk)
  The icap server believes that this is the end of body and does not
  expect  more data.
 
 I see! Writing last-chunk when the body is [still] expected is obviously
 wrong and should be fixed. I will try to reproduce and fix this.

I cannot reproduce the above. I made a server that responds to an HTTP
GET with HTTP headers and then waits before sending the message body.
There is no Content-Length header. After Squid received the HTTP
response headers and successfully wrote the ICAP message prefix to the
ICAP server, I get:

| ICAPModXact::noteCommWrote called [Comm(13wr)w(2)/]
| ICAP/ICAPModXact.cc(164) Wrote 670 bytes
| ICAP/ICAPModXact.cc(258) will write up to 0 bytes of prime virgin body
| ICAPModXact has no writeable prime virgin body content
| ICAP/ICAPModXact.cc(287) will write 0 raw bytes of prime virgin body
| ICAPModXact::noteCommWrote ended [Comm(13r)w(5)/]

As you can see, ICAPModXact refuses to write anything when there is no
response body yet. Was your test case different?

Thank you,

Alex.




Re: squid-3 icap related patch

2006-10-03 Thread Adrian Chadd
On Wed, Oct 04, 2006, Tsantilas Christos wrote:
 Hi,
  I can not reproduce the problem any more using the
 squid3-icap sources.
 I am testing for some hours the icap client and looks OK.
 I believe that you can ignore this patch.

Thanks a lot for your ICAP feedback, and thanks to Alex for importing
all the fixes into Squid-3.

Some of us were worried we'd have to learn ICAP to make it all
work before the release :)




Adrian