Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c util.c util_script.c
Brad Nicholes wrote: -1 as well. This is now causing compiler errors on NetWare. Please revert this patch! Can you provide an indication of exactly what broke so we will know what to avoid in future. Or was the breakage actually due to the the mod_cache problem reported last night? Thanks, Allan
Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c util.c util_script.c
mod_cache is a different issue. The compiler used to build the netware NLMs is very sensitive to type mismatches. @@ -3793,7 +3794,7 @@ core_net_rec *net = f-ctx; core_ctx_t *ctx = net-in_ctx; const char *str; -apr_size_t len; +apr_ssize_t len; Changing the type from apr_size_t to apr_ssize_t introduced a type mismatch in the call to apr_bucket_read() which expects an apr_size_t. Type casting it back to an apr_size_t to fix the problem seems like it would have defeated the whole purpose of doing it in the first place. Besides the fact that apr_bucket_read() can't give you back anything bigger than an apr_size_t anyway. Brad [EMAIL PROTECTED] Monday, October 25, 2004 10:01:53 AM Brad Nicholes wrote: -1 as well. This is now causing compiler errors on NetWare. Please revert this patch! Can you provide an indication of exactly what broke so we will know what to avoid in future. Or was the breakage actually due to the the mod_cache problem reported last night? Thanks, Allan
Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c util.c util_script.c
whoa! -1 Was this even discussed on the list? You just changed the entire module API and introduced a dozen potential security holes. Why on earth is it changing nvec to apr_size_t and then downcasting its use? Why is any of this even needed? Roy On Oct 22, 2004, at 8:22 AM, [EMAIL PROTECTED] wrote: ake 2004/10/22 08:22:05 Modified:.CHANGES include ap_mmn.h http_protocol.h httpd.h scoreboard.h util_script.h modules/http http_protocol.c server core.c protocol.c request.c scoreboard.c util.c util_script.c Log: WIN64: API changes to clean up Windows 64bit compile warnings Revision ChangesPath 1.1614+3 -0 httpd-2.0/CHANGES Index: CHANGES === RCS file: /home/cvs/httpd-2.0/CHANGES,v retrieving revision 1.1613 retrieving revision 1.1614 diff -u -r1.1613 -r1.1614 --- CHANGES 18 Oct 2004 00:49:30 - 1.1613 +++ CHANGES 22 Oct 2004 15:22:03 - 1.1614 @@ -2,6 +2,9 @@ [Remove entries to the current 2.0 section below, when backported] + *) WIN64: API changes to clean up Windows 64bit compile warnings + [Allan Edwards] + *) mod_cache: CacheDisable will only disable the URLs it was meant to disable, not all caching. PR 31128. [Edward Rudd eddie omegaware.com, Paul Querna] 1.70 +3 -2 httpd-2.0/include/ap_mmn.h Index: ap_mmn.h === RCS file: /home/cvs/httpd-2.0/include/ap_mmn.h,v retrieving revision 1.69 retrieving revision 1.70 diff -u -r1.69 -r1.70 --- ap_mmn.h 4 Jun 2004 22:40:46 - 1.69 +++ ap_mmn.h 22 Oct 2004 15:22:04 - 1.70 @@ -84,14 +84,15 @@ * changed ap_add_module, ap_add_loaded_module, * ap_setup_prelinked_modules, ap_process_resource_config * 20040425.1 (2.1.0-dev) Added ap_module_symbol_t and ap_prelinked_module_symbols + * 20041022 (2.1.0-dev) API changes to clean up 64bit compiles */ #define MODULE_MAGIC_COOKIE 0x41503230UL /* AP20 */ #ifndef MODULE_MAGIC_NUMBER_MAJOR -#define MODULE_MAGIC_NUMBER_MAJOR 20040425 +#define MODULE_MAGIC_NUMBER_MAJOR 20041022 #endif -#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a 1.93 +10 -10httpd-2.0/include/http_protocol.h Index: http_protocol.h === RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v retrieving revision 1.92 retrieving revision 1.93 diff -u -r1.92 -r1.93 --- http_protocol.h 18 Jul 2004 20:06:38 - 1.92 +++ http_protocol.h 22 Oct 2004 15:22:04 - 1.93 @@ -338,9 +338,9 @@ * @param str The string to output * @param r The current request * @return The number of bytes sent - * @deffunc int ap_rputs(const char *str, request_rec *r) + * @deffunc apr_ssize_t ap_rputs(const char *str, request_rec *r) */ -AP_DECLARE(int) ap_rputs(const char *str, request_rec *r); +AP_DECLARE(apr_ssize_t) ap_rputs(const char *str, request_rec *r); /** * Write a buffer for the current request @@ -357,9 +357,9 @@ * @param r The current request * @param ... The strings to write * @return The number of bytes sent - * @deffunc int ap_rvputs(request_rec *r, ...) + * @deffunc apr_ssize_t ap_rvputs(request_rec *r, ...) */ -AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r,...); +AP_DECLARE_NONSTD(apr_ssize_t) ap_rvputs(request_rec *r,...); /** * Output data to the client in a printf format @@ -367,9 +367,9 @@ * @param fmt The format string * @param vlist The arguments to use to fill out the format string * @return The number of bytes sent - * @deffunc int ap_vrprintf(request_rec *r, const char *fmt, va_list vlist) + * @deffunc apr_ssize_t ap_vrprintf(request_rec *r, const char *fmt, va_list vlist) */ -AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list vlist); +AP_DECLARE(apr_ssize_t) ap_vrprintf(request_rec *r, const char *fmt, va_list vlist); /** * Output data to the client in a printf format @@ -377,9 +377,9 @@ * @param fmt The format string * @param ... The arguments to use to fill out the format string * @return The number of bytes sent - * @deffunc int ap_rprintf(request_rec *r, const char *fmt, ...) + * @deffunc apr_ssize_t ap_rprintf(request_rec *r, const char *fmt, ...) */ -AP_DECLARE_NONSTD(int) ap_rprintf(request_rec *r, const char *fmt,...) +AP_DECLARE_NONSTD(apr_ssize_t) ap_rprintf(request_rec *r, const char *fmt,...) __attribute__((format(printf,2,3))); /** * Flush all of the
Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c util.c util_script.c
Roy T. Fielding wrote: whoa! -1 Was this even discussed on the list? You just changed the entire module API and introduced a dozen potential security holes. The precursor to this patch [PATCH] WIN64: httpd API changes was posted 10/7 so I thought we had had suitable time for discussion. I have addressed the one issue that was raised. There have also been several other threads on the httpd apr lists and the feedback I had received indicated the it was appropriate to sanitize the 64 bit compile even if it incurred httpd API changes. However if there are specific security issues that this has brought up I am more than anxious to address them. Are you opposed to changing the API to fix 64 bit warnings or are there specific issues that I can address and continue to move forward rather that back out the entire patch? Why on earth is it changing nvec to apr_size_t and then downcasting Fixing some of the warnings (below) without resorting to casts ripples through some API's, but changing APR API's at this point is not possible so I had to stop there, with the implication that we have to complete this in APR 2.0. If exceeding 2Gig elements is the security hole you want plugging I can add code to check for that and error out. its use? Why is any of this even needed? These are the 64bit compile warnings I am addressing with this patch .\server\core.c(3959) : warning C4018: '' : signed/unsigned mismatch .\server\core.c(4291) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data .\server\core.c(4296) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data .\server\core.c(4336) : warning C4267: 'function' : conversion from 'size_t' to 'int', possible loss of data .\modules\http\http_protocol.c(665) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data .\modules\http\http_protocol.c(1922) : warning C4267: 'return' : conversion from 'size_t' to 'long', possible loss of data .\server\protocol.c(1400) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data .\server\protocol.c(1464) : warning C4244: 'initializing' : conversion from '__int64' to 'int', possible loss of data .\server\protocol.c(1473) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data .\server\protocol.c(1520) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data .\server\request.c(1231) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data .\server\util.c(461) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(600) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(633) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(663) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(758) : warning C4244: 'function' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(768) : warning C4244: 'function' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(894) : warning C4267: 'function' : conversion from 'size_t' to 'int', possible loss of data .\server\util.c(1195) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(1435) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(1492) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data .\server\util.c(1493) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data .\server\util.c(1978) : warning C4244: 'return' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(1987) : warning C4244: 'return' : conversion from '__int64' to 'int', possible loss of data .\server\util.c(2082) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data .\server\util_script.c(288) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data .\server\util_script.c(289) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data .\server\util_script.c(435) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data .\server\util_script.c(666) : warning C4244: '=' : conversion from '__int64' to 'int', possible loss of data .\server\scoreboard.c(109) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data Allan
Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c util.c util_script.c
The precursor to this patch [PATCH] WIN64: httpd API changes was posted 10/7 so I thought we had had suitable time for discussion. I have addressed the one issue that was raised. That explains why I didn't see it -- I was in Switzerland. There have also been several other threads on the httpd apr lists and the feedback I had received indicated the it was appropriate to sanitize the 64 bit compile even if it incurred httpd API changes. However if there are specific security issues that this has brought up I am more than anxious to address them. Are you opposed to changing the API to fix 64 bit warnings or are there specific issues that I can address and continue to move forward rather that back out the entire patch? I am opposed to changing the API just to mask warnings within the implementations. In any case, these changes cannot possibly be correct -- the API has to be changed from the bottom-up, not top-down. It is far safer to cast module-provided data from int up to 64 bits than it is to cast it down from 64 bit to int. Fix mismatches of the standard library functions first, then fix APR, then carefully change our implementation so that it works efficiently on the right data types as provided by APR, and finally fix the API so that modules can work. If that isn't possible, then just live with those warnings on win64. In any case, changes like +/* Cast to eliminate 64 bit warning */ +rv = apr_file_gets(buf, (int)bufsiz, cfp); are absolutely forbidden. Roy
Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c uti...
Roy is right... Willy-nilly throwing casts on data objects just to satisfy some anal-retentive urge to not see any warnings appearing during a compile is the absolute WRONG thing to do when it comes to porting 32-bit code to 64-bit platforms. The situation is NOT as simple as it was when 32-bits hit the scene and everybody had to port all the 16-bit stuff. I've been working with 64 bit for quite some time and I can tell you that there are still many goblins lurking in this department and many assumptions that are not true. Example... Roy wrote... It is far safer to cast module-provided data from int up to 64 bits than it is to cast it down from 64 bit to int. Probably... but there is an assumption being made there on Roy's part. He is assuming that the cast will automatically clear the high order 32 bits. One SHOULD be able to 'assume' that in order not to violate the 'principle of least astonishment' but, unfortunately, on some current 64 platforms this is not the case. A simple cast of an 'int' to a 'long long' ( or the equivalent 64 bit wide data item ) will NOT 'clear' all the high order bits that reach the target. I am not going to name any names here on this (public) forum but all I can tell you is that if you think a simple typecast in ANY direction ( up or down ) on a 64 bit platform is automatically setting or clearing high/low order bits... think again. Consult with your chip manufacturer and read all defect reports and read your 64 bit compiler or assembler manual carefully. Also... take a (careful) look at the ASM code being produced. Trust... but verify. Roy is absolutely right when he says that the only way to go is from 'the bottom up' ( and keep those chip and compiler manuals and defect reports close by ). Yours... Kevin Kiley In a message dated 10/22/2004 8:20:38 PM Central Daylight Time, [EMAIL PROTECTED] writes: The precursor to this patch "[PATCH] WIN64: httpd API changes" was posted 10/7 so I thought we had had suitable time for discussion. I have addressed the one issue that was raised. That explains why I didn't see it -- I was in Switzerland. There have also been several other threads on the httpd apr lists and the feedback I had received indicated the it was appropriate to sanitize the 64 bit compile even if it incurred httpd API changes. However if there are specific security issues that this has brought up I am more than anxious to address them. Are you opposed to changing the API to fix 64 bit warnings or are there specific issues that I can address and continue to move forward rather that back out the entire patch? I am opposed to changing the API just to mask warnings within the implementations. In any case, these changes cannot possibly be correct -- the API has to be changed from the bottom-up, not top-down. It is far safer to cast module-provided data from int up to 64 bits than it is to cast it down from 64 bit to int. Fix mismatches of the standard library functions first, then fix APR, then carefully change our implementation so that it works efficiently on the right data types as provided by APR, and finally fix the API so that modules can work. If that isn't possible, then just live with those warnings on win64. In any case, changes like + /* Cast to eliminate 64 bit warning */ + rv = apr_file_gets(buf, (int)bufsiz, cfp); are absolutely forbidden. Roy
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: jorton 2004/05/17 08:24:31 Modified:server core.c Log: * server/core.c (core_output_filter): Don't explicitly delete the EOC bucket, and don't buffer the brigade if it ends in an EOC. Won't this change result in a memory leak? Bill
Re: cvs commit: httpd-2.0/server core.c
On Mon, May 17, 2004 at 01:06:04PM -0400, Bill Stoddard wrote: [EMAIL PROTECTED] wrote: jorton 2004/05/17 08:24:31 Modified:server core.c Log: * server/core.c (core_output_filter): Don't explicitly delete the EOC bucket, and don't buffer the brigade if it ends in an EOC. Won't this change result in a memory leak? The brigade is destroyed further down the function, so no. joe
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: nd 2004/04/10 11:40:53 Modified:server core.c Log: ErrorDocument default changes broke inheritance. consider: Directory /foo ErrorDocument 404 blah /Directory DIrectory /foo/bar ErrorDocument 500 boo # 404 is now fallen back to default /Directory This patch solves the problem. thanks for catching that. guess my test suite wasn't quite good enough :) --Geoff
Re: cvs commit: httpd-2.0/server core.c
On Sat, 10 Apr 2004 [EMAIL PROTECTED] wrote: nd 2004/04/10 14:44:43 Modified:.CHANGES server core.c Log: accept URLs as ServerAdmin contact. If it's not recognized as an URL, assume an email address and prepend it with mailto: in server outputs. Careful with this, since I believe it also affects the SERVER_ADMIN env variable. The docs should warn that some scripts might depend on this being an email address. Joshua.
Re: cvs commit: httpd-2.0/server core.c
* Joshua Slive [EMAIL PROTECTED] wrote: Modified:.CHANGES server core.c Log: accept URLs as ServerAdmin contact. If it's not recognized as an URL, assume an email address and prepend it with mailto: in server outputs. Careful with this, since I believe it also affects the SERVER_ADMIN env variable. The docs should warn that some scripts might depend on this being an email address. Yep. I'm just changing the docs. I don't see it that critical anyway, since it was always just a string and even might be unset (which leads to a non-email default). nd
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: geoff 2004/04/08 17:56:26 Modified:.CHANGES server core.c Log: Enable special ErrorDocument value 'default' which restores the canned server response for the scope of the directive. if this stays in 2.1 it will need some docs. I saw that the 2.1 docs are maintained in HEAD just as with the code, but I'm unsure of whether to edit the .xml, .html.en, or both. --Geoff
Re: cvs commit: httpd-2.0/server core.c
On Thu, 8 Apr 2004, Geoffrey Young wrote: [EMAIL PROTECTED] wrote: geoff 2004/04/08 17:56:26 Modified:.CHANGES server core.c Log: Enable special ErrorDocument value 'default' which restores the canned server response for the scope of the directive. if this stays in 2.1 it will need some docs. I saw that the 2.1 docs are maintained in HEAD just as with the code, but I'm unsure of whether to edit the .xml, .html.en, or both. Just edit the .xml. (There is a big ugly note at the top of the .html.en telling you not to touch it.) Then you can build the .html.en using the instructions here: http://httpd.apache.org/docs-project/docsformat.html Or if you don't want to bother, just do the .xml and someone else will take care of building the .html. Joshua.
Re: cvs commit: httpd-2.0/server core.c
Just edit the .xml. (There is a big ugly note at the top of the .html.en telling you not to touch it.) Then you can build the .html.en using the instructions here: http://httpd.apache.org/docs-project/docsformat.html Or if you don't want to bother, just do the .xml and someone else will take care of building the .html. cool, I'll do that. thanks. (and sorry I didn't see the note :) --Geoff
Re: cvs commit: httpd-2.0/server core.c
Uhmmm, this isn't an MMN bump :) If you were adding this new structure *type* as a member of another structure (say as the last member of the conn_rec) and the structure grew (in such a way that folks could be blindly oblivious to the fact that conn_rec just got a bit bigger), that's an MMN bump. It's not the case here. MMN bumps are for module authors who must be pedantic, e.g. they create their own Apache structures, e.g. conn_rec's and pass them around. When a module does something like that, it needs to be alerted that the structures that it creates might not be 100% binary compatible. MMN bumps are for producers of apache-like structures, not for the consumers who trust the elements they need to retrieve from the structures are still present in the new version. Bill At 04:03 PM 1/25/2004, [EMAIL PROTECTED] wrote: nd 2004/01/25 14:03:38 Modified:.CHANGES include ap_mmn.h ap_release.h httpd.h server core.c Log: Add core version query function ap_get_server_revision and accompanying ap_version_t structure (minor MMN bump). The function is similar to apr_version() and allow for exact querying of the core revision level. Revision ChangesPath 1.1377+4 -0 httpd-2.0/CHANGES Index: CHANGES === RCS file: /home/cvs/httpd-2.0/CHANGES,v retrieving revision 1.1376 retrieving revision 1.1377 diff -u -u -r1.1376 -r1.1377 --- CHANGES 25 Jan 2004 15:40:07 - 1.1376 +++ CHANGES 25 Jan 2004 22:03:38 - 1.1377 @@ -2,6 +2,10 @@ [Remove entries to the current 2.0 section below, when backported] + *) Add core version query function (ap_get_server_revision) and + accompanying ap_version_t structure (minor MMN bump). + [André Malo] + *) mod_rewrite: EOLs sent by external rewritemaps are now consumed as whole. That way, on systems with more than one EOL character rewritemap programs no longer need to switch stdout to binary 1.63 +2 -1 httpd-2.0/include/ap_mmn.h Index: ap_mmn.h === RCS file: /home/cvs/httpd-2.0/include/ap_mmn.h,v retrieving revision 1.62 retrieving revision 1.63 diff -u -u -r1.62 -r1.63 --- ap_mmn.h 1 Jan 2004 13:26:16 - 1.62 +++ ap_mmn.h 25 Jan 2004 22:03:38 - 1.63 @@ -118,6 +118,7 @@ * 20030821 (2.1.0-dev) bumped mod_include's entire API * 20030821.1 (2.1.0-dev) added XHTML doctypes * 20030821.2 (2.1.0-dev) added ap_escape_errorlog_item + * 20030821.3 (2.1.0-dev) added ap_get_server_revision / ap_version_t */ #define MODULE_MAGIC_COOKIE 0x41503230UL /* AP20 */ @@ -125,7 +126,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20030821 #endif -#define MODULE_MAGIC_NUMBER_MINOR 2 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 3 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a 1.81 +14 -3 httpd-2.0/include/ap_release.h Index: ap_release.h === RCS file: /home/cvs/httpd-2.0/include/ap_release.h,v retrieving revision 1.80 retrieving revision 1.81 diff -u -u -r1.80 -r1.81 --- ap_release.h 1 Jan 2004 13:26:16 - 1.80 +++ ap_release.h 25 Jan 2004 22:03:38 - 1.81 @@ -59,6 +59,8 @@ #ifndef AP_RELEASE_H #define AP_RELEASE_H +#include apr_general.h /* stringify */ + /* * The below defines the base string of the Server: header. Additional * tokens can be added via the ap_add_version_component() API call. @@ -73,9 +75,18 @@ */ #define AP_SERVER_BASEVENDOR Apache Software Foundation #define AP_SERVER_BASEPRODUCT Apache -#define AP_SERVER_MAJORVERSION 2 -#define AP_SERVER_MINORVERSION 1 -#define AP_SERVER_PATCHLEVEL 0-dev + +#define AP_SERVER_MAJORVERSION_NUMBER 2 +#define AP_SERVER_MINORVERSION_NUMBER 1 +#define AP_SERVER_PATCHLEVEL_NUMBER 0 +#define AP_SERVER_ADD_STRING -dev + +/* keep old macros as well */ +#define AP_SERVER_MAJORVERSION APR_STRINGIFY(AP_SERVER_MAJORVERSION_NUMBER) +#define AP_SERVER_MINORVERSION APR_STRINGIFY(AP_SERVER_MINORVERSION_NUMBER) +#define AP_SERVER_PATCHLEVEL APR_STRINGIFY(AP_SERVER_PATCHLEVEL_NUMBER) \ + AP_SERVER_ADD_STRING + #define AP_SERVER_MINORREVISION AP_SERVER_MAJORVERSION . AP_SERVER_MINORVERSION #define AP_SERVER_BASEREVISION AP_SERVER_MINORREVISION . AP_SERVER_PATCHLEVEL #define AP_SERVER_BASEVERSION AP_SERVER_BASEPRODUCT / AP_SERVER_BASEREVISION 1.206 +19 -0 httpd-2.0/include/httpd.h Index: httpd.h === RCS file:
Re: cvs commit: httpd-2.0/server core.c
On Tue, 3 Feb 2004, William A. Rowe, Jr. wrote: Uhmmm, this isn't an MMN bump :) If you were adding this new structure *type* as a member of another structure (say as the last member of the conn_rec) and the structure grew (in such a way that folks could be blindly oblivious to the fact that conn_rec just got a bit bigger), that's an MMN bump. It's not the case here. I disagree. If you add a function to the API without bumping the MMN, then how is a module that wants to use the new function if it exists and yet still compile if it doesn't supposed to work? It can't. --Cliff
Re: cvs commit: httpd-2.0/server core.c
* Cliff Woolley [EMAIL PROTECTED] wrote: On Tue, 3 Feb 2004, William A. Rowe, Jr. wrote: Uhmmm, this isn't an MMN bump :) If you were adding this new structure *type* as a member of another structure (say as the last member of the conn_rec) and the structure grew (in such a way that folks could be blindly oblivious to the fact that conn_rec just got a bit bigger), that's an MMN bump. It's not the case here. I disagree. If you add a function to the API without bumping the MMN, then how is a module that wants to use the new function if it exists and yet still compile if it doesn't supposed to work? It can't. Yes, that's my understanding as well: major bumps break backwards compat and minor bumps keep forward compat. nd
Re: [Bug?] cvs commit: httpd-2.0/server core.c
What about the case where they did have sendfile, but did not use large file support? [Did/Do] we attempt to test the EnableSendfile logic? Yes, that happens in core.c/default_handler() when apr_file_open() is called. If the build has send_file support then the APR_SENDFILE_ENABLED flag is passed to apr_file_open() based on the ENABLE_SENDFILE_* flag. Not arguing that breaking up huge responses is a bad thing :) However I'm somewhat confused why apr doesn't handle this gracefully. Agreed. There seems to be a lot of code in core.c that is based on the APR_HAS_SENDFILE flag. My guess is that at least some of it, if not all, could be cleaned up. But then I am not on a platform that has sendfile() so it is hard for me to tell what is required and what isn't. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com William A. Rowe, Jr. [EMAIL PROTECTED] Tuesday, January 13, 2004 7:01:06 PM At 07:05 PM 1/13/2004, Brad Nicholes wrote: I don't think so because the split into multiple bucket code was only enabled if both large_file and send_file was enabled. Which meant that on a non-large_file build the check for ENABLE_SENDFILE_OFF wasn't there anyway. If they have large_file support and don't have send_file (ie. NetWare), then the file must be split into multiple buckets or it doesn't work (32/64 bit type mismatches in the file size). If they have large_file support and send_file, then everything is as it was before. What about the case where they did have sendfile, but did not use large file support? [Did/Do] we attempt to test the EnableSendfile logic? If not, perhaps we should. There are other cases, e.g. some NFS volume strategies, where a raw kernel sendfile turns out to be fatal on some platforms. I'm not sure why a check for ENABLE_SENDFILE_OFF is here anyway. This code doesn't really have anything to do with whether or not sendfile is used. All it does is split a large file into multiple smaller buckets. If later down the line sendfile is used to actually send the file from multiple buckets, great. If not, that is fine also (as demonstrated by the fact that NetWare doesn't have sendfile() and it all works fine). Not arguing that breaking up huge responses is a bad thing :) However I'm somewhat confused why apr doesn't handle this gracefully. Bill
Re: [Bug?] cvs commit: httpd-2.0/server core.c
Woha...
At 11:50 AM 1/8/2004, [EMAIL PROTECTED] wrote:
bnicholes2004/01/08 09:50:03
Modified:server core.c
Log:
If large file support is enabled allow the file to be split into AP_MAX_SENDFILE
sized buckets. Otherwise Apache will be unable to send files larger than 2 gig due
to signed 32-bit limitations.
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.254
retrieving revision 1.255
diff -u -r1.254 -r1.255
--- core.c1 Jan 2004 13:26:23 - 1.254
+++ core.c8 Jan 2004 17:50:03 - 1.255
@@ -3508,8 +3508,12 @@
}
bb = apr_brigade_create(r-pool, c-bucket_alloc);
-#if APR_HAS_SENDFILE APR_HAS_LARGE_FILES
+#if APR_HAS_LARGE_FILES
+#if APR_HAS_SENDFILE
if ((d-enable_sendfile != ENABLE_SENDFILE_OFF)
+#else
+if (
+#endif
(r-finfo.size AP_MAX_SENDFILE)) {
/* APR_HAS_LARGE_FILES issue; must split into mutiple buckets,
* no greater than MAX(apr_size_t), and more granular than that
Ok that is a messy one to grok but I think I got it...
Haven't you broken the EnableSendfile off directive if the user is trying to avoid
using sendfile on non-largefile builds?
E.g. if someone determines that their sendfile implementation is broken, will
the server still react properly to EnableSendfile off on linux or bsd?
Bill
Re: [Bug?] cvs commit: httpd-2.0/server core.c
I don't think so because the split into multiple bucket code was
only enabled if both large_file and send_file was enabled. Which meant
that on a non-large_file build the check for ENABLE_SENDFILE_OFF wasn't
there anyway. If they have large_file support and don't have send_file
(ie. NetWare), then the file must be split into multiple buckets or it
doesn't work (32/64 bit type mismatches in the file size). If they have
large_file support and send_file, then everything is as it was before.
I'm not sure why a check for ENABLE_SENDFILE_OFF is here anyway. This
code doesn't really have anything to do with whether or not sendfile is
used. All it does is split a large file into multiple smaller buckets.
If later down the line sendfile is used to actually send the file from
multiple buckets, great. If not, that is fine also (as demonstrated by
the fact that NetWare doesn't have sendfile() and it all works fine).
Brad
Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com
[EMAIL PROTECTED] Tuesday, January 13, 2004 4:04:30 PM
Woha...
At 11:50 AM 1/8/2004, [EMAIL PROTECTED] wrote:
bnicholes2004/01/08 09:50:03
Modified:server core.c
Log:
If large file support is enabled allow the file to be split into
AP_MAX_SENDFILE sized buckets. Otherwise Apache will be unable to send
files larger than 2 gig due to signed 32-bit limitations.
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.254
retrieving revision 1.255
diff -u -r1.254 -r1.255
--- core.c1 Jan 2004 13:26:23 - 1.254
+++ core.c8 Jan 2004 17:50:03 - 1.255
@@ -3508,8 +3508,12 @@
}
bb = apr_brigade_create(r-pool, c-bucket_alloc);
-#if APR_HAS_SENDFILE APR_HAS_LARGE_FILES
+#if APR_HAS_LARGE_FILES
+#if APR_HAS_SENDFILE
if ((d-enable_sendfile != ENABLE_SENDFILE_OFF)
+#else
+if (
+#endif
(r-finfo.size AP_MAX_SENDFILE)) {
/* APR_HAS_LARGE_FILES issue; must split into mutiple
buckets,
* no greater than MAX(apr_size_t), and more granular
than that
Ok that is a messy one to grok but I think I got it...
Haven't you broken the EnableSendfile off directive if the user is
trying to avoid
using sendfile on non-largefile builds?
E.g. if someone determines that their sendfile implementation is
broken, will
the server still react properly to EnableSendfile off on linux or bsd?
Bill
Re: [Bug?] cvs commit: httpd-2.0/server core.c
At 07:05 PM 1/13/2004, Brad Nicholes wrote: I don't think so because the split into multiple bucket code was only enabled if both large_file and send_file was enabled. Which meant that on a non-large_file build the check for ENABLE_SENDFILE_OFF wasn't there anyway. If they have large_file support and don't have send_file (ie. NetWare), then the file must be split into multiple buckets or it doesn't work (32/64 bit type mismatches in the file size). If they have large_file support and send_file, then everything is as it was before. What about the case where they did have sendfile, but did not use large file support? [Did/Do] we attempt to test the EnableSendfile logic? If not, perhaps we should. There are other cases, e.g. some NFS volume strategies, where a raw kernel sendfile turns out to be fatal on some platforms. I'm not sure why a check for ENABLE_SENDFILE_OFF is here anyway. This code doesn't really have anything to do with whether or not sendfile is used. All it does is split a large file into multiple smaller buckets. If later down the line sendfile is used to actually send the file from multiple buckets, great. If not, that is fine also (as demonstrated by the fact that NetWare doesn't have sendfile() and it all works fine). Not arguing that breaking up huge responses is a bad thing :) However I'm somewhat confused why apr doesn't handle this gracefully. Bill
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: trawick 2003/12/10 14:40:33 Modified:.CHANGES server core.c Log: Fix Limit and LimitExcept parsing to require a closing '' in the initial container. PR:25414 Submitted by: Geoffrey Young geoff apache.org] Reviewed by: Jeff Trawick excellent, thanks. and sorry about the space problems in the patch. I usually use attachments but didn't see the upload field in the bug form, which I'll use from now on. --Geoff
RE: cvs commit: httpd-2.0/server core.c request.c
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] Sent: Tuesday, July 01, 2003 3:25 AM striker 2003/06/30 18:25:07 Modified:.Tag: APACHE_2_0_BRANCH CHANGES STATUS include Tag: APACHE_2_0_BRANCH http_core.h modules/http Tag: APACHE_2_0_BRANCH http_request.c server Tag: APACHE_2_0_BRANCH core.c request.c Removed: buildTag: APACHE_2_0_BRANCH httpd_roll_release Whoops! Well, the thing had to go some day, because I don't want someone making the mistake of using it anymore. It wasn't supposed to go in this commit though. Sorry for the noise. Sander
Re: cvs commit: httpd-2.0/server core.c request.c util.c
William A. Rowe, Jr. wrote: The configuration and context below seems odd to me; : You haven't resolved any Directory, Files, Locations etc in the code fragment above... it's too early in the request processing cycle. It seems this should not be a dir_conf flag, but actually a server_conf flag since your patch only resolves the directive relative to a given server. that's right, i didn't. as explained earlier, this is currently RSRC_CONF, which means it *can't* appear in any of those containers, so that point is moot. that makes it essentially server-scope -- but by treating it as per-dir now, we don't need to shift from per-server to per-dir when we have a more finely grained solution. And what is the impact of this patch on proxies and using mod_rewrite to proxy certain URIs? i will investigate. i'm tempted to consider this a piece of rope, however, and as long as it doesn't open any security exposures, caveat emptor. It would be best if we unparsed and tracked the offsets in the source and unescaped query strings of individual components (scheme, user, host, path, path_info and query). We could do something as simple as counting the number of slashes in the source and target paths, and failing only when those two components mismatch. this has been mentioned several times as a 'gee, it would be nice.' unfortunately, it is also a major hassle due to all the rewriting potential, et cetera. it's definitely for later. -- #kenP-)} Ken Coar, Sanagendamgagwedweinini http://Golux.Com/coar/ Author, developer, opinionist http://Apache-Server.Com/ Millennium hand and shrimp!
Re: cvs commit: httpd-2.0/server core.c request.c util.c
coar2003/01/23 13:34:14
Modified:include ap_mmn.h http_core.h httpd.h
server core.c request.c util.c
Log:
here we go. add a directive that will keep %2f from being
decoded into '/', allowing the *_walk to do their magic and
return 404 if it's in the path, and allowing it in the path-info.
The configuration and context below seems odd to me;
--- core.c18 Jan 2003 03:37:54 - 1.229
+++ core.c23 Jan 2003 21:34:13 - 1.230
@@ -3054,6 +3070,8 @@
AP_INIT_ITERATE2(AddOutputFilterByType, add_ct_output_filters,
(void *)APR_OFFSETOF(core_dir_config, ct_output_filters), OR_FILEINFO,
output filter name followed by one or more content-types),
+AP_INIT_FLAG(AllowEncodedSlashes, set_allow2f, NULL, RSRC_CONF,
+ Allow URLs containing '/' encoded as '%2F'),
--- request.c 12 Dec 2002 07:05:54 - 1.122
+++ request.c 23 Jan 2003 21:34:13 - 1.123
@@ -147,13 +147,22 @@
/* Ignore embedded %2F's in path for proxy requests */
if (!r-proxyreq r-parsed_uri.path) {
-access_status = ap_unescape_url(r-parsed_uri.path);
+core_dir_config *d;
+d = ap_get_module_config(r-per_dir_config, core_module);
+if (d-allow_encoded_slashes) {
+access_status = ap_unescape_url_keep2f(r-parsed_uri.path);
+}
+else {
+access_status = ap_unescape_url(r-parsed_uri.path);
+}
You haven't resolved any Directory, Files, Locations etc in the
code fragment above... it's too early in the request processing cycle.
It seems this should not be a dir_conf flag, but actually a server_conf flag
since your patch only resolves the directive relative to a given server.
Does this make sense?
And what is the impact of this patch on proxies and using mod_rewrite
to proxy certain URIs? It seems folks have %2f's popping up more and
more often in their query args - and we have a number of much more
pressing bugs that the query /foo?answer=yes%2fno would be discarded.
Usually, path_info is under the web developer's control - but the query
string is rarely in our control, but the users/browsers' control.
I'm afraid that the proxied flavor of this would result in ?answer=yes%2fno
which would be re-escaped to the backend server or redirect as something
like ?answer=yes%252fno which would be a real problem.
It would be good to come up with a schema where the unparse/parse would
actually be their reciprocals.
It would be better if unparse/parse would respect the query_args and be
just a little more tolerant of %2f once the path is complete and the query
string begins.
It would be best if we unparsed and tracked the offsets in the source and
unescaped query strings of individual components (scheme, user, host,
path, path_info and query). We could do something as simple as counting
the number of slashes in the source and target paths, and failing only when
those two components mismatch.
Bill
Re: cvs commit: httpd-2.0/server core.c request.c util.c
* [EMAIL PROTECTED] wrote:
Index: ap_mmn.h
===
RCS file: /home/cvs/httpd-2.0/include/ap_mmn.h,v
retrieving revision 1.52
retrieving revision 1.53
diff -u -u -r1.52 -r1.53
--- ap_mmn.h3 Sep 2002 23:39:43 - 1.52
+++ ap_mmn.h23 Jan 2003 21:34:12 - 1.53
@@ -111,6 +111,7 @@
* 20020625 (2.0.40-dev) Changed conn_rec-keepalive to an enumeration
* 20020628 (2.0.40-dev) Added filter_init to filter registration functions
* 20020903 (2.0.41-dev) APR's error constants changed
+ * 20020903.1 (2.0.44-dev) allow_encoded_slashes added to core_dir_config
This should now be 2.0.45-dev, shouldn't it?
nd
--
$_=q?tvc!uif)%*|#Bopuifs!A`#~tvc!Xibu)%*|qsjou#Kvtu!A`#~tvc!KBQI!)*|~
tvc!ifmm)%*|#Qfsm!A`#~tvc!jt)%*|(Ibdlfs(~ # What the hell is JAPH? ;
@_=split/\s\s+#/;$_=(join''=map{chr(ord( # André Malo ;
$_)-1)}split//=$_[0]).$_[1];s s.*s$_see; # http://www.perlig.de/ ;
Re: cvs commit: httpd-2.0/server core.c request.c util.c
* Justin Erenkrantz wrote: --On Friday, January 24, 2003 00:11:22 +0100 André Malo [EMAIL PROTECTED] wrote: + * 20020903.1 (2.0.44-dev) allow_encoded_slashes added to core_dir_config This should now be 2.0.45-dev, shouldn't it? 2.1.0-dev. -- justin *err* yes ... nd -- die (eval q-qq:Just Another Perl Hacker :-) # André Malo, http://www.perlig.de/ #
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: gregames2002/12/09 14:19:26 Modified:.CHANGES server core.c Log: core_output_filter: re-instate the deferred_write pool patch so we don't leak fd's until the end of a keepalive connection. Thanks to: Jeff Trawick for the original concept Sander Striker for the mmap ring idea Cliff Woolley for implementing the above change Can I get some +1's for moving this into stable (or -1's and reasons not to do so)? - Its one month birthday is tomorrow, - it is certainly in mainline code, and - AFAIK it hasn't caused any problems since the apr_mmap ownership redesign. Greg
Re: cvs commit: httpd-2.0/server core.c
On Wed, 8 Jan 2003, Greg Ames wrote: Can I get some +1's for moving this into stable (or -1's and reasons not to do so)? - Its one month birthday is tomorrow, - it is certainly in mainline code, and - AFAIK it hasn't caused any problems since the apr_mmap ownership redesign. +1 from here --Cliff
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: can't mix declarations and statements except with gcc 3.2 :) (or possibly RedHat 8's patches to it) ouch... Thanks for cleaning it up, Jeff. Greg
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote:
trawick 2002/11/08 05:17:09
Modified:server core.c
Log:
adjust some parents to make the expression a tiny bit clearer and to
make gcc happy
core.c: In function `core_output_filter':
core.c:3884: warning: suggest parentheses around within ||
Revision ChangesPath
1.219 +2 -2 httpd-2.0/server/core.c
Index: core.c
===
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.218
retrieving revision 1.219
diff -u -r1.218 -r1.219
--- core.c 6 Nov 2002 19:34:51 - 1.218
+++ core.c 8 Nov 2002 13:17:09 - 1.219
@@ -3878,8 +3878,8 @@
* (IOW, this response is a bit more complex, but we save it
* with the hope of concatenating with another response)
*/
-if (nbytes + flen AP_MIN_BYTES_TO_WRITE
- (!fd !more !APR_BUCKET_IS_FLUSH(last_e))
+if ((nbytes + flen AP_MIN_BYTES_TO_WRITE
+ !fd !more !APR_BUCKET_IS_FLUSH(last_e))
|| (APR_BUCKET_IS_EOS(last_e)
c-keepalive == AP_CONN_KEEPALIVE)) {
ooops...the nbytes subexpression is supposed to be common to both of the OR
clauses, like it was before my commit. I'll straighten it out after lunch.
hmmm, I wonder why I didn't see gcc warnings...will investigate.
Greg
Re: FW: cvs commit: httpd-2.0/server core.c
At 08:47 AM 10/18/2002, Bill Stoddard wrote: Hey Bill, Just ran into a problem with this patch... mod_mem_cache has an option to cache open file descriptors. With this patch, we will never used sendfile to send a cached fd. This 'should' work... where is the code broken? We cache the open file descriptor from a first request, no? So the flag should be set on that initial request, correct? When we set aside the 'fd', we really must set aside the flags (so we know what sort of fd we set aside.) The apr_file_dup should be 'keeping' that flag from the original fd when creating the long-lived fd. Can you point out where in the code that flag disappears? wrowe 2002/10/14 10:42:45 Modified:include http_core.h server core.c Log: Introduce an EnableSendfile directive (defaulted to ON) to allow users to disable sendfile mechanics for NFS volume mounts and other kernel objects that don't support sendfile. And EnableSendfile off can be used to help narrow bugs down to the sendfile mechanics or eliminate the possiblity that sendfile is a factor on any given platform.
RE: FW: cvs commit: httpd-2.0/server core.c
I see what the problem is now. Patch on the way. Bill At 08:47 AM 10/18/2002, Bill Stoddard wrote: Hey Bill, Just ran into a problem with this patch... mod_mem_cache has an option to cache open file descriptors. With this patch, we will never used sendfile to send a cached fd. This 'should' work... where is the code broken? We cache the open file descriptor from a first request, no? So the flag should be set on that initial request, correct? When we set aside the 'fd', we really must set aside the flags (so we know what sort of fd we set aside.) The apr_file_dup should be 'keeping' that flag from the original fd when creating the long-lived fd. Can you point out where in the code that flag disappears? wrowe 2002/10/14 10:42:45 Modified:include http_core.h server core.c Log: Introduce an EnableSendfile directive (defaulted to ON) to allow users to disable sendfile mechanics for NFS volume mounts and other kernel objects that don't support sendfile. And EnableSendfile off can be used to help narrow bugs down to the sendfile mechanics or eliminate the possiblity that sendfile is a factor on any given platform.
FW: cvs commit: httpd-2.0/server core.c
Hey Bill, Just ran into a problem with this patch... mod_mem_cache has an option to cache open file descriptors. With this patch, we will never used sendfile to send a cached fd. BillS wrowe 2002/10/14 10:42:45 Modified:include http_core.h server core.c Log: Introduce an EnableSendfile directive (defaulted to ON) to allow users to disable sendfile mechanics for NFS volume mounts and other kernel objects that don't support sendfile. And EnableSendfile off can be used to help narrow bugs down to the sendfile mechanics or eliminate the possiblity that sendfile is a factor on any given platform.
Re: cvs commit: httpd-2.0/server core.c util_script.c
On Wed, Oct 02, 2002 at 03:54:18PM -0700, Joshua Slive wrote: On Wed, 2 Oct 2002, Marc Slemko wrote: Lets not encode env variables, as we discussed earlier. Escaping them is bogus and doesn't solve anything since there are all sorts of variables that aren't and shouldn't be encoded. +1 to what Marc says. The encoding serves no purpose. Preventing the sample cgi scripts from being included in a make install would be a great idea. If this change does get reverted so that SERVER_NAME is not encoded internally again, the error includes need updating so that is is encoded there instead: --- error/include/bottom.html~ 2002-07-11 20:07:03.0 +0100 +++ error/include/bottom.html 2002-10-09 12:04:59.0 +0100 -6,7 +6,7 dl dd address - a href=/!--#echo encoding=none var=SERVER_NAME --/a + a href=/!--#echo encoding=url var=SERVER_NAME --/a br / !--#config timefmt=%c -- small!--#echo encoding=none var=DATE_LOCAL --/small
Re: cvs commit: httpd-2.0/server core.c util_script.c
* Joe Orton wrote:
If this change does get reverted so that SERVER_NAME is not encoded
internally again, the error includes need updating so that is is
encoded there instead:
--- error/include/bottom.html~ 2002-07-11 20:07:03.0 +0100
+++ error/include/bottom.html 2002-10-09 12:04:59.0 +0100
-6,7 +6,7
dl
dd
address
- a href=/!--#echo encoding=none var=SERVER_NAME --/a
+ a href=/!--#echo encoding=url var=SERVER_NAME --/a
should be encoding=entity. url is not right here.
nd
--
package Hacker::Perl::Another::Just;print
qq~@{[reverse split/::/ =__PACKAGE__]}~;
# André Malo # http://www.perlig.de #
Re: cvs commit: httpd-2.0/server core.c util_script.c
On Wed, 2 Oct 2002, Marc Slemko wrote: Lets not encode env variables, as we discussed earlier. Escaping them is bogus and doesn't solve anything since there are all sorts of variables that aren't and shouldn't be encoded. +1 to what Marc says. The encoding serves no purpose. Preventing the sample cgi scripts from being included in a make install would be a great idea. Joshua.
Re: cvs commit: httpd-2.0/server core.c util_script.c
If someone reverts that -part- of the commit, and changes CHANGES to reflect this group decision, I will bring in that commit tomorrow a.m. before the final tag and roll. Sorry if I wasn't clear on the consensus decision. +1. Bill At 05:54 PM 10/2/2002, Joshua Slive wrote: On Wed, 2 Oct 2002, Marc Slemko wrote: Lets not encode env variables, as we discussed earlier. Escaping them is bogus and doesn't solve anything since there are all sorts of variables that aren't and shouldn't be encoded. +1 to what Marc says. The encoding serves no purpose. Preventing the sample cgi scripts from being included in a make install would be a great idea. Joshua.
Re: cvs commit: httpd-2.0/server core.c
On 1 Oct 2002 [EMAIL PROTECTED] wrote:
gstein 2002/10/01 09:24:41
Modified:server core.c
Log:
Fix bug in the default handler. POST is not allowed on regular files.
The resource must be handled by something *other* than the default
handler.
-1. This is going to break PHP. PHP is a filter now, which means that
the page is served by the default_handler. Since PHP requests are allowed
to use POST, this is now broken.
As I said before, the bug is in mod_dav, and must be fixed there.
Ryan
Revision ChangesPath
1.207 +8 -0 httpd-2.0/server/core.c
Index: core.c
===
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.206
retrieving revision 1.207
diff -u -r1.206 -r1.207
--- core.c 30 Sep 2002 23:43:18 - 1.206
+++ core.c 1 Oct 2002 16:24:41 - 1.207
@@ -3259,6 +3259,14 @@
return HTTP_NOT_FOUND;
}
+/* we understood the POST method, but it isn't legal for this
+ particular resource. */
+if (r-method_number == M_POST) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ This resource does not accept the POST method.);
+return HTTP_METHOD_NOT_ALLOWED;
+}
+
if ((status = apr_file_open(fd, r-filename, APR_READ | APR_BINARY, 0,
r-pool)) != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
--
___
Ryan Bloom [EMAIL PROTECTED]
550 Jean St
Oakland CA 94610
---
PHP POST handling (was: cvs commit: httpd-2.0/server core.c)
On Tue, Oct 01, 2002 at 12:29:53PM -0400, [EMAIL PROTECTED] wrote: On 1 Oct 2002 [EMAIL PROTECTED] wrote: gstein 2002/10/01 09:24:41 Modified:server core.c Log: Fix bug in the default handler. POST is not allowed on regular files. The resource must be handled by something *other* than the default handler. -1. This is going to break PHP. PHP is a filter now, which means that the page is served by the default_handler. Since PHP requests are allowed to use POST, this is now broken. As I said before, the bug is in mod_dav, and must be fixed there. That will get fixed. I keep saying that, but you keep harping on default handler isn't broken. Bunk. The default_handler is broken. If you POST to a resource, it returns the resource. That isn't right. But the PHP point is a good one. So how do we prevent a POST from returning the resource, yet make it available for PHP? I think that we have a model problem here. For a POST method, you need something to *handle* the POST. Somebody actually needs to accept the posted content, do something with it, and then *generate* a response. That response might actually have other filter processing to apply to it. It almost seems like PHP needs to act as a handler for POST requests (to deal with the posted content), and as a filter for GET requests. Does that seem right? Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: jerenkrantz2002/06/26 22:00:23 Modified:server core.c Log: - Fix segfault in core_output_filter when we are passed an empty brigade. - Stash the remainder of the brigade in more when we see a flush bucket. Previous to this commit, we would only process the buckets before the flush on a HTTP/1.0 request and then return. We are proably only finding this now since we now correctly check for keepalive connection status. Do you have a simple test case for this? I'd like to add it it to httpd-test. Thanks, --Brian
Re: cvs commit: httpd-2.0/server core.c
On Wed, Jun 26, 2002 at 10:20:56PM -0700, Brian Pane wrote: Do you have a simple test case for this? I'd like to add it it to httpd-test. You already have one. =) mod_bucketeer's test portion in include.t caught it. They are now passing for me. Horray for mod_bucketeer... -- justin
Re: cvs commit: httpd-2.0/server core.c protocol.c
[EMAIL PROTECTED] writes: jerenkrantz2002/06/26 12:45:08 Modified:.CHANGES include ap_mmn.h httpd.h modules/arch/win32 mod_isapi.c modules/http http_core.c http_protocol.c http_request.c modules/loggers mod_log_config.c modules/proxy proxy_http.c server core.c protocol.c Log: Change conn_rec-keepalive to an enumerated value of AP_CONN_UNKNOWN AP_CONN_CLOSE AP_CONN_KEEPALIVE This also fixes a problem where ap_discard_request_body would not discard the body when keepalive was 0. This actually meant the keepalive status was unknown *not* closed, but no one ever remembered that. This problem was seen with mod_dav sending error responses (as reported by Karl Fogel). Thanks, this resolves some bogus-chunk-header tests that started failing with Greg Ames' commit a couple of days ago. -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
On 15 Jun 2002 [EMAIL PROTECTED] wrote: rbb 2002/06/14 22:49:06 Modified:.CHANGES server core.c Log: Make the default_handler catch all requests that aren't served by another handler. This also gets us to return a 404 if a directory is requested, there is no DirectoryIndex, and mod_autoindex isn't loaded. PR: 8045 Submitted by: Justin Erenkrantz Just to clarify. There was a comment in PR 8045 that returning 500 was better than 404 if a directory exists, but there is no index. I completely disagree. Users see a 500, and they get scared. IMO, a 404 means that we could not find a valid response, which is this case exactly. Plus, this matches what we did in 1.3, and users expect it. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 550 Jean St Oakland CA 94610 ---
config handling (was: Re: cvs commit: httpd-2.0/server core.c)
On Sat, May 18, 2002 at 12:32:20PM -0500, William A. Rowe, Jr. wrote: ... Ok, segfault is gone. So is some validation that ServerRoot occured in a global context. Actually, that means that LoadModule and others are not tested to be in the global context either. (see further below) ... I'm just getting my head wrapped around cfg parsing, so anyone with a whole lot more background here might be able to provide pointers. Notice that we can't defer ServerRoot processing unless we defer LoadModule processing. EXEC_ON_READ is *ONLY* intended to be used for things like LoadModule. Essentially, LoadModule needs to be executed right away so that we can properly *parse* the rest of the file. Outside of the initial parse into the configuration tree, and whatever is needed to build that tree, NOTHING should ever be EXEC_ON_READ. I know that some things have gotten a bit sloppy with that, but I've never had the time or inclination to go and do a review. Once you get the tree loaded, then you should be able to simply scan it for the validation. You should be able to detect an improperly positioned ServerRoot just by looking at the tree. Note that the ideal config parse/load looks like: 1) process the config, constructing the tree. invoke any EXEC_ON_READ directives, as these are required to complete this step 2) process the tree to find logging directives and execute them [ these might be EXEC_ON_READ today, which would be abdness ] 3) process the tree for all the directives, now that logging is available Note that we really only need to hit the file once to parse the tree. The logic was never completed for this, however, so we actually read/parse the tree twice. If we REALLY get a better handle on this, I see no reason to maintain the test-load then re-load configuration logic. If we can pre-process what we must with EXEC_ON_READ, then open logs and the like, we should be able to directly proceed to run_mpm, without dumping it all and reloading. Yup. On Win32, we load-unload-reload the parent, then load-unload-reload the child config. Losing both redundant unload-load sequences will be a huge win at startup. Yup. If we process the tree in a much smarter fashion, then nothing should need to be unloaded. ... Regarding the first question, about validating ServerRoot. As I mentioned above, you can go do the validation during the second pass. Of course, that means that it was a bit late. It may be possible to build in some flags that say this directive can only occur at the top level. When the parser sees a directive with that flag set, and it isn't at the top level, then it would simply barf it up. [ I think the only at top level makes a lot more sense than the flags we have nowadays. the current flags are a confusing much of where a directive can or can't appear; the names are bad, the combinations aren't clear, and even the symbols aren't properly namespace (e.g. OR_LIMIT) ] Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: config handling (was: Re: cvs commit: httpd-2.0/server core.c)
On Mon, 20 May 2002, Greg Stein wrote: On Sat, May 18, 2002 at 12:32:20PM -0500, William A. Rowe, Jr. wrote: On Win32, we load-unload-reload the parent, then load-unload-reload the child config. Losing both redundant unload-load sequences will be a huge win at startup. Yup. If we process the tree in a much smarter fashion, then nothing should need to be unloaded. One thing I _like_ about the load-unload-reload is that it generally forces you (the module author) to consider the graceful restart case, rather than simply crashing (or getting buggy) the first time someone does it. [Sorry if you're using those terms in a technical fashion that I'm not following.] OTOH, on Windows the parent and child both have to load things, so you get a similar effect. [Speaking of this, one thing I'd like to see for Windows would be a way for the parent process to cache the config (or parse tree) and pass it directly to the child, so that you don't have the possibility of changing config when a new child is spawned due to MaxRequestsPerChild. Yeah, I _should_ submit a patch rather than a request.] Later, scott
Re: config handling (was: Re: cvs commit: httpd-2.0/server core.c)
At 07:31 PM 5/20/2002, Scott Hess wrote: On Mon, 20 May 2002, Greg Stein wrote: On Sat, May 18, 2002 at 12:32:20PM -0500, William A. Rowe, Jr. wrote: On Win32, we load-unload-reload the parent, then load-unload-reload the child config. Losing both redundant unload-load sequences will be a huge win at startup. Yup. If we process the tree in a much smarter fashion, then nothing should need to be unloaded. One thing I _like_ about the load-unload-reload is that it generally forces you (the module author) to consider the graceful restart case, rather than simply crashing (or getting buggy) the first time someone does it. [Sorry if you're using those terms in a technical fashion that I'm not following.] As much as I agree, a wise soul recently pointed out to me, offline, that if a module author doesn't test the graceful restart case, that's their foolishness, everyone shouldn't have to pay for it once [or twice] per startup :-) OTOH, on Windows the parent and child both have to load things, so you get a similar effect. Not really. It is a nasty effect, but entirely dissimilar :-) [Speaking of this, one thing I'd like to see for Windows would be a way for the parent process to cache the config (or parse tree) and pass it directly to the child, so that you don't have the possibility of changing config when a new child is spawned due to MaxRequestsPerChild. Yeah, I _should_ submit a patch rather than a request.] We tossed about the idea of passing all pool_userdata from conf pool to the child conf pool. Dunno if that's the right solution. But given all the new inheritance we deployed in the winnt mpm, it would be really nice to make that all accessible to mod_log_config, and it's cousins, and even more distant relatives such as SSL keys. Bill
Re: cvs commit: httpd-2.0/server core.c
On 18 May 2002 [EMAIL PROTECTED] wrote:
to work around problematic configs. So I'm reverting those changes,
for now.
Thanks.
ServerRoot is global only, and it MUST be read immediately, so that
part of the last patch stays.
Mmm, nope, sorry, but that one has to go, too.
--Cliff
(gdb) bt
#0 find_parent (dirp=0x0, what=0x8123918 Directory) at core.c:957
#1 0x080d5298 in ap_check_cmd_context (cmd=0xb660, forbidden=31)
at core.c:993
#2 0x080d6965 in set_server_root (cmd=0xb660, dummy=0xbfffd4b0,
arg=0x8197b60 /root/apache/test) at core.c:2014
#3 0x080c7faa in invoke_cmd (cmd=0x81246b4, parms=0xb660,
mconfig=0xbfffd4b0, args=0xbfffd52a ) at config.c:699
#4 0x080c8e73 in execute_now (cmd_line=0x8197b18 ServerRoot,
args=0xbfffd517 \/root/apache/test\, parms=0xb660, p=0x813c960,
ptemp=0x8190ab0, sub_tree=0xbfffd4b0, parent=0x0) at config.c:1310
#5 0x080c8732 in ap_build_config_sub (p=0x813c960, temp_pool=0x8190ab0,
l=0xbfffd50c ServerRoot \/root/apache/test\, parms=0xb660,
current=0xbfffd504, curr_parent=0xbfffd508, conftree=0x812f4f4)
at config.c:912
#6 0x080c8b42 in ap_build_config (parms=0xb660, p=0x813c960,
temp_pool=0x8190ab0, conftree=0x812f4f4) at config.c:1114
#7 0x080c926e in ap_process_resource_config (s=0x8143aa8,
fname=0x8194950 /root/apache/test/conf/httpd.conf, conftree=0x812f4f4,
p=0x813c960, ptemp=0x8190ab0) at config.c:1495
#8 0x080c9953 in ap_read_config (process=0x813a9d8, ptemp=0x8190ab0,
filename=0x8121a70 conf/httpd.conf, conftree=0x812f4f4) at config.c:1773
#9 0x080cb3b5 in main (argc=1, argv=0xb7b4) at main.c:552
#10 0x4033874f in __libc_start_main () from /lib/libc.so.6
(gdb) frame 2
#2 0x080d6965 in set_server_root (cmd=0xb660, dummy=0xbfffd4b0,
arg=0x8197b60 /root/apache/test) at core.c:2014
2014const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
(gdb) p *cmd
$1 = {info = 0x0, override = 150, limited = -1, limited_xmethods = 0x0,
xlimited = 0x0, config_file = 0x8197ad8, directive = 0x0, pool = 0x813c960,
temp_pool = 0x8190ab0, server = 0x8143aa8, path = 0x0, cmd = 0x81246b4,
context = 0x0, err_directive = 0x8197b28}
(gdb) p *cmd-cmd
$2 = {name = 0x812533d ServerRoot, func = {
no_args = 0x80d6950 set_server_root,
raw_args = 0x80d6950 set_server_root,
take1 = 0x80d6950 set_server_root, take2 = 0x80d6950 set_server_root,
take3 = 0x80d6950 set_server_root, flag = 0x80d6950 set_server_root},
cmd_data = 0x0, req_override = 384, args_how = TAKE1,
errmsg = 0x8125300 Common directory of server-related files (logs, confs, etc.)}
--
Cliff Woolley
[EMAIL PROTECTED]
Charlottesville, VA
Re: cvs commit: httpd-2.0/server core.c
wrowe 02/05/18 10:22:24 Modified:server core.c Log: Resolve the EXEC_ON_READ bit for ServerRoot and other modules that test the directive context. Should eliminate the segfault. Ok, segfault is gone. So is some validation that ServerRoot occured in a global context. Actually, that means that LoadModule and others are not tested to be in the global context either. We don't set up the vhost server till afterwards. Try setting ServerRoot within a vhost and it can't pick that up, can't warn us of a bad config. I'm just getting my head wrapped around cfg parsing, so anyone with a whole lot more background here might be able to provide pointers. Notice that we can't defer ServerRoot processing unless we defer LoadModule processing. If we REALLY get a better handle on this, I see no reason to maintain the test-load then re-load configuration logic. If we can pre-process what we must with EXEC_ON_READ, then open logs and the like, we should be able to directly proceed to run_mpm, without dumping it all and reloading. On Win32, we load-unload-reload the parent, then load-unload-reload the child config. Losing both redundant unload-load sequences will be a huge win at startup. [Caviat; we will always have unload-load server restart sequences for the parent process. But never, on win32 in the child process, a restart churns up a whole new child process, so the child will really win.] Bill
Re: cvs commit: httpd-2.0/server core.c
At 12:19 PM 5/18/2002, you wrote: On 18 May 2002 [EMAIL PROTECTED] wrote: to work around problematic configs. So I'm reverting those changes, for now. Thanks. ServerRoot is global only, and it MUST be read immediately, so that part of the last patch stays. Mmm, nope, sorry, but that one has to go, too. No, it has to stay. However, I think I have an adaquate solution in the last patch committed, so we work around the segfault now. Other issues remain... I detailed them in another post. Bill
Re: cvs commit: httpd-2.0/server core.c
On Sat, 18 May 2002, William A. Rowe, Jr. wrote: ServerRoot is global only, and it MUST be read immediately, so that part of the last patch stays. Mmm, nope, sorry, but that one has to go, too. No, it has to stay. However, I think I have an adaquate solution in the last patch committed, so we work around the segfault now. Other issues remain... I detailed them in another post. Ahhh... yep, that did it. And with the LoadModule thing, I understand why ServerRoot needs to be EXEC_ON_READ. We do clearly have those other issues to contend with as you mentioned, but it's progress. +1. --Cliff
Re: cvs commit: httpd-2.0/server core.c
On 17 May 2002 [EMAIL PROTECTED] wrote:
wrowe 02/05/17 12:34:53
Modified:server core.c
Log:
We need to grab ServerRoot, LogLevel, and ErrorLog right off the bat
as we are reading the config.
This closes a bug where ServerRoot /foo + LoadModule relative/mod_foo.so
won't load mod_foo.so.
Reported by: Ron Leung [EMAIL PROTECTED]
Umm, every one of these directives being EXEC_ON_READ causes the server to
segfault hard at startup. (I tried un-exec-on-reading them one at a time,
and it kept segfaulted on each one in turn until they were all gone.)
--Cliff
(gdb) bt
#0 find_parent (dirp=0x0, what=0x8123918 Directory) at core.c:957
#1 0x080d5298 in ap_check_cmd_context (cmd=0xb5a0, forbidden=30)
at core.c:993
#2 0x080d6cbe in set_loglevel (cmd=0xb5a0, dummy=0xbfffd3f0,
arg=0x819c5e0 warn) at core.c:2175
#3 0x080c7faa in invoke_cmd (cmd=0x812478c, parms=0xb5a0,
mconfig=0xbfffd3f0, args=0xbfffd459 ) at config.c:699
#4 0x080c8e73 in execute_now (cmd_line=0x819c5a8 LogLevel,
args=0xbfffd455 warn, parms=0xb5a0, p=0x813c960, ptemp=0x8190ab0,
sub_tree=0xbfffd3f0, parent=0x0) at config.c:1310
#5 0x080c8732 in ap_build_config_sub (p=0x813c960, temp_pool=0x8190ab0,
l=0xbfffd44c LogLevel warn, parms=0xb5a0, current=0xbfffd444,
curr_parent=0xbfffd448, conftree=0x812f4f4) at config.c:912
#6 0x080c8b42 in ap_build_config (parms=0xb5a0, p=0x813c960,
temp_pool=0x8190ab0, conftree=0x812f4f4) at config.c:1114
#7 0x080c926e in ap_process_resource_config (s=0x8143aa8,
fname=0x8194950 /root/apache/test/conf/httpd.conf, conftree=0x812f4f4,
p=0x813c960, ptemp=0x8190ab0) at config.c:1495
#8 0x080c9953 in ap_read_config (process=0x813a9d8, ptemp=0x8190ab0,
filename=0x8121a70 conf/httpd.conf, conftree=0x812f4f4) at config.c:1773
#9 0x080cb3b5 in main (argc=1, argv=0xb6f4) at main.c:552
#10 0x4033874f in __libc_start_main () from /lib/libc.so.6
(gdb) frame 1
#1 0x080d5298 in ap_check_cmd_context (cmd=0xb5a0, forbidden=30)
at core.c:993
993 if (((forbidden NOT_IN_DIRECTORY)
(gdb) list
988 return apr_pstrcat(cmd-pool, cmd-cmd-name, gt,
989 cannot occur within Directory/Location/Files
990section, NULL);
991 }
992
993 if (((forbidden NOT_IN_DIRECTORY)
994 ((found = find_parent(cmd-directive, Directory))
995 || (found = find_parent(cmd-directive, DirectoryMatch
996 || ((forbidden NOT_IN_LOCATION)
997 ((found = find_parent(cmd-directive, Location))
(gdb) p *cmd-err_directive
$2 = {directive = 0x819c5a8 LogLevel, args = 0x819c5d8 warn, next = 0x0,
first_child = 0x0, parent = 0x0, data = 0x0,
filename = 0x8197af0 /root/apache/test/conf/httpd.conf, line_num = 460}
--
Cliff Woolley
[EMAIL PROTECTED]
Charlottesville, VA
Re: cvs commit: httpd-2.0/server core.c protocol.c
Jeff Trawick wrote: +AP_DECLARE(void) ap_setup_make_content_type(apr_pool_t *pool); What's up with this? This has to go in a header file included by both core.c and protocol.c. It's fixed now (I moved the declaration to http_protocol.h and wrapped it in #ifdef CORE_PRIVATE) --Brian
Re: cvs commit: httpd-2.0/server core.c
Roy T. Fielding wrote:
I don't understand why you didn't simply reverse the test and
enclose the frequent case inside the if {} block.
D'oh! of course! commit coming shortly.
I assume it
was just to avoid indenting a large block of code, which is not
sufficient justification for a goto.
No, it just didn't occur to me at the time. I wouldn't want to use that
technique for all of the error cases though. Then the indentation would get out
of hand.
I've seen kernel level code that used a pattern like:
if (funky_error1) then {
goto exit_with_error1;
}
do important stuff;
if (funky_error2) then {
goto exit_with_error2;
}
do more stuff;
etc etc;
...which resulted in cache friendly code that this goto agnostic didn't have any
trouble reading. I guess that pattern imprinted itself on my brain.
Greg
Re: cvs commit: httpd-2.0/server core.c
I don't understand why you didn't simply reverse the test and
enclose the frequent case inside the if {} block. I assume it
was just to avoid indenting a large block of code, which is not
sufficient justification for a goto.
A goto often has unforeseen effects on high-level optimizations
that can be as bad as a pipeline flush.
Roy
Re: cvs commit: httpd-2.0/server core.c
Bill Stoddard wrote: Please lets not go overboard with these types of optimizations. This could get out of control quickly. Having code that is easy to read, understand and maintain is -much- more important than saving a a few extra cycles. In a previous job where I had a lot of low level hardware profiling information available, we found that on average, instructions which incurred cache misses took something like 26 times longer to execute than instructions which didn't miss. That difference is likely to grow as CPUs get faster relative to main memory. There is a fair amount of research and development in feedback-directed compilation techniques to automate such optimizations. For example, http://www.research.ibm.com/journal/sj/372/schmidt.html . But since this bothers people, I will move the infrequently executed instructions back into the hot path. Greg
Re: cvs commit: httpd-2.0/server core.c
Bill Stoddard wrote: Please lets not go overboard with these types of optimizations. This could get out of control quickly. Having code that is easy to read, understand and maintain is -much- more important than saving a a few extra cycles. In a previous job where I had a lot of low level hardware profiling information available, we found that on average, instructions which incurred cache misses took something like 26 times longer to execute than instructions which didn't miss. That difference is likely to grow as CPUs get faster relative to main memory. There is a fair amount of research and development in feedback-directed compilation techniques to automate such optimizations. For example, http://www.research.ibm.com/journal/sj/372/schmidt.html . But since this bothers people, I will move the infrequently executed instructions back into the hot path. Greg I am not against restructuring the code to be more friendly to instruction caches and I am not dogmatically opposed to judicious use of goto, however... The use of goto in this case exceeds my pain threshold by a substantial margin. This approach taken to its logical conclusion would yield unsupportable code. Bill
Re: cvs commit: httpd-2.0/server core.c
Please lets not go overboard with these types of optimizations. This could get out of
control quickly. Having code that is easy to read, understand and maintain is -much-
more
important than saving a a few extra cycles.
Bill
- Original Message -
From: [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Sent: Wednesday, April 24, 2002 10:31 AM
Subject: cvs commit: httpd-2.0/server core.c
gregames02/04/24 07:31:28
Modified:server core.c
Log:
default_handler: short circuit the method checks. Move the code to deal
with unusual methods to the end of function to reduce i-cache clutter.
Revision ChangesPath
1.171 +13 -11httpd-2.0/server/core.c
Index: core.c
===
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.170
retrieving revision 1.171
diff -u -r1.170 -r1.171
--- core.c 22 Apr 2002 08:08:38 - 1.170
+++ core.c 24 Apr 2002 14:31:28 - 1.171
@@ -3167,18 +3167,8 @@
return errstatus;
}
-if (r-method_number == M_INVALID) {
-ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
- Invalid method in request %s, r-the_request);
-return HTTP_NOT_IMPLEMENTED;
-}
-
-if (r-method_number == M_OPTIONS) {
-return ap_send_http_options(r);
-}
-
if (r-method_number != M_GET r-method_number != M_POST) {
-return HTTP_METHOD_NOT_ALLOWED;
+goto unusual_method;
}
if (r-finfo.filetype == 0) {
@@ -3248,6 +3238,18 @@
APR_BRIGADE_INSERT_TAIL(bb, e);
return ap_pass_brigade(r-output_filters, bb);
+
+unusual_method:
+if (r-method_number == M_INVALID) {
+ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
+ Invalid method in request %s, r-the_request);
+return HTTP_NOT_IMPLEMENTED;
+}
+
+if (r-method_number == M_OPTIONS) {
+return ap_send_http_options(r);
+}
+return HTTP_METHOD_NOT_ALLOWED;
}
static int net_time_filter(ap_filter_t *f, apr_bucket_brigade *b,
Re: cvs commit: httpd-2.0/server core.c
On Wed, Apr 24, 2002 at 02:31:29PM -, [EMAIL PROTECTED] wrote:
gregames02/04/24 07:31:28
Modified:server core.c
Log:
default_handler: short circuit the method checks. Move the code to deal
with unusual methods to the end of function to reduce i-cache clutter.
Can't we live without the goto? Move those statements inside
if (r-method_number != M_GET r-method_number != M_POST) {
check? goto statement considered harmful.
You'd make the common case faster, but please let's not worry
about the instruction cache. -- justin
Revision ChangesPath
1.171 +13 -11httpd-2.0/server/core.c
Index: core.c
===
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.170
retrieving revision 1.171
diff -u -r1.170 -r1.171
--- core.c 22 Apr 2002 08:08:38 - 1.170
+++ core.c 24 Apr 2002 14:31:28 - 1.171
@@ -3167,18 +3167,8 @@
return errstatus;
}
-if (r-method_number == M_INVALID) {
-ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
- Invalid method in request %s, r-the_request);
-return HTTP_NOT_IMPLEMENTED;
-}
-
-if (r-method_number == M_OPTIONS) {
-return ap_send_http_options(r);
-}
-
if (r-method_number != M_GET r-method_number != M_POST) {
-return HTTP_METHOD_NOT_ALLOWED;
+goto unusual_method;
}
if (r-finfo.filetype == 0) {
@@ -3248,6 +3238,18 @@
APR_BRIGADE_INSERT_TAIL(bb, e);
return ap_pass_brigade(r-output_filters, bb);
+
+unusual_method:
+if (r-method_number == M_INVALID) {
+ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
+ Invalid method in request %s, r-the_request);
+return HTTP_NOT_IMPLEMENTED;
+}
+
+if (r-method_number == M_OPTIONS) {
+return ap_send_http_options(r);
+}
+return HTTP_METHOD_NOT_ALLOWED;
}
static int net_time_filter(ap_filter_t *f, apr_bucket_brigade *b,
Re: cvs commit: httpd-2.0/server core.c
Greg,
Please do not mix function change with code style changes.
Bill
gregames02/03/05 07:46:21
Modified:server core.c
Log:
fix Directory ~ blah containers.
also, eradicate a few nefarious tabs which were found lurking in the vicinity.
Submitted by: Rob Simonson [EMAIL PROTECTED]
Reviewed by: Greg Ames
Revision ChangesPath
1.157 +4 -3 httpd-2.0/server/core.c
Index: core.c
===
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.156
retrieving revision 1.157
diff -u -r1.156 -r1.157
--- core.c 5 Mar 2002 05:24:21 - 1.156
+++ core.c 5 Mar 2002 15:46:21 - 1.157
@@ -1403,7 +1403,7 @@
}
AP_CORE_DECLARE_NONSTD(const char *) ap_limit_section(cmd_parms *cmd, void *dummy,
- const char *arg) {
+ const char *arg) {
const char *limited_methods = ap_getword(cmd-pool, arg, '');
void *tog = cmd-cmd-cmd_data;
apr_int64_t limited = 0;
@@ -1500,12 +1500,13 @@
cmd-override = OR_ALL|ACCESS_CONF;
if (!strcmp(cmd-path, ~)) {
- cmd-path = ap_getword_conf(cmd-pool, arg);
+cmd-path = ap_getword_conf(cmd-pool, arg);
if (!cmd-path)
return Directory ~ block must specify a path;
+r = ap_pregcomp(cmd-pool, cmd-path, REG_EXTENDED|USE_ICASE);
}
else if (thiscmd-cmd_data) { /* DirectoryMatch */
- r = ap_pregcomp(cmd-pool, cmd-path, REG_EXTENDED|USE_ICASE);
+r = ap_pregcomp(cmd-pool, cmd-path, REG_EXTENDED|USE_ICASE);
}
else if (!strcmp(cmd-path, /) == 0)
{
Re: the three filter types (was: cvs commit: httpd-2.0/server core.c)
On Mon, Mar 04, 2002 at 08:35:35PM -0800, Ryan Bloom wrote: On Sun, Mar 03, 2002 at 10:34:55PM -, [EMAIL PROTECTED] wrote: ... Just wanted to state that I believe Ryan has the correct distinction here -- that we have three types of filters. However, the third type is a misnomer, and we can derive the proper name from the HTTP model: resource YES, resource is markedly better than CONTENT. I have been using resource all day when explaining this to people at work. Always scary when Greg and I agree. :-) Oh, for christssakes... put up some kind of argument... ;-) -- Greg Stein, http://www.lyra.org/
Filter Breakage is Re: cvs commit: httpd-2.0/server core.c
On Mon, Mar 04, 2002 at 09:20:03AM -, [EMAIL PROTECTED] wrote:
jerenkrantz02/03/04 01:20:03
Modified:server core.c
Log:
Ensure that net_time filter isn't added on subreqs - we assume that it is
added on !r-main requests. This led to infinite loop/SEGV when dealing
with anything that created a subreq.
(I don't think core_create_req is a good place for adding this filter.)
This fixes one of my problems.
I now have two further problems.
1) Since fast_redirect() is called from a hook, but all hooks are run
on this new request - it is possible that some hooks are left to
be run from the original request. This causes a problem for the
AddOutputFilterByType hook (core_filters_type) since it is a fixup
as well and is run after the mod_dir/mod_negotiation hooks. Therefore,
when using this directive, it adds each filter twice causing segfaults
and infinite loops. Oops.
We could play with hook ordering, but I'm pretty sure that's the
wrong thing to be doing. Of course, I'm advocating removing
fast_redirect for precisely this problem.
2) This code still can't handle filtering order properly across
sub-requests. There are two problems with this approach. The
first is that the lists aren't distinct and are corrupted as we
generate the subreqs. The second problem is that I believe it is
possible for a configuration to add a protocol-level filter (say,
mod_headers or mod_deflate), and this new semantic does not
support that properly (probably due to the assumption that
protocol filters can't be inserted in a subreq).
Imagine the following hypothetical configuration:
Alias /manual /foo/manual
Directory /foo/manual
Options Indexes MultiViews
AddOutputFilter INCLUDES .html
/Directory
Let's request /manual/ from our server. What happens?
First off we create subreqs for index.html. That triggers the
mod_negotiation reqs. When mod_negotiation runs, it'll actually
create multiple subreqs that are *valid* - (i.e. index.html.en,
index.html.fr, index.html.de, etc, etc.). Therefore, the hooks
will be run on *each* of these. It'll then pick one of these
subreqs to be our request and call fast_redirect() on it. And,
when mod_negotiation is done, mod_dir will cast fast_redirect()
once more (to unravel our request paths). (Hopefully, by now
Ryan sees where this is headed.)
mod_mime will dutifully add INCLUDES to each of the subreqs as
mod_negotiation runs each subreq. Aha! But, what happens to
proto_output_filters? It's prev will be modified to point at
this subreq's INCLUDES filter. But, let's say we take the 3rd
subreq (i.e. not the last subreq we ran). So, we do our
fast_redirect() magic and we promote r-output_filters from the
*third* subreq. proto_output_filter's prev is pointing at the
*last* subreq's INCLUDES filter. Uh, oh. output_filters chain
is corrupted. Witness this:
(gdb) print r-output_filters
$122 = (struct ap_filter_t *) 0x81a8538
(gdb) print r-output_filters-next
$123 = (ap_filter_t *) 0x8166aa8
(gdb) print r-output_filters-next-prev
$124 = (ap_filter_t *) 0x81a6560
Oops. So, when we go run ap_run_insert_filter hook (to add
the protocol-handling filters), we try to add byterange filter,
but we get screwed and we miss the following code at
util_filter.c (line 356):
if (*outf ((*outf)-prev == first)) {
(*outf)-prev = f;
}
So, the previous isn't updated. No big deal, you say? Let's
consider the second filter we add in ap_run_insert_filter -
content-length. When, it comes in, our check to INSERT_BEFORE
returns false, so we'll insert it afterwards. Because of our
bad prev pointers, this code (line 368) never gets called:
if (fscan-next-prev == fscan) {
f-prev = fscan;
fscan-next-prev = f;
fscan-next = f;
}
That should be adding the next pointer such that fscan-next points
at our newly created filter. Since that call fails, only the
first filter will be inserted as we go forward in the list (and
it is indeterminate as you would traverse backwards in the list).
That's one problem. The solution here would be to make the
output_filters chain for *each* subreq to be distinct. But, that
requires copying the lists on each subreq such that the filter
chains are distinct.
Currently, we end up with:
Includes-Byterange-Core
with HTTP and content-length being hidden in the prev of Core. So,
mod_dir/mod_negotiation requests with AddOutputFilter[ByType] are
still broken.
The second half of the filtering problem is that we don't promote
r-proto_output_filters when doing a fast_redirect. What if we
changed our example directive to be:
AddOutputFilter DEFLATE .html
It'd get added to the r-output_filters of the subreq and the
r-proto_output_filters would be correct as well for this
subreq. However, when we do the promotion in fast_redirect(),
only r-output_filters is used. So, r-output_filters is
DEFLATE-CORE. But, r-proto_output_filters after fast_redirect
is called is: CORE *not* DEFLATE-CORE since we don't update
RE: Filter Breakage is Re: cvs commit: httpd-2.0/server core.c
I now have two further problems 1) Since fast_redirect() is called from a hook, but all hooks are run on this new request - it is possible that some hooks are left to be run from the original request This causes a problem for the AddOutputFilterByType hook (core_filters_type) since it is a fixup as well and is run after the mod_dir/mod_negotiation hooks Therefore, when using this directive, it adds each filter twice causing segfaults and infinite loops Oops We could play with hook ordering, but I'm pretty sure that's the wrong thing to be doing Of course, I'm advocating removing fast_redirect for precisely this problem I've got the problem that I just don't see that behavior I have add an AddOutputFilterByType directive to my htdocs directory, and I requested GET / HTTP/10, and I received my page back successfully I need a config that will reliably reproduce before I can fix this 2) This code still can't handle filtering order properly across sub-requests There are two problems with this approach The first is that the lists aren't distinct and are corrupted as we generate the subreqs The filter chains must be corrupted as we generate subrequests The problem is that we have a single chain, but we need to hook into the middle of the chain as we go This has always been a problem with the filter chain, but it wasn't as obvious when we had a singly linked list I have chosen to make the prev pointer always stay on the same chain, so that a sub-request is literally inserting itself into the filter chain I haven't had time to test your config that is causing you problems, but I will as soon as I get to the office The second problem is that I believe it is possible for a configuration to add a protocol-level filter (say, mod_headers or mod_deflate), and this new semantic does not support that properly (probably due to the assumption that protocol filters can't be inserted in a subreq) Neither mod_headers or mod_deflate should be protocol level filters They both operate specifically on individual resources, and should be resource level filters Change that and this problem will go away Hope this provides some explanations as to the problems I'm seeing Again, I'm not at all sold on this proto_output_filters trick as I don't think it will solve our problem However, I'm willing to wait for Ryan to fix it because he seems pretty sure that this will work, but I'm definitely concerned about the extra complexity we're introducing -- justin The problem is that we are removing complexity, not adding it The problems that we have seen so far have all been to a number of hacks that we have added to get error pages and everything else correct In EVERY case so far, we have solved a seg fault or loop by not adding a filter in a case where it didn't make sense to add it anyway Also, and please be incredibly clear about this The bugs that you are seeing now, have NOTHING what so ever to do with the protocol filter concept The bugs that you are seeing now are 100% related to trying to fix the problem of losing filters when they are added at the wrong stage If you don't believe me, go back to Saturday night's tree before my last commit that night That had the protocol filters, and they worked correctly Ryan
Re: Filter Breakage is Re: cvs commit: httpd-2.0/server core.c
On Mon, Mar 04, 2002 at 07:07:03AM -0800, Ryan Bloom wrote: I now have two further problems 1) Since fast_redirect() is called from a hook, but all hooks are run on this new request - it is possible that some hooks are left to be run from the original request This causes a problem for the AddOutputFilterByType hook (core_filters_type) since it is a fixup as well and is run after the mod_dir/mod_negotiation hooks Therefore, when using this directive, it adds each filter twice causing segfaults and infinite loops Oops We could play with hook ordering, but I'm pretty sure that's the wrong thing to be doing Of course, I'm advocating removing fast_redirect for precisely this problem I've got the problem that I just don't see that behavior I have add an AddOutputFilterByType directive to my htdocs directory, and I requested GET / HTTP/10, and I received my page back successfully I need a config that will reliably reproduce before I can fix this I think you need two levels of subreqs I was using the manual/ wich causes mod_dir-mod_negotiation In fact, I believe just a MultiView will cause this (no need for mod_dir) So, /manual/indexhtml should trigger this The filter chains must be corrupted as we generate subrequests The problem is that we have a single chain, but we need to hook into the middle of the chain as we go This has always been a problem with the filter chain, but it wasn't as obvious when we had a singly linked list I have chosen to make the prev pointer always stay on the same chain, so that a sub-request is literally inserting itself into the filter chain I haven't had time to test your config that is causing you problems, but I will as soon as I get to the office Were you able to reproduce it? As I indicated in a private email earlier today, the following config is broken: Alias /manual /pkg/apache-20-cvs/manual Directory /pkg/apache-20-cvs/manual Options Indexes FollowSymLinks MultiViews AllowOverride None Order allow,deny Allow from all AddOutputFilter DEFLATE html /Directory (This config showcases all of the bugs I've seen) If you have the right patches (hinted at by my earlier message), AddOutputFilterByType DEFLATE text/html would work, but that's corrupted since the hooks are run twice Neither mod_headers or mod_deflate should be protocol level filters They both operate specifically on individual resources, and should be resource level filters Change that and this problem will go away Let's say I come up with some protocol-level filter that has a legitimate use and I can't add it via AddOutputFilter Bogus You're ignoring the problem by restricting what filters we can have Bad idea, IMHO The problem is that we are removing complexity, not adding it The problems that we have seen so far have all been to a number of hacks that we have added to get error pages and everything else correct In EVERY case so far, we have solved a seg fault or loop by not adding a filter in a case where it didn't make sense to add it anyway Also, and please be incredibly clear about this The bugs that you are seeing now, have NOTHING what so ever to do with the protocol filter concept The bugs that you are seeing now are 100% related to trying to fix the problem of losing filters when they are added at the wrong stage If you don't believe me, go back to Saturday night's tree before my last commit that night That had the protocol filters, and they worked correctly Nope, I believe the protocol filters are essentially a no-op They aren't doing anything because things are only added to r-proto_out_filters when we run ap_run_insert_filters which is after subreqs I'm not seeing any benefit by having proto filters - they just seem to serve to complicate things -- justin
the three filter types (was: cvs commit: httpd-2.0/server core.c)
On Sun, Mar 03, 2002 at 10:34:55PM -, [EMAIL PROTECTED] wrote: rbb 02/03/03 14:34:55 Modified:modules/http http_core.c server core.c Log: Classify some of the input filters as the correct types. Previous to this patch, the type wasn't too important, because all filters were put on the same list. After this patch, the filter type is very important, because there are three different types of filters, and they are all treated differently, namely: CONNECTION: Filters of this type are valid for the lifetime of this connection. PROTOCOL: Filters of this type are valid for the lifetime of this request from the point of view of the client, this means that the request is valid from the time that the request is sent until the time that the response is received. CONTENT:Filters of this type are valid for the time that this content is used to satisfy a request. For simple requests, this is identical to PROTOCOL, but internal redirects and sub-requests can change the content without ending the request. Just wanted to state that I believe Ryan has the correct distinction here -- that we have three types of filters. However, the third type is a misnomer, and we can derive the proper name from the HTTP model: resource In HTTP, you talk to a resource on the server. With our redirect stuff, all we're doing is pointing to a different resource. The connection and protocol remain the same. IMO, Apache should lose the concept of a request_rec except for way down in the MPM levels. Only the MPM deals with requests. Instead, we move to a model where resources are first-class objects and the server talks to them in interesting ways. We map incoming URLs to resource objects, and let it fly. The default resource would represent a file in the filesystem, but it could also be a CGI script, a file in a Subversion repository, or the source for a PHP script grabbed via a proxy-like socket connection. In Apache 2.1, I'd like to move towards the resource model, and also create a resource mapping tree. Rather than this whole location / directory walk crap, we navigate down a tree. Each node identifies a different provider of resources. The root node (/) refers to the filesystem provider, keyed in at DocumentRoot. The Alias directive creates new nodes in the location tree, also referring to the filesystem provider, but they point to a different space in the filesystem. etc etc. Much of this resource model has already been fleshed out in mod_dav's dav_resource structure. That resource model is how mod_dav can perform its operations against arbitrary backend storage mechanisms. It seems perfectly valid to promote to a more integral concept of Apache itself. That would allow things like mod_autoindex to ask a resource for a list of child resources, and to build a nicely formatted page to navigate to them (today, mod_autoindex is hard-wired to the filesystem, which means we cannot apply its mechanisms to collections of resources in a Subversion repository). Cheers, -g -- Greg Stein, http://www.lyra.org/
RE: the three filter types (was: cvs commit: httpd-2.0/server core.c)
On Sun, Mar 03, 2002 at 10:34:55PM -, [EMAIL PROTECTED] wrote: rbb 02/03/03 14:34:55 Modified:modules/http http_core.c server core.c Log: Classify some of the input filters as the correct types. Previous to this patch, the type wasn't too important, because all filters were put on the same list. After this patch, the filter type is very important, because there are three different types of filters, and they are all treated differently, namely: CONNECTION: Filters of this type are valid for the lifetime of this connection. PROTOCOL: Filters of this type are valid for the lifetime of this request from the point of view of the client, this means that the request is valid from the time that the request is sent until the time that the response is received. CONTENT: Filters of this type are valid for the time that this content is used to satisfy a request. For simple requests, this is identical to PROTOCOL, but internal redirects and sub-requests can change the content without ending the request. Just wanted to state that I believe Ryan has the correct distinction here -- that we have three types of filters. However, the third type is a misnomer, and we can derive the proper name from the HTTP model: resource YES, resource is markedly better than CONTENT. I have been using resource all day when explaining this to people at work. Always scary when Greg and I agree. :-) Ryan
Re: the three filter types (was: cvs commit: httpd-2.0/server core.c)
At 07:53 PM 3/4/2002, you wrote: Just wanted to state that I believe Ryan has the correct distinction here -- that we have three types of filters However, the third type is a misnomer, and we can derive the proper name from the HTTP model: resource Resource, body, content, whichever The trick is that any 'resource' can be aggregated, while the connection filters are pretty fixed [or mutate in stable ways, such as connection:upgrade semantics], protocol filters are pretty fixed [they are designed by the top resource], but we can munge together a bunch of 'resource' streams into one 'request' [as once defined in 13 :-] In HTTP, you talk to a resource on the server With our redirect stuff, all we're doing is pointing to a different resource The connection and protocol remain the same Bingo, for the most part Absolutely on when it comes to merging several resources [sub-requests] Only thing is that which resource will have some thing to do with the protocol (want the gz flavor? here it is) In Apache 21, I'd like to move towards the resource model, and also create a resource mapping tree Rather than this whole location / directory walk crap, we navigate down a tree Each node identifies a different provider of resources The root node (/) refers to the filesystem provider, keyed in at DocumentRoot The Alias directive creates new nodes in the location tree, also referring to the filesystem provider, but they point to a different space in the filesystem etc etc You sort of describe what happened with map_to_storage directory only applies to surprise files :) I don't know if we can rip away Location altogether While it would be nice, the location concept gives us some high-level abstractions, such as auth contexts, etc, that are still relevant/useful But I would like to see us continue to 'discourage' their use in so far as users 'presumed' the two work the same URI space != file mounts :) Much of this resource model has already been fleshed out in mod_dav's dav_resource structure I need to study this, but I agree that replacing the filesystem altogether with a dav-like provider model is the only way to move forward But not for release 20 :) Let's get planning for 21 - ROADMAP is calling Good thoughts Greg; Bill
Re: cvs commit: httpd-2.0/server core.c
Ryan Bloom wrote: I don't care how de-stabilized the code base becomes. That's a very alarming thing to read. Making perchild work is one thing. Warping, or even just tweaking, common code in order to make perchild work is something else again. -- #kenP-)} Ken Coar, Sanagendamgagwedweinini http://Golux.Com/coar/ Author, developer, opinionist http://Apache-Server.Com/ Millennium hand and shrimp!
RE: cvs commit: httpd-2.0/server core.c
Ryan Bloom wrote: I don't care how de-stabilized the code base becomes. That's a very alarming thing to read. Making perchild work is one thing. Warping, or even just tweaking, common code in order to make perchild work is something else again. If you read the statement in context, it shouldn't be alarming. It was in response to a statement that we shouldn't be re-writing any code, because it destabilizes the current code. I patently disagree with that statement. I also disagree that just tweaking common code should be a problem. Perhaps most importantly though, I don't think the changes for perchild will extend beyond the MPM. Ryan
Re: cvs commit: httpd-2.0/server core.c
Ryan Bloom wrote: I also disagree that just tweaking common code should be a problem. Perhaps most importantly though, I don't think the changes for perchild will extend beyond the MPM. That's cool, then. However, it *does* seem that recently (last few months) there tends to be a lot of collateral impact from seemingly innocuous core changes, so I urge caution. Unlike others here, I'm not emotionally invested in getting 2.0 out soon. If it takes a few more months for it to be Ready, that's fine with me. I think there are some interested parties that would rather have it released sooner than later, though, and I would really rather release something 'good' than something that's just 'good enough'. -- #kenP-)} Ken Coar, Sanagendamgagwedweinini http://Golux.Com/coar/ Author, developer, opinionist http://Apache-Server.Com/ Millennium hand and shrimp!
Re: cvs commit: httpd-2.0/server core.c
On Tue, 26 Feb 2002, Rodent of Unusual Size wrote: | Unlike others here, I'm not emotionally invested in getting 2.0 | out soon. If it takes a few more months for it to be Ready, | that's fine with me. I think there are some interested parties | that would rather have it released sooner than later, though, | and I would really rather release something 'good' than something | that's just 'good enough'. Oh, definitely. There's no reason to go half-way, when (esp. in the case of this) one should go as far as one can to release something that has very high quality associated with it. Afterall, this is the most widely used web server on the internet. I'm not saying ap 2.0 has to be out next week, or even next month for that matter. There just seems to be a lack of foresight here. The way things are now, a few more months will turn into a few more months after that, etc. etc. Apache 2.0 is entering into it's third year of on-going development, and lots of good code and ideas have been put into it. But the longer it goes on without any enforced release management or goals, the more it looks like vaporware to the masses out there who arent in the know. It's not a question of 'where's the beef?. It's more like does anyone have a clue as to when the beef will be done? /dale
Re: cvs commit: httpd-2.0/server core.c
Dale Ghent wrote: On Tue, 26 Feb 2002, Rodent of Unusual Size wrote: | Unlike others here, I'm not emotionally invested in getting 2.0 | out soon. If it takes a few more months for it to be Ready, | that's fine with me. I think there are some interested parties | that would rather have it released sooner than later, though, | and I would really rather release something 'good' than something | that's just 'good enough'. Oh, definitely. There's no reason to go half-way, when (esp. in the case of this) one should go as far as one can to release something that has very high quality associated with it. Afterall, this is the most widely used web server on the internet. I'm not saying ap 2.0 has to be out next week, or even next month for that matter. There just seems to be a lack of foresight here. The way things are now, a few more months will turn into a few more months after that, etc. etc. Apache 2.0 is entering into it's third year of on-going development, and lots of good code and ideas have been put into it. But the longer it goes on without any enforced release management or goals, the more it looks like vaporware to the masses out there who arent in the know. It's not a question of 'where's the beef?. It's more like does anyone have a clue as to when the beef will be done? But we do know when it will be done. There's a list of items under the Release Showstoppers that define what needs to happen before we can declare a 2.0.x version Generally Available. Fortunately, this list is short. It doesn't contain any new features, which is a good thing. It contains one performance fix and half a dozen important functional bugs. --Brian
RE: cvs commit: httpd-2.0/server core.c
RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released Huh!?!?!?!? You can't veto fixing code that doesn't work. Even more, you can't veto work that somebody wants to do. The whole point of Open Source is that we all get to do what is most interesting to us. I am fairly confidant in saying that I want to see a GA release more than anybody else here, except for possibly Bill Stoddard, because the two of us have been working on 2.0 for longer than anybody else on this list. However, this push for GA regardless of quality or how fun the project is really makes me wish that we weren't nearly so close to one. :-( Ryan
RE: cvs commit: httpd-2.0/server core.c
RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released Huh!?!?!?!? You can't veto fixing code that doesn't work. Even more, you can't veto work that somebody wants to do. The whole point of Open Source is that we all get to do what is most interesting to us. I am fairly confidant in saying that I want to see a GA release more than anybody else here, except for possibly Bill Stoddard, because the two of us have been working on 2.0 for longer than anybody else on this list. However, this push for GA regardless of quality or how fun the project is really makes me wish that we weren't nearly so close to one. :-( BTW, the use of vetos on this list is completely against the way things should be. We use vetos now as an offhanded way of saying that we don't like something, which isn't how they are meant to be used. A veto is basically telling somebody that their approach to a solution is wrong, and that it will break other code, or not scale, or that you have a better way of doing it. They should be rare, and they should only EVER happen with a valid technical justification. I think we have had ten vetos in the last few weeks, which is a lot more than we should have had. I know that there are people on the list who will laugh that I am the person saying this, as I used to use vetos a lot, and sometimes in the wrong places. Chalk that up to being young, inexperienced, and cocky. Vetos are annoying, and they stop the code from progressing forward as it should, please, let's go back to using them sparingly, and only when there is code that is egregiously wrong. Ryan
RE: cvs commit: httpd-2.0/server core.c
RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released Huh!?!?!?!? You can't veto fixing code that doesn't work. If you were just proposing a fix, I'd have no objections (well, until I saw the code :-) The crucial difference is that you're talking about a rewrite of a large amount of code. We've seen how badly the code base becomes destabilized after major rewrites. I don't care how de-stabilized the code base becomes. We have an MPM that absolutely does not work right now. I have committed a note explaining that I finally figured out how to solve the bug, and you tried to veto the whole concept of fixing the module. Even more, I am most likely to ONLY touch the MPM or fix bugs that exist in the core that stop the MPM from working. How can you even think of veto'ing that? Even more, you can't veto work that somebody wants to do. The whole point of Open Source is that we all get to do what is most interesting to us. I am fairly confidant in saying that I want to see a GA release more than anybody else here, except for possibly Bill Stoddard, because the two of us have been working on 2.0 for longer than anybody else on this list. However, this push for GA regardless of quality or how fun the project is really makes me wish that we weren't nearly so close to one. With regard to quality, that's exactly why we need to stop doing gratuitious rewrites of large subsystems. While it's not impossible to make such changes work, the code quality has dropped with each major change--and it's taken a lot of work to fix all the peripheral things that got broken as side-effects of rewrites. This isn't gratuitous. The MPM doesn't work, period. If it worked, it wouldn't be an issue. As for side-effects of rewrites, they happen. If the code is easier to maintain moving forward as a result of the rewrite and the code is better because of the rewrite, then the rewrite was worthwhile, and the side-effects are a small price to pay. As for how fun the project is, I'm in agreement that that's something we all consider when deciding what code to write, but it's usually not a criterion that I apply when evaluating commits. The reason is that a big code rewrite can yield O(1) incremental fun but O(n) incremental work: it's fun for the person who writes the code, but a lot of extra work for n other people who end up debugging it once it's in the code base. This is really simple. If there is a bug, it needs to get fixed. If a developer says they want to fix something, then they should be allowed to. The whole idea of telling somebody that they can't do something is completely borked. My main complaint with this though, is that we shouldn't be throwing out vetos left and right. We should be having conversations and coming to resolutions without vetos. Vetos should be rare, and only used in extreme situations. Seeing at least one veto every week is part of what makes this less fun. Ryan
Re: cvs commit: httpd-2.0/server core.c
On Sun, Feb 24, 2002 at 11:29:30PM -0800, Brian Pane wrote: [EMAIL PROTECTED] wrote: RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released Hmm...we've already decided that we won't let perchild hold up a GA no matter how broken it is, so the way I see it we're only obligated to make sure our users know it is experimental and incomplete. Progress should never be hindered by a release. Stability is a different story, though... -aaron
Re: cvs commit: httpd-2.0/server core.c
On Sun, 24 Feb 2002, Brian Pane wrote: [EMAIL PROTECTED] wrote: RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released I would agree with Brian here. We should get 2.0 out and punt on per-child for now. Let's get this baby stable and performant and out to the public. Cheers, -- Sander van Zoest [EMAIL PROTECTED] San Diego, CA, US http://Sander.vanZoest.com/
Re: cvs commit: httpd-2.0/server core.c
Ryan Bloom wrote: RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released Huh!?!?!?!? You can't veto fixing code that doesn't work. If you were just proposing a fix, I'd have no objections (well, until I saw the code :-) The crucial difference is that you're talking about a rewrite of a large amount of code. We've seen how badly the code base becomes destabilized after major rewrites. I don't care how de-stabilized the code base becomes. We have an MPM that absolutely does not work right now. I have committed a note explaining that I finally figured out how to solve the bug, and you tried to veto the whole concept of fixing the module. No, I vetoed putting a rewrite of the MPM in the critical path for GA. I have no objection to fixing the bug with a minimal incremental fix now, or rewriting it post-GA, or declaring perchild experimental-use-only (and thus not critical path for GA) and then doing a rewrite at any time. --Brian
Re: cvs commit: httpd-2.0/server core.c
I agree that documentation should cover this, but it should also be displayed in the ./configure if someone chooses that, to say, display a warning at the end of the configure or some such thing. At least there'd be an I told you so in there. On Mon, 2002-02-25 at 19:31, Joshua Slive wrote: On Mon, 25 Feb 2002, Aaron Bannert wrote: On Mon, Feb 25, 2002 at 04:58:50PM -0800, Brian Pane wrote: ...or declaring perchild experimental-use-only (and thus not critical path for GA) and then doing a rewrite at any time. When we voted on whether perchild was a showstopper or not I considered that to be equivalent to perchild is experimental, use at your own risk. The real question is: How do we make it very obvious that perchild is experimental? A big red WARNING somewhere? Perhaps some code in configure.in to detect --with-mpm=perchild? That's what documentation is for. If people are going to switch to non-default mpms without reading the documentation, they get what is coming to them. And by the way, +1 on fixing perchild now. I think Ryan should feel free to do anything he wants to improve the mpm, as long as it doesn't involve major mucking with non-mpm code. The only thing that could break is perchild, and perchild is broken anyway. Joshua. -- Austin Gonyou Systems Architect, CCNA Coremetrics, Inc. Phone: 512-698-7250 email: [EMAIL PROTECTED] It is the part of a good shepherd to shear his flock, not to skin it. Latin Proverb
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] wrote: RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: + +* With AP_MODE_EXHAUSTIVE in the core, it is finally clear to me + how the Perchild MPM should be re-written. It hasn't worked + correctly since filters were added because it wasn't possible to + get the content that had already been written and the socket at + the same time. This mode lets us do that, so the MPM can be + fixed. -1 on an MPM rewrite until after 2.0 GA is released --Brian
Re: cvs commit: httpd-2.0/server core.c
On 21 Feb 2002 [EMAIL PROTECTED] wrote:
e = apr_bucket_file_create(fd, 0, AP_MAX_SENDFILE, r-pool);
while (fsize AP_MAX_SENDFILE) {
-APR_BRIGADE_INSERT_TAIL(bb, e);
-apr_bucket_copy(e, e);
+apr_bucket *ce;
+apr_bucket_copy(e, ce);
+APR_BRIGADE_INSERT_TAIL(bb, ce);
e-start += AP_MAX_SENDFILE;
fsize -= AP_MAX_SENDFILE;
}
At first I thought this was giving a duplicate copy of the first bucket,
but then I realized that by moving the INSERT call down, you've avoided
that problem. However, aren't we leaking e?
--Cliff
--
Cliff Woolley
[EMAIL PROTECTED]
Charlottesville, VA
Re: cvs commit: httpd-2.0/server core.c
From: Cliff Woolley [EMAIL PROTECTED]
Sent: Thursday, February 21, 2002 12:28 PM
On 21 Feb 2002 [EMAIL PROTECTED] wrote:
e = apr_bucket_file_create(fd, 0, AP_MAX_SENDFILE, r-pool);
while (fsize AP_MAX_SENDFILE) {
-APR_BRIGADE_INSERT_TAIL(bb, e);
-apr_bucket_copy(e, e);
+apr_bucket *ce;
+apr_bucket_copy(e, ce);
+APR_BRIGADE_INSERT_TAIL(bb, ce);
e-start += AP_MAX_SENDFILE;
fsize -= AP_MAX_SENDFILE;
}
At first I thought this was giving a duplicate copy of the first bucket,
but then I realized that by moving the INSERT call down, you've avoided
that problem. However, aren't we leaking e?
No, because e is then is then inserted at the tail after the while {} loop.
Re: cvs commit: httpd-2.0/server core.c
On Thu, 21 Feb 2002, Cliff Woolley wrote: At first I thought this was giving a duplicate copy of the first bucket, but then I realized that by moving the INSERT call down, you've avoided that problem. However, aren't we leaking e? Oh, I see it. Nevermind. Sometimes I wish diff -u would give just a couple more lines of context. :( -- Cliff Woolley [EMAIL PROTECTED] Charlottesville, VA
Re: cvs commit: httpd-2.0/server core.c
[EMAIL PROTECTED] writes: jerenkrantz02/02/18 20:45:53 Modified:.CHANGES include ap_mmn.h http_core.h server core.c Log: Introduce AddOutputFilterByType directive. AddOutputFilterByType DEFLATE text/html Why does this warrant a change to MMN? What module APIs have been changed/removed? (My guess is that it was reasonable to change it anyway to cover API changes like install_transport_filters - pre_connection, but I'm curious about why you changed it with this commit.) -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
On Tue, Feb 19, 2002 at 12:52:01PM -0500, Jeff Trawick wrote: [EMAIL PROTECTED] writes: jerenkrantz02/02/18 20:45:53 Modified:.CHANGES include ap_mmn.h http_core.h server core.c Log: Introduce AddOutputFilterByType directive. AddOutputFilterByType DEFLATE text/html Why does this warrant a change to MMN? What module APIs have been changed/removed? (My guess is that it was reasonable to change it anyway to cover API changes like install_transport_filters - pre_connection, but I'm curious about why you changed it with this commit.) A new field was added to core_dir_config. I believe that warrants a bump. If I'm wrong, ooops. -- justin
RE: cvs commit: httpd-2.0/server core.c
On Tue, Feb 19, 2002 at 12:52:01PM -0500, Jeff Trawick wrote: [EMAIL PROTECTED] writes: jerenkrantz02/02/18 20:45:53 Modified:.CHANGES include ap_mmn.h http_core.h server core.c Log: Introduce AddOutputFilterByType directive. AddOutputFilterByType DEFLATE text/html Why does this warrant a change to MMN? What module APIs have been changed/removed? (My guess is that it was reasonable to change it anyway to cover API changes like install_transport_filters - pre_connection, but I'm curious about why you changed it with this commit.) A new field was added to core_dir_config. I believe that warrants a bump. If I'm wrong, ooops. -- justin Core_dir_config should be a private structure, so it doesn't require a MMN bump. Ryan
Re: cvs commit: httpd-2.0/server core.c
On Tue, Feb 19, 2002 at 03:25:23PM -0800, Ryan Bloom wrote: Core_dir_config should be a private structure, so it doesn't require a MMN bump. Just looking at when MMN has been bumped before, it looks like almost all changes to core_dir_config have been followed by MMN bump. Why have people done that? (I'm curious now.) -- justin
Re: cvs commit: httpd-2.0/server core.c
From: [EMAIL PROTECTED] Sent: Monday, February 18, 2002 10:45 PM jerenkrantz02/02/18 20:45:53 Modified:.CHANGES include ap_mmn.h http_core.h server core.c Log: Introduce AddOutputFilterByType directive. AddOutputFilterByType DEFLATE text/html Why not InputFilterByType while you are at it, since they are such tight siblings :-?
Re: cvs commit: httpd-2.0/server core.c
On Wed, Feb 06, 2002 at 04:16:55PM -, [EMAIL PROTECTED] wrote: trawick 02/02/06 08:16:55 Modified:server core.c Log: yet another tweak to empty brigade checking on entry to core_input_filter(): since APR_BRIGADE_EMPTY() assumes a non-empty brigade, we have to check before invoking that macro since APR_BRIGADE_EMPTY() can make a brigade empty, we have to check after invoking that macro An obvious variation of this fix would be to change APR_BRIGADE_NORMALIZE() to deal with empty brigades. You mean APR_BRIGADE_NORMALIZE assumes a non-empty brigade, right? -- justin
Re: cvs commit: httpd-2.0/server core.c
On Wed, 6 Feb 2002, Justin Erenkrantz wrote: You mean APR_BRIGADE_NORMALIZE assumes a non-empty brigade, right? -- justin Yeah... must have been a typo. Boy do I hate normalize... makes me need another cup of coffee just thinking about it. :) Normalize is definitely next on my hitlist. --Cliff -- Cliff Woolley [EMAIL PROTECTED] Charlottesville, VA
Re: cvs commit: httpd-2.0/server core.c
Justin Erenkrantz [EMAIL PROTECTED] writes: On Wed, Feb 06, 2002 at 04:16:55PM -, [EMAIL PROTECTED] wrote: trawick 02/02/06 08:16:55 ... An obvious variation of this fix would be to change APR_BRIGADE_NORMALIZE() to deal with empty brigades. You mean APR_BRIGADE_NORMALIZE assumes a non-empty brigade, right? -- justin yessir -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
Justin Erenkrantz [EMAIL PROTECTED] writes: On Tue, Feb 05, 2002 at 10:56:44PM -, [EMAIL PROTECTED] wrote: trawick 02/02/05 14:56:44 Modified:.CHANGES server core.c Log: In core_input_filter, check for an empty brigade after APR_BRIGADE_NORMALIZE(). Otherwise, we can get segfaults if a client says it will post some data but we get FIN before any data arrives. Ah, yes. That looks right. Do we think this fixes the daedalus cores? -- justin I'm quite sure it fixes the prevalent scenario. Greg mentioned another segfault flavor on the input side. I haven't looked at that one to see what it is about. -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
I suspect something is messed up in your build. f-ctx (and net-c) are both
initialized
in the core_install_transport_filters(conn_rec *c, apr_socket_t *csd) call. It compiles
and serves pages cleanly for me. Can you verify if core_install_transport_filters is
being
run?
Bill
[EMAIL PROTECTED] writes:
stoddard02/01/29 17:57:53
Modified:server core.c
Log:
Now how did this slip by. Fixes a segfault. I tested this code, really...
I'm not sure this is the only one.
var decls:
core_net_rec *net = f-ctx;
core_ctx_t *ctx = net-in_ctx;
The first time through f-ctx is NULL, so net-in_ctx is a
segfault. Right?
Index: core.c
===
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.140
retrieving revision 1.141
diff -u -r1.140 -r1.141
--- core.c 29 Jan 2002 19:02:03 - 1.140
+++ core.c 30 Jan 2002 01:57:53 - 1.141
@@ -3209,7 +3209,7 @@
core_output_filter_ctx_t *ctx = net-out_ctx;
if (ctx == NULL) {
-ctx = apr_pcalloc(net-c-pool, sizeof(*ctx));
+ctx = apr_pcalloc(c-pool, sizeof(*ctx));
net-out_ctx = ctx;
}
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
Bill Stoddard [EMAIL PROTECTED] writes: I suspect something is messed up in your build. never :) (seriously... I spend most of my day watching make extraclean scroll by... even after a config file change) All I gotta say is: #0 core_output_filter (f=0x40615fec, b=0x405ddff4) at core.c:3209 3209core_output_filter_ctx_t *ctx = net-out_ctx; (gdb) p net $1 = (core_net_rec *) 0x0 (gdb) f-ctx (and net-c) are both initialized in the core_install_transport_filters(conn_rec *c, apr_socket_t *csd) call. It compiles and serves pages cleanly for me. Can you verify if core_install_transport_filters is being run? I'll check... -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
Bill Stoddard [EMAIL PROTECTED] writes: I suspect something is messed up in your build. f-ctx (and net-c) are both initialized in the core_install_transport_filters(conn_rec *c, apr_socket_t *csd) call. It compiles and serves pages cleanly for me. Can you verify if core_install_transport_filters is being run? Does this backtrace tell you anything (proxy)? #0 core_output_filter (f=0x40615fec, b=0x405ddff4) at core.c:3209 3209core_output_filter_ctx_t *ctx = net-out_ctx; #0 core_output_filter (f=0x40615fec, b=0x405ddff4) at core.c:3209 #1 0x809ec63 in ap_pass_brigade (next=0x40615fec, bb=0x405ddff4) at util_filter.c:445 #2 0x40b2a7c9 in ap_proxy_http_request (p=0x4052efc0, r=0x40562e84, p_conn=0x405e3fec, origin=0x40603fb8, conf=0x40deffb0, uri=0x405e1fd4, url=0x405f9fec /cgi-bin/printenv, bb=0x405ddff4, server_portstr=0xb94c ) at proxy_http.c:640 #3 0x40b2b245 in ap_proxy_http_handler (r=0x40562e84, conf=0x40deffb0, url=0x405f9fec /cgi-bin/printenv, proxyname=0x0, proxyport=0) at proxy_http.c:1016 ---Type return to continue, or q return to quit---#4 0x40a18832 in proxy_run_scheme_han dler (r=0x40562e84, conf=0x40deffb0, url=0x405cdfd6 http://127.0.0.1:8099/cgi-bin/printenv;, proxyhost=0x0, proxyport=0) at mod_proxy.c:1008 #5 0x40a17ab7 in proxy_handler (r=0x40562e84) at mod_proxy.c:460 #6 0x8093ed2 in ap_run_handler (r=0x40562e84) at config.c:185 #7 0x8094427 in ap_invoke_handler (r=0x40562e84) at config.c:359 #8 0x8070463 in ap_process_request (r=0x40562e84) at http_request.c:290 #9 0x806c511 in ap_process_http_connection (c=0x4054efb8) at http_core.c:287 #10 0x809cd8e in ap_run_process_connection (c=0x4054efb8) at connection.c:84 #11 0x809d056 in ap_process_connection (c=0x4054efb8, csd=0x40544fd4) at connection.c:231 #12 0x8092ade in child_main (child_num_arg=2) at prefork.c:717 #13 0x8092c0b in make_child (s=0x40b00f9c, slot=2) at prefork.c:807 #14 0x8092c71 in startup_children (number_to_start=5) at prefork.c:830 #15 0x8092fce in ap_mpm_run (_pconf=0x40220fc0, plog=0x4078dfc0, s=0x40b00f9c) at prefork.c:1037 #16 0x8098257 in main (argc=3, argv=0xbc34) at main.c:498 Here is the request I sent: GET http://127.0.0.1:/cgi-bin/printenv HTTP/1.0 ( is my port, so I'm proxying to my own server) This is a bare-bones proxy configuration. If you need more info let me know. Rather than set breakpoints in that hook to see if I get there I'd rather teach my scripts to save backtraces from coredumps while I've got a live problem. -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
Compiles but not tested. Bill Index: proxy_http.c === RCS file: /home/cvs/httpd-2.0/modules/proxy/proxy_http.c,v retrieving revision 1.124 diff -u -r1.124 proxy_http.c --- proxy_http.c 29 Jan 2002 21:08:37 - 1.124 +++ proxy_http.c 30 Jan 2002 03:46:54 - @@ -444,7 +444,7 @@ p_conn-addr, p_conn-name); /* set up the connection filters */ -ap_proxy_pre_http_connection(*origin); +ap_run_install_transport_filters(*origin, p_conn-sock); } return OK; } - Original Message - From: Jeff Trawick [EMAIL PROTECTED] To: [EMAIL PROTECTED] Sent: Tuesday, January 29, 2002 9:54 PM Subject: Re: cvs commit: httpd-2.0/server core.c Bill Stoddard [EMAIL PROTECTED] writes: I suspect something is messed up in your build. f-ctx (and net-c) are both initialized in the core_install_transport_filters(conn_rec *c, apr_socket_t *csd) call. It compiles and serves pages cleanly for me. Can you verify if core_install_transport_filters is being run? Does this backtrace tell you anything (proxy)? #0 core_output_filter (f=0x40615fec, b=0x405ddff4) at core.c:3209 3209core_output_filter_ctx_t *ctx = net-out_ctx; #0 core_output_filter (f=0x40615fec, b=0x405ddff4) at core.c:3209 #1 0x809ec63 in ap_pass_brigade (next=0x40615fec, bb=0x405ddff4) at util_filter.c:445 #2 0x40b2a7c9 in ap_proxy_http_request (p=0x4052efc0, r=0x40562e84, p_conn=0x405e3fec, origin=0x40603fb8, conf=0x40deffb0, uri=0x405e1fd4, url=0x405f9fec /cgi-bin/printenv, bb=0x405ddff4, server_portstr=0xb94c ) at proxy_http.c:640 #3 0x40b2b245 in ap_proxy_http_handler (r=0x40562e84, conf=0x40deffb0, url=0x405f9fec /cgi-bin/printenv, proxyname=0x0, proxyport=0) at proxy_http.c:1016 ---Type return to continue, or q return to quit---#4 0x40a18832 in proxy_run_scheme_han dler (r=0x40562e84, conf=0x40deffb0, url=0x405cdfd6 http://127.0.0.1:8099/cgi-bin/printenv;, proxyhost=0x0, proxyport=0) at mod_proxy.c:1008 #5 0x40a17ab7 in proxy_handler (r=0x40562e84) at mod_proxy.c:460 #6 0x8093ed2 in ap_run_handler (r=0x40562e84) at config.c:185 #7 0x8094427 in ap_invoke_handler (r=0x40562e84) at config.c:359 #8 0x8070463 in ap_process_request (r=0x40562e84) at http_request.c:290 #9 0x806c511 in ap_process_http_connection (c=0x4054efb8) at http_core.c:287 #10 0x809cd8e in ap_run_process_connection (c=0x4054efb8) at connection.c:84 #11 0x809d056 in ap_process_connection (c=0x4054efb8, csd=0x40544fd4) at connection.c:231 #12 0x8092ade in child_main (child_num_arg=2) at prefork.c:717 #13 0x8092c0b in make_child (s=0x40b00f9c, slot=2) at prefork.c:807 #14 0x8092c71 in startup_children (number_to_start=5) at prefork.c:830 #15 0x8092fce in ap_mpm_run (_pconf=0x40220fc0, plog=0x4078dfc0, s=0x40b00f9c) at prefork.c:1037 #16 0x8098257 in main (argc=3, argv=0xbc34) at main.c:498 Here is the request I sent: GET http://127.0.0.1:/cgi-bin/printenv HTTP/1.0 ( is my port, so I'm proxying to my own server) This is a bare-bones proxy configuration. If you need more info let me know. Rather than set breakpoints in that hook to see if I get there I'd rather teach my scripts to save backtraces from coredumps while I've got a live problem. -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
Bill Stoddard [EMAIL PROTECTED] writes: Compiles but not tested. it works for me... -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server core.c
Bill Stoddard [EMAIL PROTECTED] writes: Compiles but not tested. it works for me... Just tested it on Windows. Bombs w/o the patch. Ok with. Committing. Bill
Re: cvs commit: httpd-2.0/server core.c
On Wed, Jan 02, 2002 at 05:15:34PM -0500, Bill Stoddard wrote: This patch breaks the proxy. Specifically, anyone who uses ap_proxy_make_fake_req(). Get a seg fault in ap_get_limit_req_body because r-per_dir_config is NULL. I'll spend some time on this tomorrow unless someone wants to jump on it tonight. Is it valid for r-per_dir_config to be null? Hmm. I wonder if ap_get_limit_req_body should be fixed to handle this case instead of ap_http_filter? -- justin No. It's entirely invalid. At the very least - you are looking the r-server-lookup_defaults, plus the Location sections in per_dir_config. That's always true, anything that changes that assumption is broken. Now if either proxy or your patch skips the initial Location lookup (or it is otherwise circumvented) then you get what you pay for. It's not that clear to me what the right solution should be. Checkout ap_proxy_http_process_response(). This function reads the -response- from the proxied server and dummies up a request_rec to do so. So is this a valid approach or not? If it is, then we do not need to do location/directory walks (and it is fine if r-per_dir_config is NULL. Bill
Re: cvs commit: httpd-2.0/server core.c
On Thursday 03 January 2002 05:16 am, Bill Stoddard wrote: On Wed, Jan 02, 2002 at 05:15:34PM -0500, Bill Stoddard wrote: This patch breaks the proxy. Specifically, anyone who uses ap_proxy_make_fake_req(). Get a seg fault in ap_get_limit_req_body because r-per_dir_config is NULL. I'll spend some time on this tomorrow unless someone wants to jump on it tonight. Is it valid for r-per_dir_config to be null? Hmm. I wonder if ap_get_limit_req_body should be fixed to handle this case instead of ap_http_filter? -- justin No. It's entirely invalid. At the very least - you are looking the r-server-lookup_defaults, plus the Location sections in per_dir_config. That's always true, anything that changes that assumption is broken. Now if either proxy or your patch skips the initial Location lookup (or it is otherwise circumvented) then you get what you pay for. It's not that clear to me what the right solution should be. Checkout ap_proxy_http_process_response(). This function reads the -response- from the proxied server and dummies up a request_rec to do so. So is this a valid approach or not? If it is, then we do not need to do location/directory walks (and it is fine if r-per_dir_config is NULL. We must be able to dummy up request_rec structures in order to use filters that aren't attached to a request. I believe that r-per_dir_config should be allowed to be NULL. Ryan __ Ryan Bloom [EMAIL PROTECTED] Covalent Technologies [EMAIL PROTECTED] --
