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: [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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: [EMAIL PROTECTED] 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 [EMAIL PROTECTED] 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")') + * (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 = '\0'; @@ -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 = '\0'; }
