2.0.59: ETag mtimes on 32- and 64-bit machines

Thread: 2.0.59: ETag mtimes on 32- and 64-bit machines

Search this list only Search all lists
2.0.59: ETag mtimes on 32- and 64-bit machines
	2007-08-24 10:32:32

Hi there Forgive me if this is the wrong list. It's not really a user question but I'm not sure it's a dev question, either, because I'm just looking for clarification that my changes are correct. We have a mix of 32- and 64-bit machines in our server farm across which we'd like to guarantee consistent ETags (inodes turned off, of course). As discussed here:   http://issues.apache.org/bugzilla/show_bug.cgi?id=40064 the ETags differ between architectures: [draytm01dev draytm01]$ GET -ed http://32bit/images/test.jpg \| egrep '(ETag\|Last-M\|Length)' ETag: "2e30-9b91cfc0" Content-Length: 11824 Last-Modified: Fri, 17 Sep 2004 10:27:19 GMT [draytm01dev draytm01]$ GET -ed http://64bit/images/test.jpg \| egrep '(ETag\|Last-M\|Length)' ETag: "2e30-3e4469b91cfc0" Content-Length: 11824 Last-Modified: Fri, 17 Sep 2004 10:27:19 GMT The problem is that the 64-bit (apr_uint64_t) mtime is cast to an unsigned long before conversion to hex, effectively wiping out the high 32 bits of the mtime on a 32-bit machine. Issue #40064 has a patch for Apache 2.2 which changes etag_ulong_to_hex() to etag_uint64_to_hex() and avoids casting the mtime to an (arch-dependent) unsigned long. We can't move to 2.2 at the moment so instead I patched 2.0.59 with the same changes (diff below -- note 2.2.x moved this code out to http_etag.c). Initially it didn't work -- the 32-bit machine still returned a truncated ETag. I fixed it with (in etag_uint64_to_hex()): - ;   int shift = sizeof(unsigned long) * 8 - 4; + ; int shift = sizeof(apr_uint64_t) * 8 - 4; Is this right? I'm not a C programmer but it seems right to me: without this change etag_uint64_to_hex() only converts the low 32 bits (ie, length of an unsigned int on a 32-bit machine). So now I have: [draytm01dev draytm01]$ GET -ed http://32bit/images/test.jpg \| egrep '(ETag\|Last-M\|Length)' ETag: "2e30-3e4469b91cfc0" Content-Length: 11824 Last-Modified: Fri, 17 Sep 2004 10:27:19 GMT [draytm01dev draytm01]$ GET -ed http://64bit/images/test.jpg \| egrep '(ETag\|Last-M\|Length)' ETag: "2e30-3e4469b91cfc0" Content-Length: 11824 Last-Modified: Fri, 17 Sep 2004 10:27:19 GMT If this is the correct fix then perhaps it should be applied to 2.2.x. If it's not, perhaps you could point me in the right direction :~) I'm guessing there won't be any interest in incorporating it into 2.0.x as 32-bit users will suddenly see their ETags change between point releases. Looking forward to any comments, Mark Drayton diff -ur httpd-2.0.59-orig/modules/http/http_protocol.c httpd-2.0.59 /modules/http/http_protocol.c --- httpd-2.0.59-orig/modules/http/http_protocol.c     2006-07-12 08:40:55.000000000 +0100 +++ httpd-2.0.59/modules/http/http_protocol.c  2007-08-24 16:06:56.000000000 +0100 -2698,16 +2698,17   l->method_list->;nelts = 0; } -/* Generate the human-readable hex representation of an unsigned long +/* Generate the human-readable hex representation of an apr_uin64_t * (basically a faster version of 'sprintf("%lx")9;) + * (basically a faster version of 'sprintf("%llx")') / #define HEX_DIGITS "0123456789abcdef" -static char etag_ulong_to_hex(char next, unsigned long u) +static char etag_uint64_to_hex(char next, apr_uint64_t u) ;{     int printing = 0;   int shift = sizeof(unsigned long) 8 - 4;   do { -   ;   unsigned long next_digit = ((u >> shift) & (unsigned long)0xf); +       unsigned short next_digit = ((u >> shift) & (apr_uint64_t)0xf);         if (next_digit) {     ;     next++ = HEX_DIGITS[next_digit];     ;     printing = 1; -2717,12 +2718,12       }     ; shift -= 4;   } while (shift); - ; next++ = HEX_DIGITS[u & (unsigned long)0xf]; +   next++ = HEX_DIGITS[u & (apr_uint64_t)0xf];   ; return next;  } #define ETAG_WEAK "W/" -#define CHARS_PER_UNSIGNED_LONG (sizeof(unsigned long) 2) +#define CHARS_PER_UINT64 (sizeof(apr_uint64_t) * 2) ;/* * Construct an entity tag (ETag) from resource information. If it's a real ; * file, build in some of the file characteristics. ; If the modification time -2785,7 +2786,7       ; * FileETag keywords.       ; /   ;   etag = apr_palloc(r->pool, weak_len + sizeof(""--"";) + -   ;         ;         3 CHARS_PER_UNSIGNED_LONG + 1); + ;         ;         ;   3 * CHARS_PER_UINT64 + 1);         next = etag;         if (weak) {     ;     while (weak) { -2795,21 +2796,21       next++ = '";';       ; bits_added = 0;   ;   if (etag_bits & ETAG_INODE) { - ;         next = etag_ulong_to_hex(next, (unsigned long)r->;finfo.inode); + ;         next = etag_uint64_to_hex(next, r->finfo.inode);   ;         bits_added \|= ETAG_INODE;       }     ; if (etag_bits & ETAG_SIZE) { ;         if (bits_added != 0) {     ;         next++ = '-';;     ;     } -   ;       next = etag_ulong_to_hex(next, (unsigned long)r->;finfo.size); +   ;       next = etag_uint64_to_hex(next, r-> finfo.size); ;         bits_added \|= ETAG_SIZE;         }     ; if (etag_bits & ETAG_MTIME) {     ;     if (bits_added != 0) {     ;         next++ = '-';;     ;     } -   ;       next = etag_ulong_to_hex(next, (unsigned long)r->;mtime); +         ; next = etag_uint64_to_hex(next, r->mtime);   ;   }     ; next++ = '";';       ; next = '';; -2819,7 +2820,7       ; * Not a file document, so just use the mtime: [W/]"mtime"     ;   /   ;   etag = apr_palloc(r->pool, weak_len + sizeof("""") + -   ;         ;         CHARS_PER_UNSIGNED_LONG + 1); + ;         ;         ;   CHARS_PER_UINT64 + 1);         next = etag;         if (weak) {     ;     while (weak) { -2827,7 +2828,7       ;   }     ; }     ; next++ = '";'; -    ; next = etag_ulong_to_hex(next, (unsigned long)r->;mtime); +       next = etag_uint64_to_hex(next, r->mtime);   ;   next++ = '";';       ; *next = '';;     }
Re: 2.0.59: ETag mtimes on 32- and 64-bit machines
	2007-08-31 06:57:05
On Fri, Aug 24, 2007 at 04:32:32PM +0100, Mark Drayton wrote: ... > Issue #40064 has a patch for Apache 2.2 which changes etag_ulong_to_hex() to > etag_uint64_to_hex() and avoids casting the mtime to an (arch-dependent) > unsigned long. We can't move to 2.2 at the moment so instead I patched > 2.0.59 with the same changes (diff below -- note 2.2.x moved this code out > to http_etag.c). Initially it didn't work -- the 32-bit machine still > returned a truncated ETag. I fixed it with (in etag_uint64_to_hex()): > > - int shift = sizeof(unsigned long) * 8 - 4; > + int shift = sizeof(apr_uint64_t) * 8 - 4; > > Is this right? I'm not a C programmer but it seems right to me: without this > change etag_uint64_to_hex() only converts the low 32 bits (ie, length of an > unsigned int on a 32-bit machine). So now I have: Yes, that's correct - this was fixed in Subversion exactly as you describe: http://svn.apache.org/viewvc?view=rev&rev=517238 (initial patch) http://svn.apache.org/viewvc?view=rev&rev=517654 (corrected) I've added a note to this effect to the bug report. joe

[1-2]

about | contact Other archives ( Real Estate discussion Medical topics )