You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Roy T. Fielding" <fi...@gbiv.com> on 2004/10/22 20:37:23 UTC

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, ake@apache.org 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  Changes    Path
>   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 -0000	1.1613
>   +++ CHANGES	22 Oct 2004 15:22:03 -0000	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 -0000	1.69
>   +++ ap_mmn.h	22 Oct 2004 15:22:04 -0000	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 -10    httpd-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 -0000	1.92
>   +++ http_protocol.h	22 Oct 2004 15:22:04 -0000	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 data for the current request to the client
>   @@ -443,9 +443,9 @@
>     * @param bufsiz The size of the buffer
>     * @return Number of bytes inserted into the buffer.  When done 
> reading, 0
>     *         if EOF, or -1 if there was an error
>   - * @deffunc long ap_get_client_block(request_rec *r, char *buffer, 
> apr_size_t bufsiz)
>   + * @deffunc apr_ssize_t ap_get_client_block(request_rec *r, char 
> *buffer, apr_size_t bufsiz)
>     */
>   -AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, 
> apr_size_t bufsiz);
>   +AP_DECLARE(apr_ssize_t) ap_get_client_block(request_rec *r, char 
> *buffer, apr_size_t bufsiz);
>
>    /**
>     * In HTTP/1.1, any method can have a body.  However, most GET 
> handlers
>
>
>
>   1.213     +4 -4      httpd-2.0/include/httpd.h
>
>   Index: httpd.h
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/include/httpd.h,v
>   retrieving revision 1.212
>   retrieving revision 1.213
>   diff -u -r1.212 -r1.213
>   --- httpd.h	12 Aug 2004 05:22:59 -0000	1.212
>   +++ httpd.h	22 Oct 2004 15:22:04 -0000	1.213
>   @@ -1091,7 +1091,7 @@
>        /** Pathname for ServerPath */
>        const char *path;
>        /** Length of path */
>   -    int pathlen;
>   +    apr_size_t pathlen;
>
>        /** Normal names for ServerAlias servers */
>        apr_array_header_t *names;
>   @@ -1244,7 +1244,7 @@
>     * address of field is shifted to the next non-comma, non-whitespace
>     * character.  len is the length of the item excluding any 
> beginning whitespace.
>     */
>   -AP_DECLARE(const char *) ap_size_list_item(const char **field, int 
> *len);
>   +AP_DECLARE(const char *) ap_size_list_item(const char **field, 
> apr_size_t *len);
>
>    /**
>     * Retrieve an HTTP header field list item, as separated by a comma,
>   @@ -1587,7 +1587,7 @@
>     * @param c The character to search for
>     * @return The index of the first occurrence of c in str
>     */
>   -AP_DECLARE(int) ap_ind(const char *str, char c);	/* Sigh... */
>   +AP_DECLARE(apr_ssize_t) ap_ind(const char *str, char c);	/* Sigh... 
> */
>
>    /**
>     * Search a string from right to left for the first occurrence of a
>   @@ -1596,7 +1596,7 @@
>     * @param c The character to search for
>     * @return The index of the first occurrence of c in str
>     */
>   -AP_DECLARE(int) ap_rind(const char *str, char c);
>   +AP_DECLARE(apr_ssize_t) ap_rind(const char *str, char c);
>
>    /**
>     * Given a string, replace any bare " with \" .
>
>
>
>   1.55      +1 -1      httpd-2.0/include/scoreboard.h
>
>   Index: scoreboard.h
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/include/scoreboard.h,v
>   retrieving revision 1.54
>   retrieving revision 1.55
>   diff -u -r1.54 -r1.55
>   --- scoreboard.h	15 Sep 2004 12:08:39 -0000	1.54
>   +++ scoreboard.h	22 Oct 2004 15:22:04 -0000	1.55
>   @@ -164,7 +164,7 @@
>    int ap_create_scoreboard(apr_pool_t *p, ap_scoreboard_e t);
>    apr_status_t ap_reopen_scoreboard(apr_pool_t *p, apr_shm_t **shm, 
> int detached);
>    void ap_init_scoreboard(void *shared_score);
>   -AP_DECLARE(int) ap_calc_scoreboard_size(void);
>   +AP_DECLARE(apr_size_t) ap_calc_scoreboard_size(void);
>    apr_status_t ap_cleanup_scoreboard(void *d);
>
>    AP_DECLARE(void) ap_create_sb_handle(ap_sb_handle_t **new_sbh, 
> apr_pool_t *p,
>
>
>
>   1.24      +2 -2      httpd-2.0/include/util_script.h
>
>   Index: util_script.h
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/include/util_script.h,v
>   retrieving revision 1.23
>   retrieving revision 1.24
>   diff -u -r1.23 -r1.24
>   --- util_script.h	9 Feb 2004 20:38:21 -0000	1.23
>   +++ util_script.h	22 Oct 2004 15:22:04 -0000	1.24
>   @@ -53,7 +53,7 @@
>     * @return The length of the path info
>     * @deffunc int ap_find_path_info(const char *uri, const char 
> *path_info)
>     */
>   -AP_DECLARE(int) ap_find_path_info(const char *uri, const char 
> *path_info);
>   +AP_DECLARE(apr_size_t) ap_find_path_info(const char *uri, const 
> char *path_info);
>
>    /**
>     * Add CGI environment variables required by HTTP/1.1 to the 
> request's
>   @@ -131,7 +131,7 @@
>     * @deffunc int ap_scan_script_header_err_core(request_rec *r, char 
> *buffer, int (*getsfunc)(char *, int, void *), void *getsfunc_data)
>     */
>    AP_DECLARE(int) ap_scan_script_header_err_core(request_rec *r, char 
> *buffer,
>   -				       int (*getsfunc) (char *, int, void *),
>   +				       int (*getsfunc) (char *, apr_size_t, void *),
>    				       void *getsfunc_data);
>
>    #ifdef __cplusplus
>
>
>
>   1.485     +2 -2      httpd-2.0/modules/http/http_protocol.c
>
>   Index: http_protocol.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
>   retrieving revision 1.484
>   retrieving revision 1.485
>   diff -u -r1.484 -r1.485
>   --- http_protocol.c	29 Sep 2004 14:38:42 -0000	1.484
>   +++ http_protocol.c	22 Oct 2004 15:22:04 -0000	1.485
>   @@ -662,7 +662,7 @@
>     */
>    AP_DECLARE(int) ap_method_number_of(const char *method)
>    {
>   -    int len = strlen(method);
>   +    apr_size_t len = strlen(method);
>        int which = lookup_builtin_method(method, len);
>
>        if (which != UNKNOWN_METHOD)
>   @@ -1858,7 +1858,7 @@
>     * Returns 0 on End-of-body, -1 on error or premature chunk end.
>     *
>     */
>   -AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer,
>   +AP_DECLARE(apr_ssize_t) ap_get_client_block(request_rec *r, char 
> *buffer,
>                                         apr_size_t bufsiz)
>    {
>        apr_status_t rv;
>
>
>
>   1.288     +10 -7     httpd-2.0/server/core.c
>
>   Index: core.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/core.c,v
>   retrieving revision 1.287
>   retrieving revision 1.288
>   diff -u -r1.287 -r1.288
>   --- core.c	22 Sep 2004 18:21:29 -0000	1.287
>   +++ core.c	22 Oct 2004 15:22:04 -0000	1.288
>   @@ -2298,7 +2298,7 @@
>        }
>
>        cmd->server->path = arg;
>   -    cmd->server->pathlen = (int)strlen(arg);
>   +    cmd->server->pathlen = strlen(arg);
>        return NULL;
>    }
>
>   @@ -3009,19 +3009,20 @@
>    }
>
>    static apr_status_t writev_it_all(apr_socket_t *s,
>   -                                  struct iovec *vec, int nvec,
>   +                                  struct iovec *vec, apr_size_t 
> nvec,
>                                      apr_size_t len, apr_size_t 
> *nbytes)
>    {
>        apr_size_t bytes_written = 0;
>        apr_status_t rv;
>        apr_size_t n = len;
>   -    int i = 0;
>   +    apr_size_t i = 0;
>
>        *nbytes = 0;
>
>        /* XXX handle checking for non-blocking socket */
>        while (bytes_written != len) {
>   -        rv = apr_socket_sendv(s, vec + i, nvec - i, &n);
>   +        /* Cast to eliminate 64 bit warning */
>   +        rv = apr_socket_sendv(s, vec + i, (apr_int32_t)(nvec - i), 
> &n);
>            *nbytes += n;
>            bytes_written += n;
>            if (rv != APR_SUCCESS)
>   @@ -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;
>
>        if (mode == AP_MODE_INIT) {
>            /*
>   @@ -4288,12 +4289,14 @@
>
>                memset(&hdtr, '\0', sizeof(hdtr));
>                if (nvec) {
>   -                hdtr.numheaders = nvec;
>   +                /* Cast to eliminate 64 bit warning */
>   +                hdtr.numheaders = (int)nvec;
>                    hdtr.headers = vec;
>                }
>
>                if (nvec_trailers) {
>   -                hdtr.numtrailers = nvec_trailers;
>   +                /* Cast to eliminate 64 bit warning */
>   +                hdtr.numtrailers = (int)nvec_trailers;
>                    hdtr.trailers = vec_trailers;
>                }
>
>
>
>
>   1.154     +7 -7      httpd-2.0/server/protocol.c
>
>   Index: protocol.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
>   retrieving revision 1.153
>   retrieving revision 1.154
>   diff -u -r1.153 -r1.154
>   --- protocol.c	21 Sep 2004 21:07:23 -0000	1.153
>   +++ protocol.c	22 Oct 2004 15:22:05 -0000	1.154
>   @@ -1387,7 +1387,7 @@
>        return c;
>    }
>
>   -AP_DECLARE(int) ap_rputs(const char *str, request_rec *r)
>   +AP_DECLARE(apr_ssize_t) ap_rputs(const char *str, request_rec *r)
>    {
>        apr_size_t len;
>
>   @@ -1441,9 +1441,9 @@
>        return APR_SUCCESS;
>    }
>
>   -AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, 
> va_list va)
>   +AP_DECLARE(apr_ssize_t) ap_vrprintf(request_rec *r, const char 
> *fmt, va_list va)
>    {
>   -    apr_size_t written;
>   +    apr_ssize_t written;
>        struct ap_vrprintf_data vd;
>        char vrprintf_buf[AP_IOBUFSIZE];
>
>   @@ -1461,7 +1461,7 @@
>        *(vd.vbuff.curpos) = '\0';
>
>        if (written != -1) {
>   -        int n = vd.vbuff.curpos - vrprintf_buf;
>   +        apr_size_t n = vd.vbuff.curpos - vrprintf_buf;
>
>            /* last call to buffer_output, to finish clearing the 
> buffer */
>            if (buffer_output(r, vrprintf_buf,n) != APR_SUCCESS)
>   @@ -1473,10 +1473,10 @@
>        return written;
>    }
>
>   -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, ...)
>    {
>        va_list va;
>   -    int n;
>   +    apr_ssize_t n;
>
>        if (r->connection->aborted)
>            return -1;
>   @@ -1488,7 +1488,7 @@
>        return n;
>    }
>
>   -AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r, ...)
>   +AP_DECLARE_NONSTD(apr_ssize_t) ap_rvputs(request_rec *r, ...)
>    {
>        va_list va;
>        const char *s;
>
>
>
>   1.139     +2 -1      httpd-2.0/server/request.c
>
>   Index: request.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/request.c,v
>   retrieving revision 1.138
>   retrieving revision 1.139
>   diff -u -r1.138 -r1.139
>   --- request.c	11 Oct 2004 19:27:29 -0000	1.138
>   +++ request.c	22 Oct 2004 15:22:05 -0000	1.139
>   @@ -1213,7 +1213,8 @@
>            /* We start now_merged from NULL since we want to build
>             * a locations list that can be merged to any vhost.
>             */
>   -        int len, sec_idx;
>   +        apr_size_t len;
>   +        int sec_idx;
>            int matches = cache->walked->nelts;
>            walk_walked_t *last_walk = 
> (walk_walked_t*)cache->walked->elts;
>            cache->cached = entry_uri;
>
>
>
>   1.78      +1 -1      httpd-2.0/server/scoreboard.c
>
>   Index: scoreboard.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/scoreboard.c,v
>   retrieving revision 1.77
>   retrieving revision 1.78
>   diff -u -r1.77 -r1.78
>   --- scoreboard.c	22 Sep 2004 13:35:31 -0000	1.77
>   +++ scoreboard.c	22 Oct 2004 15:22:05 -0000	1.78
>   @@ -88,7 +88,7 @@
>        return APR_SUCCESS;
>    }
>
>   -AP_DECLARE(int) ap_calc_scoreboard_size(void)
>   +AP_DECLARE(apr_size_t) ap_calc_scoreboard_size(void)
>    {
>        ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
>        ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
>
>
>
>   1.151     +26 -19    httpd-2.0/server/util.c
>
>   Index: util.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/util.c,v
>   retrieving revision 1.150
>   retrieving revision 1.151
>   diff -u -r1.150 -r1.151
>   --- util.c	15 Sep 2004 11:44:05 -0000	1.150
>   +++ util.c	22 Oct 2004 15:22:05 -0000	1.151
>   @@ -451,7 +451,7 @@
>    AP_DECLARE(void) ap_getparents(char *name)
>    {
>        char *next;
>   -    int l, w, first_dot;
>   +    apr_ssize_t l, w, first_dot;
>
>        /* Four paseses, as per RFC 1808 */
>        /* a) remove ./ path segments */
>   @@ -480,7 +480,7 @@
>        while (name[l] != '\0') {
>            if (name[l] == '.' && name[l + 1] == '.' && IS_SLASH(name[l 
> + 2])
>                && (l == 0 || IS_SLASH(name[l - 1]))) {
>   -            register int m = l + 3, n;
>   +            register apr_ssize_t m = l + 3, n;
>
>                l = l - 2;
>                if (l >= 0) {
>   @@ -592,7 +592,7 @@
>    {
>        const char *last_slash = ap_strrchr_c(s, '/');
>        char *d;
>   -    int l;
>   +    apr_size_t l;
>
>        if (last_slash == NULL) {
>            return apr_pstrdup(p, "");
>   @@ -623,7 +623,7 @@
>    AP_DECLARE(char *) ap_getword(apr_pool_t *atrans, const char 
> **line, char stop)
>    {
>        const char *pos = *line;
>   -    int len;
>   +    apr_size_t len;
>        char *res;
>
>        while ((*pos != stop) && *pos) {
>   @@ -653,7 +653,7 @@
>    AP_DECLARE(char *) ap_getword_white(apr_pool_t *atrans, const char 
> **line)
>    {
>        const char *pos = *line;
>   -    int len;
>   +    apr_size_t len;
>        char *res;
>
>        while (!apr_isspace(*pos) && *pos) {
>   @@ -705,12 +705,12 @@
>     * all honored
>     */
>
>   -static char *substring_conf(apr_pool_t *p, const char *start, int 
> len,
>   +static char *substring_conf(apr_pool_t *p, const char *start, 
> apr_size_t len,
>                                char quote)
>    {
>        char *result = apr_palloc(p, len + 2);
>        char *resp = result;
>   -    int i;
>   +    apr_size_t i;
>
>        for (i = 0; i < len; ++i) {
>            if (start[i] == '\\' && (start[i + 1] == '\\'
>   @@ -887,11 +887,13 @@
>        return (int)EOF;
>    }
>
>   -static void *cfg_getstr(void *buf, size_t bufsiz, void *param)
>   +static void *cfg_getstr(void *buf, apr_size_t bufsiz, void *param)
>    {
>        apr_file_t *cfp = (apr_file_t *) param;
>        apr_status_t rv;
>   -    rv = apr_file_gets(buf, bufsiz, cfp);
>   +
>   +    /* Cast to eliminate 64 bit warning */
>   +    rv = apr_file_gets(buf, (int)bufsiz, cfp);
>        if (rv == APR_SUCCESS) {
>            return buf;
>        }
>   @@ -971,7 +973,7 @@
>        new_cfg->param = file;
>        new_cfg->name = apr_pstrdup(p, name);
>        new_cfg->getch = (int (*)(void *)) cfg_getch;
>   -    new_cfg->getstr = (void *(*)(void *, size_t, void *)) 
> cfg_getstr;
>   +    new_cfg->getstr = (void *(*)(void *, apr_size_t, void *)) 
> cfg_getstr;
>        new_cfg->close = (int (*)(void *)) cfg_close;
>        new_cfg->line_number = 0;
>        *ret_cfg = new_cfg;
>   @@ -1152,7 +1154,7 @@
>     * of field is shifted to the next non-comma, non-whitespace 
> character.
>     * len is the length of the item excluding any beginning whitespace.
>     */
>   -AP_DECLARE(const char *) ap_size_list_item(const char **field, int 
> *len)
>   +AP_DECLARE(const char *) ap_size_list_item(const char **field, 
> apr_size_t *len)
>    {
>        const unsigned char *ptr = (const unsigned char *)*field;
>        const unsigned char *token;
>   @@ -1218,7 +1220,8 @@
>        const unsigned char *ptr;
>        unsigned char *pos;
>        char *token;
>   -    int addspace = 0, in_qpair = 0, in_qstr = 0, in_com = 0, 
> tok_len = 0;
>   +    int addspace = 0, in_qpair = 0, in_qstr = 0, in_com = 0;
>   +    apr_size_t tok_len = 0;
>
>        /* Find the beginning and maximum length of the list item so 
> that
>         * we can allocate a buffer for the new string and reset the 
> field.
>   @@ -1411,7 +1414,7 @@
>        const char *ptr = *accept_line;
>        const char *tok_start;
>        char *token;
>   -    int tok_len;
>   +    apr_size_t tok_len;
>
>        /* Find first non-white byte */
>
>   @@ -1484,7 +1487,8 @@
>    AP_DECLARE(int) ap_find_last_token(apr_pool_t *p, const char *line,
>                                       const char *tok)
>    {
>   -    int llen, tlen, lidx;
>   +    apr_size_t llen, tlen;
>   +    apr_ssize_t lidx;
>
>        if (!line)
>            return 0;
>   @@ -1969,7 +1973,7 @@
>        return (x ? 1 : 0);                /* If the first character is 
> ':', it's broken, too */
>    }
>
>   -AP_DECLARE(int) ap_ind(const char *s, char c)
>   +AP_DECLARE(apr_ssize_t) ap_ind(const char *s, char c)
>    {
>        const char *p = ap_strchr_c(s, c);
>
>   @@ -1978,7 +1982,7 @@
>        return p - s;
>    }
>
>   -AP_DECLARE(int) ap_rind(const char *s, char c)
>   +AP_DECLARE(apr_ssize_t) ap_rind(const char *s, char c)
>    {
>        const char *p = ap_strrchr_c(s, c);
>
>   @@ -2079,10 +2083,13 @@
>    AP_DECLARE(char *) ap_pbase64encode(apr_pool_t *p, char *string)
>    {
>        char *encoded;
>   -    int l = strlen(string);
>   +    apr_size_t l = strlen(string);
>   +
>   +    /* Cast to eliminate 64 bit warning */
>   +    encoded = (char *) apr_palloc(p, 1 + 
> apr_base64_encode_len((int)l));
>
>   -    encoded = (char *) apr_palloc(p, 1 + apr_base64_encode_len(l));
>   -    l = apr_base64_encode(encoded, string, l);
>   +    /* Cast to eliminate 64 bit warning */
>   +    l = apr_base64_encode(encoded, string, (int)l);
>        encoded[l] = '\0'; /* make binary sequence into string */
>
>        return encoded;
>
>
>
>   1.92      +13 -12    httpd-2.0/server/util_script.c
>
>   Index: util_script.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/util_script.c,v
>   retrieving revision 1.91
>   retrieving revision 1.92
>   diff -u -r1.91 -r1.92
>   --- util_script.c	1 Aug 2004 01:12:30 -0000	1.91
>   +++ util_script.c	22 Oct 2004 15:22:05 -0000	1.92
>   @@ -283,10 +283,10 @@
>     * and find as much of the two that match as possible.
>     */
>
>   -AP_DECLARE(int) ap_find_path_info(const char *uri, const char 
> *path_info)
>   +AP_DECLARE(apr_size_t) ap_find_path_info(const char *uri, const 
> char *path_info)
>    {
>   -    int lu = strlen(uri);
>   -    int lp = strlen(path_info);
>   +    apr_ssize_t lu = strlen(uri);
>   +    apr_size_t lp = strlen(path_info);
>
>        while (lu-- && lp-- && uri[lu] == path_info[lp]);
>
>   @@ -354,7 +354,7 @@
>            apr_table_setn(e, "SCRIPT_NAME", r->uri);
>        }
>        else {
>   -        int path_info_start = ap_find_path_info(r->uri, 
> r->path_info);
>   +        apr_size_t path_info_start = ap_find_path_info(r->uri, 
> r->path_info);
>
>            apr_table_setn(e, "SCRIPT_NAME",
>                          apr_pstrndup(r->pool, r->uri, 
> path_info_start));
>   @@ -394,12 +394,12 @@
>    }
>
>    AP_DECLARE(int) ap_scan_script_header_err_core(request_rec *r, char 
> *buffer,
>   -                                       int (*getsfunc) (char *, 
> int, void *),
>   +                                       int (*getsfunc) (char *, 
> apr_size_t, void *),
>                                           void *getsfunc_data)
>    {
>        char x[MAX_STRING_LEN];
>        char *w, *l;
>   -    int p;
>   +    apr_size_t p;
>        int cgi_status = HTTP_OK;
>        apr_table_t *merge;
>        apr_table_t *cookie_table;
>   @@ -581,9 +581,10 @@
>        return OK;
>    }
>
>   -static int getsfunc_FILE(char *buf, int len, void *f)
>   +static int getsfunc_FILE(char *buf, apr_size_t len, void *f)
>    {
>   -    return apr_file_gets(buf, len, (apr_file_t *) f) == APR_SUCCESS;
>   +    /* Cast to eliminate 64 bit warning */
>   +    return apr_file_gets(buf, (int)len, (apr_file_t *) f) == 
> APR_SUCCESS;
>    }
>
>    AP_DECLARE(int) ap_scan_script_header_err(request_rec *r, 
> apr_file_t *f,
>   @@ -592,7 +593,7 @@
>        return ap_scan_script_header_err_core(r, buffer, getsfunc_FILE, 
> f);
>    }
>
>   -static int getsfunc_BRIGADE(char *buf, int len, void *arg)
>   +static int getsfunc_BRIGADE(char *buf, apr_size_t len, void *arg)
>    {
>        apr_bucket_brigade *bb = (apr_bucket_brigade *)arg;
>        const char *dst_end = buf + len - 1; /* leave room for 
> terminating null */
>   @@ -650,11 +651,11 @@
>        const char *curpos;
>    };
>
>   -static int getsfunc_STRING(char *w, int len, void *pvastrs)
>   +static int getsfunc_STRING(char *w, apr_size_t len, void *pvastrs)
>    {
>        struct vastrs *strs = (struct vastrs*) pvastrs;
>        const char *p;
>   -    int t;
>   +    apr_size_t t;
>
>        if (!strs->curpos || !*strs->curpos)
>            return 0;
>   @@ -674,7 +675,7 @@
>        }
>        else
>            strs->curpos += t;
>   -    return t;
>   +    return (int)t;
>    }
>
>    /* ap_scan_script_header_err_strs() accepts additional const char* 
> args...
>
>
>
>
>


Re: cvs commit: httpd-2.0/server core.c protocol.c request.c scoreboard.c util.c util_script.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
> 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 util.c util_script.c

Posted by Allan Edwards <ak...@us.ibm.com>.
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