Re: Squid3 BodyReader changes
Hi there, I have committed the BodyPipe changes described below (with a few modifications) to the squid3-icap branch. Using the new BodyPipe class, I was able to eliminate many ICAP-related classes and code complexities, not to mention a few memory leaks and bugs. I will be working on a few remaning XXXs next week, but please test the current code if you can, especially if you are using ICAP. **WARNING** The committed changes are significant and probably introduce new bugs! The code passed some Web Polygraph and manual surfing tests, with and without ICAP. FTP-specific changes are untested, but the old code probably did not work well with ICAP either. Please test and submit bug reports. Thank you, Alex. On Tue, 2007-01-30 at 14:33 -0700, Alex Rousskov wrote: > Folks, > > Executive summary: I am trying to fix several bugs and complaints > related to your favorite class, the BodyReader. The changes I would like > to make are significant, so I decided to post them here first. Please > see the attached BodyPipe.h sketch. I will proceed with these changes > unless there are violent objections or better ideas. The email below > documents my concerns and explains the rationale behind BodyPipe. > > > Current state: The BodyReader class does not seem to read a body. It > helps to move the body content from the body producer to the consumer. > The class is essentially a collection of loosely coupled function > pointers with protected and bare data attached to them. The class calls > consumer-side functions on behalf of the producer (or vice versa), while > updating the body size state. BodyReader refcounting was meant to > communicate abort conditions. > > Here are a few related/intermingled reasons behind BodyPipe, the > replacement design I am implementing (see the attached sketch). > > A) The reason I want to get away from the current "lose ends" approach > with function pointers is to provide a more clear, easier to follow > implementation (IMO). I want classes like ConnStateData or > ICAPClientReqmodPrecache to inherit from BodyProducer and just implement > the two or three required methods. No wrappers or bare data pointers at > the high level, where all the changes and enhancements are > taking place. > > B) The reason I want explicit provider and consumer pointers inside > BodyPipe is to be able to abort either side if the other side is having > trouble. The current idea of using refcounting to abort something does > not and cannot work (if there are two refcounted pointers, the second > pointer will not know that the first has been set to NULL). I also want > joint ownership of the BodyReader so that the producer and the consumer > know that they must clear/detach themselves before forgetting about the > pipe. Refcounting will just help to destroy the abandoned pipe. > > C1) The reason I want asynchronous calls is to separate producer from > consumer and break the call-me-back-while-I-am-calling-you loops. I > believe that at least the abortedSize assertion (bug #1862) is > caused, in part, by such loops. > > C2) Another reason for separating producer and consumer is to allow the > producer to disconnect when done, leaving a functional object (with the > remaining/last body bytes) behind. This situation happens when, for > example, an ICAP transaction is finished but the HttpStateData is still > writing the body to the origin server. > > Again, please take a look at the attached sketch. > > > I have seen this before: While sketching the design, I realized that > what I want is very similar to MsgPipe, a class used by ICAP-related > code to shovel HTTP messages between Squid core and an ICAP transaction. > If you look at MsgPipe.cc and ignore the low-level debugging macros, you > will see that there is virtually nothing there. The class simply > converts notifications from one pipe end to events and then calls the > other end when the event fires. > > I do not plan on reusing MsgPipe itself because its simple design does > not allow one object to be the end of two pipes -- there would be no way > to know which pipe the messages are coming from. I do not want to > complicate MsgPipe and its users to support multi-pipe ends. I think it > would be better to have a very similar but distinct BodyPipe class, with > method names specific to the problem at hand and extra code to handle > body size calculations. The design duplication will be there, but code > duplication should be negligible. > > I am worried that I see every problem as a nail though, which is > another motivation for this message. Again, if you have better ideas on > how to fix BodyReader-related problems, please stop me and contribute! > > Thank you, > > Alex. >
Re: squid cache information - help
On Tue, 2007-02-13 at 14:54 -0800, ccmail111 wrote:
> Thank you very much for your valuable input.
>
> Question:
> To enable SQUID3.x proxy save the page in file,
> I modified code as follows, but unable get the page.
>
> I did:
>
> File: store_client.cc
> In function:
> static void
> storeClientReadBody(void *data, const char *buf,
> ssize_t len, StoreIOState::Pointer self)
> {
>
>
>
> debug(90, 3)("storeClientReadBody: len %d", (int)
> len);
>
>
> > new code start here..
>
> {
> int rc = 0;
> FILE *ffd = 0;
> char fbuf[256];
>
> memset(fbuf, 0, sizeof(fbuf));
> strcpy(fbuf, "/tmp/");
> strcat(fbuf, sc->entry->getMD5Text());
> ffd = fopen(fbuf, "a+");
> if (!ffd) {
> fprintf(stderr,"TEST: file name: %s, ffd: %p,
> rc: (%s), name: %s, len: %d\n",
> fbuf, ffd, strerror(errno),
> sc->entry->getMD5Text(), len);
> }
> rc = fwrite(buf, 1, len, ffd );
> if (rc < 0) {
> fprintf(stderr,"TEST: write rc: %d : %s: ffd:
> %p \n", rc, strerror(errno), ffd);
> }
> fclose(ffd);
> }
> > new code end here
>
> }
You seem to be writing "buf", which is not used in storeClientReadBody()
and so probably does not contain the content you are after. I am not
sure any buffer in storeClientReadBody contains object body, but I am
not a store API expert.
Also, I do not think this kind of hack will work in general unless you
somehow protect the file from being written to by different transactions
that deal with the same store entry.
Sorry that I cannot be more helpful at this time,
Alex.
> > Here is an example of a thread offering up to four
> > starting points for
> > your question:
> >
> http://thread.gmane.org/gmane.comp.web.squid.devel/4048/
> >
> > This recent thread may also be relevant:
> >
> http://thread.gmane.org/gmane.comp.web.squid.devel/4197/
Re: squid cache information - help
Hi Alex,
Thank you very much for your valuable input.
Question:
To enable SQUID3.x proxy save the page in file,
I modified code as follows, but unable get the page.
I did:
File: store_client.cc
In function:
static void
storeClientReadBody(void *data, const char *buf,
ssize_t len, StoreIOState::Pointer self)
{
debug(90, 3)("storeClientReadBody: len %d", (int)
len);
> new code start here..
{
int rc = 0;
FILE *ffd = 0;
char fbuf[256];
memset(fbuf, 0, sizeof(fbuf));
strcpy(fbuf, "/tmp/");
strcat(fbuf, sc->entry->getMD5Text());
ffd = fopen(fbuf, "a+");
if (!ffd) {
fprintf(stderr,"TEST: file name: %s, ffd: %p,
rc: (%s), name: %s, len: %d\n",
fbuf, ffd, strerror(errno),
sc->entry->getMD5Text(), len);
}
rc = fwrite(buf, 1, len, ffd );
if (rc < 0) {
fprintf(stderr,"TEST: write rc: %d : %s: ffd:
%p \n", rc, strerror(errno), ffd);
}
fclose(ffd);
}
> new code end here
}
# ls -l /tmp/02179C10E9E2DD4469A79C8E6456895A
-rw-r--r-- 1 nobody nobody 1202 Feb 13 14:49
/tmp/02179C10E9E2DD4469A79C8E6456895A
# file /tmp/02179C10E9E2DD4469A79C8E6456895A
/tmp/02179C10E9E2DD4469A79C8E6456895A: data
# file /tmp/4A91DC3958CA0F5D657F5C262252D738
/tmp/4A91DC3958CA0F5D657F5C262252D738: data
# ls -l /tmp/4A91DC3958CA0F5D657F5C262252D738
-rw-r--r-- 1 nobody nobody 20706 Feb 13 14:49
/tmp/4A91DC3958CA0F5D657F5C262252D738
I see the files being created, but contents are
control characters (garbled).
Any suggestions ?
Thanks !
MC
--- Alex Rousskov <[EMAIL PROTECTED]>
wrote:
> On Thu, 2007-02-08 at 14:05 -0800, ccmail111 wrote:
>
> > I have: squid-3.0.PRE5-20061215 on Linux.
> > I am looking to understand how SQUID3.x stores
> > (in no cache mode) the responses received from
> Http
> > server - HTTP payload (including images etc.
> received
> > and retrieved from different sites).
> >
> > I need this information to tie it to my backend
> program
> > which does audit and such.
> > I have some APIs like stdlib (linux) and would
> like
> > to call them from appropriate locations from
> SQUID.
> >
> > Any help/pointers is appreciated !
>
> Similar questions have been discussed a few times
> recently. You may want
> to search the list archive for the word ICAP. While
> not all suggested
> solutions involve ICAP, the protocol is usually
> mentioned at least once
> in the relevant thread.
>
> Here is an example of a thread offering up to four
> starting points for
> your question:
>
http://thread.gmane.org/gmane.comp.web.squid.devel/4048/
>
> This recent thread may also be relevant:
>
http://thread.gmane.org/gmane.comp.web.squid.devel/4197/
>
> HTH,
>
> Alex.
>
>
>
Do you Yahoo!?
Everyone is raving about the all-new Yahoo! Mail beta.
http://new.mail.yahoo.com
Re: httpProcessReplyHeader question
On Tue, Feb 13, 2007, Henrik Nordstrom wrote: > tis 2007-02-13 klockan 19:45 +0800 skrev Adrian Chadd: > > > Why is there a storeAppend with no http parse? > > No idea, but a guess is to bump the entry out of the inmem_hi == 0 > condition.. What happens if you kill it? Hm. I wonder what the reply status does to the code later on.. I'll check it out and tidy it up. Adrian
Re: httpProcessReplyHeader question
tis 2007-02-13 klockan 19:45 +0800 skrev Adrian Chadd: > Why is there a storeAppend with no http parse? No idea, but a guess is to bump the entry out of the inmem_hi == 0 condition.. What happens if you kill it? Regards Henrik signature.asc Description: Detta är en digitalt signerad meddelandedel
Fwd: Send Ip address to authetication program
I want to modify squid for send ip addres to my basic authentication program (not only user an pass). I know I have to change it in auth_basic.c file. But how can I obtain the char * with IP adres for send it? I think it could be in auth_user_request_t struct of authenticateBasicStart function, but where? and how convert it to char*) Thanks in advance http://www.hhdirecto.net http://www.dechiste.com
Re: httpProcessReplyHeader question
On Tue, 2007-02-13 at 19:45 +0800, Adrian Chadd wrote:
> There's this in httpProcessReplyHeader:
>
> hdr_size = headersEnd(httpState->reply_hdr.buf, hdr_len);
> if (hdr_size)
> hdr_len = hdr_size;
> if (hdr_len > Config.maxReplyHeaderSize) {
> debug(11, 1) ("httpProcessReplyHeader: Too large reply header\n");
> /* [ahc] XXX not sure how to handle this and we should think about
> it; so fail out for now */
> storeAppend(entry, httpState->reply_hdr.buf,
> httpState->reply_hdr.size);
> memBufClean(&httpState->reply_hdr);
> reply->sline.status = HTTP_HEADER_TOO_LARGE;
> httpState->reply_hdr_state += 2;
> ctx_exit(ctx);
> return size;
> }
>
>
> Why is there a storeAppend with no http parse?
Could it be because the header is so large that it was considered
dangerous or even impossible to parse when the above code was written?
Alex.
Re: icap in squid3
On Tue, 2007-02-13 at 08:09 +0100, Axel Westerhold wrote:
> One comment on a nice feature I would like to have but still considering for
> security reasons:
>
> When an ICAP Server requieres auth for user mapping to rules/policies you
> sometimes run into a problem with sources with can't auth or destinations
> you do not want to require auth for. While you can use ACL's to get this
> done easily on squid sometimes the icap clients won't play ball. As a result
> some destinations are not using the icap virus scanner/ content system to
> make it work. So, maybe but just as a thought, it would be nice to use ACL's
> to automatically assign a username für those services so that they can be
> properly matched.
Will a default user name work? As a stop-gap measure, it is probably
much easier to implement than ACL-defined user name.
> >> A.) Do we need to split the username into parts and if so using which
> >> seperator. ('' = off or '\' or '+' etc.)
> >
> > Can the separator be up to the admin? Do we need to define it?
> >
>
> Must be configurable so empty string turns off and non-empty turns on and
> defines sperator. Samba for instance allows for Seperator modifications.
> Also, this gives squid some flexibility.
I was thinking that the user-configurable opaque text around individual
substitutions will define separators, if any. Is that too cumbersome?
> > Should we just support an arbitrary set of user-configurable header
> > names, with user-configurable values? If we add substitutions support,
> > then Squid should not really care about the meaning of the header. For
> > example,
> >
> > icap_client_add_header "X-Username" "$base64($UserName)"
> > icap_client_add_header "X-Prefix" "bar=$base64($Foo+$Bar)&foo=blah"
> > ...
>
> I like this from a technical point of view. But I can also see my customers
> which are most of the time used to windows gui and stuff and already
> complain about the squid conf, freaking out. Maybe, for common tasks we need
> some kind of macro producing the above code. At all, just as a thought, we
> might stay with a sfprint syntax first defining a format and then adding the
> values like
>
> icap_client_add_header "X-Username" "%s=%b",bar,$username
> Icap_cleint_add_header "X-Auth_Scheme"
> "LDAP://ldap.test.com:389/cn=%u,dc=%u,dc=test,dc=com",$username,$domain
>
> Where %s is a string without encoding while %b is base64 encoded and maybe
> $u is URI encoded etc. I know this asks for additional parsing but it might
> lessen the learning curve for many people.
I doubt GUI folks would freak out _more_ if we add shell-style
substitutions than if we add printf-style ones. GUI folks need a GUI
configuration tool that hides all this.
FWIW, I find shell-style more intuitive and less error-prone than
printf, but since I am not going to implement or pay for this, I would
not argue one way or the other. The developer should decide :-).
In either case, it would be great if the configuration code checked that
all substitutions are known/supported. In case of printf, it would be
nice to check that the %escapes match the arguments as well.
Thank you,
Alex.
httpProcessReplyHeader question
There's this in httpProcessReplyHeader:
hdr_size = headersEnd(httpState->reply_hdr.buf, hdr_len);
if (hdr_size)
hdr_len = hdr_size;
if (hdr_len > Config.maxReplyHeaderSize) {
debug(11, 1) ("httpProcessReplyHeader: Too large reply header\n");
/* [ahc] XXX not sure how to handle this and we should think about it;
so fail out for now */
storeAppend(entry, httpState->reply_hdr.buf, httpState->reply_hdr.size);
memBufClean(&httpState->reply_hdr);
reply->sline.status = HTTP_HEADER_TOO_LARGE;
httpState->reply_hdr_state += 2;
ctx_exit(ctx);
return size;
}
Why is there a storeAppend with no http parse? I'm about to pop out appending
header
data into the store from the http code and this is one of the (ab)users doing
things
this way.
(The other two spots in http.c which do a httpReplyParse() and storeAppend() now
do httpReplySwapOut(), which I'll change soon to implement the new behaviour.)
Adrian
