I think I finally figured it out, and it now appears to me that
including the access logging trace where it is makes perfect sense.
What
didn't make sense, and was a bug, was the meaning of the value of
"status" at the point trace filters, server traces and void traces
ran.
In effect, "status" must indicate the success/failure of sending a
response to the client. The current code didn't do that. Here is my
rewrite of the two affected functions, with comments added:
/*
*----------------------------------------------------------------------
*
* ConnAuthorize --
*
* Try to authorize a connection.
*
* Results:
* NS_OK on successful authorization,
* NS_FILTER_RETURN on authorization failure, or
* NS_ERROR on error.
*
* Side effects:
* Connection request is authorized. On failure an alternative
* response may be sent to the client.
*
*----------------------------------------------------------------------
*/
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:
/*
* NS_OK indicates that a response was sent to the client
* a this point, we must fast-forward to traces,
* so we convert the NS_OK to NS_FILTER_RETURN.
*/
if ((status = Ns_ConnReturnForbidden(conn)) == NS_OK) {
status = NS_FILTER_RETURN;
}
break;
case NS_UNAUTHORIZED:
/*
* NS_OK indicates that a response was sent to the client
* a this point, we must fast-forward to traces,
* so we convert the NS_OK to NS_FILTER_RETURN.
*/
if ((status = Ns_ConnReturnUnauthorized(conn)) == NS_OK) {
status = NS_FILTER_RETURN;
}
break;
case NS_ERROR:
default:
status = NS_ERROR;
break;
}
return status;
}
/*
*----------------------------------------------------------------------
*
* ConnRun --
*
* Run a valid connection.
*
* Results:
* None.
*
* Side effects:
* Connection request is read and parsed and the cooresponding
* service routine is called.
*
*----------------------------------------------------------------------
*/
static void
ConnRun(Conn *connPtr)
{
Ns_Conn *conn = (Ns_Conn *) connPtr;
NsServer *servPtr = connPtr->servPtr;
int i, status;
Tcl_Encoding encoding = NULL;
/*
* Initialize the connection encodings and headers. */
connPtr->outputheaders = Ns_SetCreate(NULL);
encoding = NsGetInputEncoding(connPtr);
if (encoding == NULL) {
encoding = NsGetOutputEncoding(connPtr);
if (encoding == NULL) {
encoding = connPtr->servPtr->urlEncoding;
}
}
Ns_ConnSetUrlEncoding(conn, encoding);
if (servPtr->opts.hdrcase != Preserve) {
for (i = 0; i < Ns_SetSize(connPtr->headers); ++i) {
if (servPtr->opts.hdrcase == ToLower) {
Ns_StrToLower(Ns_SetKey(connPtr->headers, i));
} else {
Ns_StrToUpper(Ns_SetKey(connPtr->headers, i));
}
}
}
/*
* Run the request.
*/
while (1) {
/*
* Proxy requests replace request logic
*/
if (connPtr->request->protocol != NULL
&& connPtr->request->host != NULL) {
status = NsConnRunProxyRequest((Ns_Conn *) connPtr);
break;
}
/*
* Each stage of request processing can return one of three
* possible results:
* NS_OK means continue to next stage
* NS_FILTER_RETURN means skip to traces
* NS_ERROR means skip to 500 response attempt
*/
status = NsRunFilters(conn, NS_FILTER_PRE_AUTH);
if (status != NS_OK) {
break;
}
status = ConnAuthorize(connPtr);
if (status != NS_OK) {
break;
}
status = NsRunFilters(conn, NS_FILTER_POST_AUTH);
if (status != NS_OK) {
break;
}
status = Ns_ConnRunRequest(conn);
break;
}
if (status == NS_ERROR) {
status = Ns_ConnReturnInternalError(conn);
}
/*
* Closing connection also runs code registered with
* ns_atclose.
*/
Ns_ConnClose(conn);
/*
* Trace filters, void filter traces and server traces
* run a response was sent to the client
* Note that a successful response includes sending
* an internal server error response.
*/
if (status == NS_OK || status == NS_FILTER_RETURN) {
/*
* Filter Traces are registered with ns_register_filter
* Ignore the return code? We need to do NsRunTraces based
* upon status before trace filters ran. Question is if we
* should skip void trace filters. We could wrap them in
another
* status check, then move on to NsRunTraces.
*/
NsRunFilters(conn, NS_FILTER_TRACE);
/*
* Filters registered outside the context of a particular
server
*/
(void) NsRunFilters(conn, NS_FILTER_VOID_TRACE);
/*
* Server traces registered with Ns_RegisterServerTrace
* Access logging is handled at this point.
*/
NsRunTraces(conn);
}
/*
* Cleanup the connections, calling any registered cleanup traces
* followed by free the connection interp if it was used.
*/
/*
* Cleanups are registered with either:
* Ns_RegisterConnCleanup: same signature as Ns_RegisterServerTrace
* Ns_RegisterCleanup: runs for all servers
*/
NsRunCleanups(conn);
/* Finished with conn */
NsFreeConnInterp(connPtr);
if (connPtr->type != NULL) {
Ns_ConnSetType(conn, NULL);
}
if (connPtr->query != NULL) {
Ns_ConnClearQuery(conn);
}
if (connPtr->outputheaders != NULL) {
Ns_SetFree(connPtr->outputheaders);
connPtr->outputheaders = NULL;
}
Ns_DStringTrunc(&connPtr->obuf, 0);
}
