On Mon, 2009-04-13 at 13:49 -0400, Jim Davidson wrote:
> Hi,
> 
> A bit old but let me try to be helpful here...
> 
> 
> On Apr 3, 2009, at 11:45 PM, Tom Jackson wrote:

....

Jim,

Thanks for adding some back story. Over the weekend I spent more time
looking into this problem and basically rewrote  one portion of ConnRun
and pulled out the auth part of the code into another static function. 

As Gustaf noticed, there was a little confusion in the code, I think
mostly related to the use and meaning of the "status" variable. This
variable has several meanings at different points in ConnRun and it gets
confusing. Plus I wasn't completely sure what was supposed to happen at
each point. 

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);
}


tom jackson


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

Reply via email to