You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2020/07/03 09:02:00 UTC

Re: svn commit: r1879465 - /httpd/httpd/trunk/server/util_etag.c


On 7/3/20 10:21 AM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Fri Jul  3 08:21:13 2020
> New Revision: 1879465
> 
> URL: http://svn.apache.org/viewvc?rev=1879465&view=rev
> Log:
> Be defensive when calculating the digest. Make sure the offset is initialised
> to zero before reading the current offset.
> 
> Modified:
>     httpd/httpd/trunk/server/util_etag.c
> 
> Modified: httpd/httpd/trunk/server/util_etag.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_etag.c?rev=1879465&r1=1879464&r2=1879465&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_etag.c (original)
> +++ httpd/httpd/trunk/server/util_etag.c Fri Jul  3 08:21:13 2020

> @@ -170,25 +174,50 @@ AP_DECLARE(char *) ap_make_etag_ex(reque
>          }
>  
>          etag = apr_palloc(r->pool, weak_len + sizeof("\"\"") +
> -        		SHA1_DIGEST_BASE64_LEN + vlv_len + 4);
> +                SHA1_DIGEST_BASE64_LEN + vlv_len + 4);
>  
> -        if (apr_file_seek(fd, APR_CUR, &offset) != APR_SUCCESS) {
> +        if ((status = apr_file_seek(fd, APR_CUR, &offset)) != APR_SUCCESS) {
> +            ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO()
> +                          "Make etag: could not seek");
> +            if (er->pathname) {
> +                apr_file_close(fd);
> +            }
>              return "";
>          }
>  
>          apr_sha1_init(&context);
>          nbytes = sizeof(buf);
> -        while (apr_file_read(fd, buf, &nbytes) == APR_SUCCESS) {
> +        while ((status = apr_file_read(fd, buf, &nbytes)) == APR_SUCCESS) {

Don't we need to set the fp in fd via apr_file_seek(fd, APR_CUR, to 0?
In case we did not open the fd on our own it might be somewhere in the file and the digest would only base on a part of the file?

Regards

Rüdiger


Re: svn commit: r1879465 - /httpd/httpd/trunk/server/util_etag.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 03 Jul 2020, at 11:02, Ruediger Pluem <rp...@apache.org> wrote:

>>         apr_sha1_init(&context);
>>         nbytes = sizeof(buf);
>> -        while (apr_file_read(fd, buf, &nbytes) == APR_SUCCESS) {
>> +        while ((status = apr_file_read(fd, buf, &nbytes)) == APR_SUCCESS) {
> 
> Don't we need to set the fp in fd via apr_file_seek(fd, APR_CUR, to 0?
> In case we did not open the fd on our own it might be somewhere in the file and the digest would only base on a part of the file?

We do indeed - r1879469.

Regards,
Graham
—