Andrew, Okay, great! The changes should probably be divided into two patches: queue.c is a behavior change and doesn't address any bugs, just returns a 500 response if possible, and allows trace filters to run. My guess is that this patch is backwards compatible with all code because someone would have noticed by now that trace filters don't always run (assuming their code relied on them running). If they noticed that trace filters were not running, then they would have someone adapted their code to the known behavior, so their code should still work.
In addition, it is now dangerous to rely on code to initialize in a pre/postauth filter and then run in a trace filter because the request url may have changed, which would alter what trace filters run, unless they are registered to the global pattern /*. In that case, you can now use ns_ictl to run traces with/without a conn (you shouldn't need a conn in a trace except for logging about the conn). ns_atclose might work in some cases, it might still have info about the conn. The other patch is to adprequest.c and adpeval.c, which is a pure bug-fix. It only restores the expected behavior of ns_adp_return, ns_adp_break and ns_adp_abort, as well as returning the previously documented AOLserver request return codes NS_OK or NS_ERROR. Unless there are additional questions about the bugfix, I'll checkout the HEAD version and commit a patch. The patch to queue.c needs at least a little community feedback, and should also include a changelog and maybe some documentation changes. tom jackson On Thu, 2009-04-09 at 20:10 -0500, Andrew Steets wrote: > Hi Tom, sorry to go dark for so long. It was operator error. I was > in a hurry and I don't think I restarted the server after I installed > the patched version of the server. > > I checked again just now and everything works as expected. > > -Andrew > > On Mon, Apr 6, 2009 at 4:14 PM, Tom Jackson <[email protected]> wrote: > > Andrew, > > > > Hmmm, well without knowing how you tested this, I can't help much. I > > created a few test adp pages. I tested them before my changes to > > identify the problems. > > > > Here is an example set of pages: > > include.adp: > > <% > > ns_adp_puts "before include" > > ns_adp_include test-ns-return.adp > > ns_adp_puts "after include" > > ns_log Notice "finished include.adp" > > %> > > test-ns-return.adp: > > <% > > ns_return 200 text/plain hi > > ns_adp_abort > > ns_log Notice "test-ns-return.adp after ns_adp_abort" > > %> > > > > The error.log should contain neither of the Notice logs. > > The access.log should have a 200 response of content length 2. > > > > Even this produces an access.log entry: > > > > <% > > ns_adp_puts hi > > ns_adp_abort > > %> > > > > A zero length 200 response: > > > > 127.0.0.1 - - [06/Apr/2009:13:54:52 -0700] "GET /just-abort.adp > > HTTP/1.1" 200 0 "" "" > > > > Did you patch the other two files? (Note that my queue.c file is not > > identical to yours, so the patch needs to be applied by hand I think.) > > > > queue.c handles changes to allow logging during error conditions > > > > adprequest.c changes allows distinguishing between actual errors and adp > > signaling and translates Tcl return codes into AOLserver request return > > codes. Changes also ensure that the adp buffer is cleaned up in all > > cases. > > > > adpeval.c changes just ensure that the tcl error code is set to > > correspond to the ADP exception code. The code probably needs a comment > > because actual errors in ADP processing is signaled when the Tcl return > > code is TCL_OK and the adp exception code is ADP_OK. Why? Because on a > > tcl error, the ADP code doesn't get to change adp.exception to something > > else. > > > > The bugs in the current code were due to the awkward but necessary > > maintenance of these two return codes. The ADP code has gone through a > > lot of significant changes, so it is easy to see how these details > > didn't make it through correctly. But the simplicity of fixing them > > indicates that the code is in pretty good shape. > > > > Anyway, that is the bugs. The code in queue.c is not bug related, but > > allows the client to receive a 500 response on error and to allow > > logging during error conditions. > > > > Here is a link to my patch to my code: > > > > http://www.junom.com/gitweb/gitweb.perl?p=aolserver.git;a=commit;h=ca26f1a > > > > tom jackson > > > > > > > > > -- > AOLserver - http://www.aolserver.com/ > > To Remove yourself from this list, simply send an email to > <[email protected]> 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 <[email protected]> with the body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: field of your email blank.
