httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Drayton" <m...@markdrayton.info>
Subject 2.0.59: ETag mtimes on 32- and 64-bit machines
Date Fri, 24 Aug 2007 15:32:32 GMT
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:

[draytm01@dev 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

[draytm01@dev 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:

[draytm01@dev 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

[draytm01@dev 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';
     }

Mime
View raw message