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
