I'm wondering if there needs to be some more specific exception flags
stored in the connection structure to handle all these cases and some
C and Tcl api's to access/modify the same. It seems you're doing the
good work of rationalizing all the error conditions that had been
pretty confused together and shuffling / updating the code to handle
the various cases but more explicit tracking of the error conditions
(execution, pre/post filter, connection, auth, etc.) could make things
easier. This may lead the code to look like more try/catch type
exception handling instead of a non-obvious code path understandable
only with the surrounding comments.
-Jim
On Apr 15, 2009, at 11:12 AM, Tom Jackson wrote:
Gustaf,
Thanks for testing on a production server. I haven't tested this with
the authorization module, but it seems like it should work okay.
The internal server loop bug that I tracked down yesterday led me to
another function which contains code very similar to the new
ConnAuthorize function, so I'm wondering if I should see if I should
maybe export ConnAuthorize to NsConnAuthorize. The overall idea is to
only run Ns_ConnReturnInternalError at the end of the request and not
from inside the authorization or redirection functions (because the
returns Ns_ConnReturnForbidden and Ns_ConnReturnUnauthorized could
also
return NS_ERROR.
Any ideas are welcome. (I think just removing
Ns_ConnReturnInternalError
from Ns_ConnRedirect will break the loop.)
tom jackson
Here's the two chunks of code:
static int
ConnAuthorize(Conn *connPtr)
{
Ns_Conn *conn = (Ns_Conn *) connPtr;
NsServer *servPtr = connPtr->servPtr;
int status;
status = Ns_AuthorizeRequest(servPtr->server,
connPtr->request->method, connPtr->request->url,
connPtr->authUser, connPtr->authPasswd, connPtr->peer);
switch (status) {
case NS_OK:
break;
case NS_FORBIDDEN:
if ((status = Ns_ConnReturnForbidden(conn)) == NS_OK) {
status = NS_FILTER_RETURN;
}
break;
case NS_UNAUTHORIZED:
if ((status = Ns_ConnReturnUnauthorized(conn)) == NS_OK) {
status = NS_FILTER_RETURN;
}
break;
case NS_ERROR:
default:
status = NS_ERROR;
break;
}
return status;
}
and from op.c:
int
Ns_ConnRedirect(Ns_Conn *conn, char *url)
{
Conn *connPtr = (Conn *) conn;
int status;
++connPtr->recursionCount;
/*
* Update the request URL.
*/
Ns_SetRequestUrl(conn->request, url);
/*
* Re-authorize and run the request.
*/
status = Ns_AuthorizeRequest(Ns_ConnServer(conn), conn->request-
>method,
conn->request->url, conn->authUser,
conn->authPasswd, Ns_ConnPeer(conn));
switch (status) {
case NS_OK:
status = Ns_ConnRunRequest(conn);
break;
case NS_FORBIDDEN:
status = Ns_ConnReturnForbidden(conn);
break;
case NS_UNAUTHORIZED:
status = Ns_ConnReturnUnauthorized(conn);
break;
case NS_ERROR:
default:
status = Ns_ConnReturnInternalError(conn);
break;
}
return status;
}
On Wed, 2009-04-15 at 12:52 +0200, Gustaf Neumann wrote:
your rewrite of the two functions below look fine to me. The structure
is much clearer now, results of the authorization handling are
handled
now consistently. I am useing these functions since a few days on one
of my servers and found nothing unusual. Many thanks to Tom!
Andrew, did you test this version as well for your test-cases?
If thise works for you as well (i would assume so), i would think
that version should go into CVS.
--
AOLserver - http://www.aolserver.com/
To Remove yourself from this list, simply send an email to <lists...@listserv.aol.com
> with the
body of "SIGNOFF AOLSERVER" in the email message. You can leave the
Subject: field of your email blank.
--
AOLserver - http://www.aolserver.com/
To Remove yourself from this list, simply send an email to
<lists...@listserv.aol.com> with the
body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject:
field of your email blank.
