From: Yann Ylavic <ylavic@apache.org>
Date: Wed, 8 Sep 2021 08:31:38 +0000 (+0000)
Subject: Merge r1893001, r1893002, r1893004 from trunk:
X-Git-Tag: candidate-2.4.49-rc1~3^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=17a14e5bdb6e293a453580d889e29b11ed025fe1;p=thirdparty%2Fapache%2Fhttpd.git

Merge r1893001, r1893002, r1893004 from trunk:


core: Set r->request_time before any logging, mod_unique_id needs it.

* server/protocol.c(read_request_line):
  Move r->request_time initialization before first APLOG_TRACE5,
  ap_log_rerror() may run the generate_log_id hooks and call mod_unique_id
  with no timestamp initialized (zero).



mod_unique_id: Follow up to r1892915: Shorter counter race condition yet.

* modules/metadata/mod_unique_id.c(gen_unique_id):
  Set the counter in network byte order for uuencoding only, allowing for
  simple cur_unique_id.counter++



mod_unique_id: Follow up to r1892915 and r1893002: Atomic counter.

* modules/metadata/mod_unique_id.c(gen_unique_id):
  Use an atomic 32bit counter to close the race condition with threaded MPMs,
  using the lower 16 bits for uuencoding still.


Submitted by: ylavic
Reviewed by: ylavic, rpluem, gbechis


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1893116 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/modules/metadata/mod_unique_id.c b/modules/metadata/mod_unique_id.c
index a25ae3fc9b8..2555749c2b8 100644
--- a/modules/metadata/mod_unique_id.c
+++ b/modules/metadata/mod_unique_id.c
@@ -26,6 +26,11 @@
 #include "apr_general.h"    /* for APR_OFFSETOF                */
 #include "apr_network_io.h"
 
+#ifdef APR_HAS_THREADS
+#include "apr_atomic.h"     /* for apr_atomic_inc32 */
+#include "mpm_common.h"     /* for ap_mpm_query */
+#endif
+
 #include "httpd.h"
 #include "http_config.h"
 #include "http_log.h"
@@ -123,6 +128,10 @@ typedef struct {
  * XXX: thrashing.
  */
 static unique_id_rec cur_unique_id;
+static apr_uint32_t cur_unique_counter;
+#ifdef APR_HAS_THREADS
+static int is_threaded_mpm;
+#endif
 
 /*
  * Number of elements in the structure unique_id_rec.
@@ -160,6 +169,11 @@ static int unique_id_global_init(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *pt
 
 static void unique_id_child_init(apr_pool_t *p, server_rec *s)
 {
+#ifdef APR_HAS_THREADS
+    is_threaded_mpm = 0;
+    ap_mpm_query(AP_MPMQ_IS_THREADED, &is_threaded_mpm);
+#endif
+
     ap_random_insecure_bytes(&cur_unique_id.root,
                              sizeof(cur_unique_id.root));
 
@@ -168,8 +182,8 @@ static void unique_id_child_init(apr_pool_t *p, server_rec *s)
      * against restart problems, and a little less protection against a clock
      * going backwards in time.
      */
-    ap_random_insecure_bytes(&cur_unique_id.counter,
-                             sizeof(cur_unique_id.counter));
+    ap_random_insecure_bytes(&cur_unique_counter,
+                             sizeof(cur_unique_counter));
 }
 
 /* Use the base64url encoding per RFC 4648, avoiding characters which
@@ -182,6 +196,10 @@ static const char uuencoder[64] = {
     '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '-', '_',
 };
 
+#ifndef APR_UINT16_MAX
+#define APR_UINT16_MAX 0xffffu
+#endif
+
 static const char *gen_unique_id(const request_rec *r)
 {
     char *str;
@@ -194,18 +212,24 @@ static const char *gen_unique_id(const request_rec *r)
         unique_id_rec foo;
         unsigned char pad[2];
     } paddedbuf;
+    apr_uint32_t counter;
     unsigned char *x,*y;
-    unsigned short counter;
     int i,j,k;
 
     memcpy(&new_unique_id.root, &cur_unique_id.root, ROOT_SIZE);
     new_unique_id.stamp = htonl((unsigned int)apr_time_sec(r->request_time));
     new_unique_id.thread_index = htonl((unsigned int)r->connection->id);
-    new_unique_id.counter = cur_unique_id.counter;
-
-    /* Increment the identifier for the next call */
-    counter = ntohs(new_unique_id.counter) + 1;
-    cur_unique_id.counter = htons(counter);
+#ifdef APR_HAS_THREADS
+    if (is_threaded_mpm)
+        counter = apr_atomic_inc32(&cur_unique_counter);
+    else
+#endif
+        counter = cur_unique_counter++;
+
+    /* The counter is two bytes for the uuencoded unique id, in network
+     * byte order.
+     */
+    new_unique_id.counter = htons(counter % APR_UINT16_MAX);
 
     /* we'll use a temporal buffer to avoid uuencoding the possible internal
      * paddings of the original structure */
diff --git a/server/protocol.c b/server/protocol.c
index 97d3d4f98a8..3d74c5b3058 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -704,13 +704,15 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
         }
     } while ((len <= 0) && (--num_blank_lines >= 0));
 
+    /* Set r->request_time before any logging, mod_unique_id needs it. */
+    r->request_time = apr_time_now();
+
     if (APLOGrtrace5(r)) {
         ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,
                       "Request received from client: %s",
                       ap_escape_logitem(r->pool, r->the_request));
     }
 
-    r->request_time = apr_time_now();
     return 1;
 }