Sorry about the ugly .diff file.  Forgot to add the -u when I did the
diff.  I already caught the trashed stack variable and made the fix so
everything looks much better in the log (real info instead of trash).  I
also believe that Apache is smart enough not to make the call on a
keep-alive.  In my testing I have seen a single request to the identd
daemon but several entries for the page and .gifs in the access_log with
the correct information.  Here is a good diff with the ap_pstrdup()
added.  If it looks good to you, I will go ahead and check it in.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> [EMAIL PROTECTED] Wednesday, September 17, 2003 12:33:30 PM >>>
At 01:09 PM 9/17/2003, Brad Nicholes wrote:
>Ah yeah, I noticed the problem with the JMP_BUF but for some reason I
>missed the local statics.  I am assuming that these local variables
are
>static simply to accomodate the setjmp() call.  If I get rid of
setjmp()
>and simply set the recv/send timeouts on the socket itself, there
>shouldn't be any reason to have the locals declared as static.
>
>See the attached .diff file.  Seems to work fine on NetWare.

First - ewwww.... not diff -u and no file reference in the .diff :-?

One more critical observation, if no longer static - this becomes
evil:

> #if (defined(NETWARE) || defined(WIN32))
>     if (setsocktimeout(sock, ap_rfc1413_timeout) == 0) {
>         if (get_rfc1413(sock, &conn->local_addr, &conn->remote_addr,
user, srv) >= 0)
>             result = user;

result points to stack - promptly trashed.  We will need to allocate
char* user from a pool, although I suggest we keep the fixed length
buffer
and simply pstrdup into conn->pool after invoking rfc1413.

Do we in 1.3 already protect against invoking this multiple times for
a keep-alive connection?  We should if we don't - it's expensive.

Bill


Attachment: rfc1413.c.diff
Description: Binary data

Reply via email to