Hi,
Yes -- I think in general, the meaning of "status" with regards to a
connection processing (or a filter that may preempt it) was that "a
valid HTTP response was sent..." If you look deep in the rat's nest
of connio.c, you'll find:
int
Ns_WriteConn(Ns_Conn *conn, char *buf, int len)
{
if (Ns_ConnWrite(conn, buf, len) != len) {
return NS_ERROR;
}
return NS_OK;
}
Ns_ConnWrite eventually calls the "send" routine from the driver (sock
or ssl). If the driver can't send the # of bytes intended, it returns
NS_ERROR. This turns any network send error into a simple "couldn't
send" error. That code was the original place where NS_ERROR could be
generated and made sense when it was sending content from a file.
There are some complications -- originally the headers were sent
before the content, i.e., there was no buffering. That was a
performance irritant so a level of buffering was put in place and so
results which previously may have meant "headers not sent due to
dropped connection" now silently buffer result and return NS_OK
always. Then with ADP there grew some encoding issues and/or gzip'ing
goals that also led to more of "get a big whole buffer ready and blast
that in one attempt, using iovec's if possible..." You can see that
in the Ns_ConnFlush and Ns_ConnFlushDirect complexity.
So, the simple "NS_ERROR means conn failed..." isn't so valid
anymore. In fact, on a long running connection the code could
cheerfully be creating a buffer to send over a socket that may have
already dropped. That knowledge -- async socket drop -- is actually
something the I/O event thread (sometimes confusing called the driver
thread for the DriverThread routine in driver.c) could easily detect.
Event-driven socket code is often more efficient than thread-based I/
O. For AOLserver, the read-side was made event-driven years ago
(helps with denial of service attacks and general efficient) but the
write side remained thread-based mostly because of the expectations
(smart or dumb) of traces and order of execution, i.e., you have a
thread that gets the read-ahead buffer, does the work, sends
(synchronously) the response, and the proceeds to the traces, cleanup,
etc. Note to add some confusion, the I/O thread currently gracefully
closes connections already -- it's possible that generates an error
condition which is never accurately logged (note the comment "NB:
Timeout Sock on end-of-file or error." -- result becomes "pretend it
actually worked and just close socket...).
If I had to do this all over again, I'd have all the I/O be event
driven. It would read the request as it does now, allow for some
async-work if needed (possible now but not used as far as I know),
dispatch the complete request context to an execution thread to do
script-stuff and provide result back to the I/O thread where event-
driven code would send result and generate a smart log entry,
including all cases of request bad, code failed, send failed, or all
ok. If while the thread was executing the I/O thread detected a
dropped socket, it could send an async-exception message to the
execution thread which may or may not be noticed or handled but an
attempt could be made.
While this may be nifty, today it would mean traces and filters would
run before the content was already sent. For a connection cleanup
(i.e., dump this temp resource), it would make sense but for an old-
school common log format it may be odd (or at least suspect, but
perhaps no less suspect than what's going on now).
-Jim
On Apr 13, 2009, at 2:48 PM, Tom Jackson wrote:
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.
--
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.