feat(http) implement 'postpone_rewrite' + 'postpone_access' directives

Our module *must* run after the Lua module. When the Lua module postpones its handlers, we postpone ours as well.
Kong · Apr 17, 2024 · 1d4534d · 1d4534d
1 parent 473a670
commit 1d4534d
Show file tree

Hide file tree

Showing 13 changed files with 754 additions and 68 deletions.
diff --git a/docs/DIRECTIVES.md b/docs/DIRECTIVES.md
@@ -26,6 +26,8 @@ By alphabetical order:
 - [tls_verify_cert](#tls_verify_cert)
 - [tls_verify_host](#tls_verify_host)
 - [wasm_call](#wasm_call)
+- [wasm_postpone_access](#wasm_postpone_access)
+- [wasm_postpone_rewrite](#wasm_postpone_rewrite)
 - [wasm_response_body_buffers](#wasm_response_body_buffers)
 - [wasm_socket_buffer_reuse](#wasm_socket_buffer_reuse)
 - [wasm_socket_buffer_size](#wasm_socket_buffer_size)
@@ -67,6 +69,8 @@ By context:
     - [proxy_wasm_request_headers_in_access](#proxy_wasm_request_headers_in_access)
     - [resolver_add](#resolver_add)
     - [wasm_call](#wasm_call)
+    - [wasm_postpone_access](#wasm_postpone_access)
+    - [wasm_postpone_rewrite](#wasm_postpone_rewrite)
     - [wasm_response_body_buffers](#wasm_response_body_buffers)
     - [wasm_socket_buffer_reuse](#wasm_socket_buffer_reuse)
     - [wasm_socket_buffer_size](#wasm_socket_buffer_size)
@@ -760,6 +764,62 @@ return `HTTP 500`.
 
 [Back to TOC](#directives)
 
+wasm_postpone_access
+--------------------
+
+**usage**    | `wasm_postpone_access <on\|off>;`
+------------:|:----------------------------------------------------------------
+**contexts** | `http{}`, `server{}`, `location{}`
+**default**  | `off`
+**example**  | `wasm_postpone_access on;`
+
+Enable the postponing of the Wasm module access phase handler.
+
+If enabled, this module's access handler will be postponed to the end of the
+access phase (i.e. after all other modules access handlers). This is mostly
+useful in OpenResty builds when wanting filter chains to run after
+`access_by_lua` blocks.
+
+If disabled, this module's access handler will run as per the order configured
+during Nginx/OpenResty compilation.
+
+> Notes
+
+This directive is auto-enabled in OpenResty builds when postponing of the Lua
+access phase is enabled (this is the default behavior, see
+[access_by_lua_no_postpone](https://github.com/openresty/lua-nginx-module?tab=readme-ov-file#access_by_lua_no_postpone)).
+This can be disabled by explicitly specifying `wasm_postpone_access off;`.
+
+[Back to TOC](#directives)
+
+wasm_postpone_rewrite
+---------------------
+
+**usage**    | `wasm_postpone_rewrite <on\|off>;`
+------------:|:----------------------------------------------------------------
+**contexts** | `http{}`, `server{}`, `location{}`
+**default**  | `off`
+**example**  | `wasm_postpone_rewrite on;`
+
+Enable the postponing of the Wasm module rewrite phase handler.
+
+If enabled, this module's rewrite handler will be postponed to the end of the
+rewrite phase (i.e. after all other modules rewrite handlers). This is mostly
+useful in OpenResty builds when wanting filter chains to run after
+`rewrite_by_lua` blocks.
+
+If disabled, this module's rewrite handler will run as per the order configured
+during Nginx/OpenResty compilation.
+
+> Notes
+
+This directive is auto-enabled in OpenResty builds when postponing of the Lua
+rewrite phase is enabled (this is the default behavior, see
+[rewrite_by_lua_no_postpone](https://github.com/openresty/lua-nginx-module?tab=readme-ov-file#rewrite_by_lua_no_postpone)).
+This can be disabled by explicitly specifying `wasm_postpone_rewrite off;`.
+
+[Back to TOC](#directives)
+
 wasm_response_body_buffers
 --------------------------
 

diff --git a/src/http/ngx_http_wasm.h b/src/http/ngx_http_wasm.h
@@ -97,6 +97,9 @@ typedef struct {
     ngx_flag_t                         pwm_req_headers_in_access;
     ngx_flag_t                         pwm_lua_resolver;
 
+    ngx_flag_t                         postpone_rewrite;
+    ngx_flag_t                         postpone_access;
+
     ngx_queue_t                        q;                      /* main_conf */
 } ngx_http_wasm_loc_conf_t;
 

diff --git a/src/http/ngx_http_wasm_module.c b/src/http/ngx_http_wasm_module.c
@@ -8,6 +8,8 @@
 #if (NGX_WASM_LUA)
 #include <ngx_wasm_lua.h>
 #include <ngx_http_lua_util.h>
+#include <ngx_http_lua_rewriteby.h>
+#include <ngx_http_lua_accessby.h>
 #endif
 
 
@@ -206,6 +208,20 @@ static ngx_command_t  ngx_http_wasm_module_cmds[] = {
       offsetof(ngx_http_wasm_loc_conf_t, pwm_lua_resolver),
       NULL },
 
+    { ngx_string("wasm_postpone_rewrite"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_LOC_CONF_OFFSET,
+      offsetof(ngx_http_wasm_loc_conf_t, postpone_rewrite),
+      NULL },
+
+    { ngx_string("wasm_postpone_access"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_LOC_CONF_OFFSET,
+      offsetof(ngx_http_wasm_loc_conf_t, postpone_access),
+      NULL },
+
       ngx_null_command
 };
 
@@ -276,6 +292,18 @@ ngx_http_wasm_create_main_conf(ngx_conf_t *cf)
     ngx_proxy_wasm_properties_init(cf);
     ngx_proxy_wasm_root_init(&mcf->pwroot, cf->pool);
 
+#if (NGX_WASM_DYNAMIC_MODULE)
+    /**
+     * Ensure ngx_lua rewrite/access handlers are executed before
+     * ngx_wasm rewrite/access.
+     *
+     * Mimic static cycle order if necessary:
+     *   1. ngx_http_lua_module rewrite/access handlers
+     *   2. ngx_wasm_core_module rewrite/access handlers
+     */
+    swap_modules_if_needed(cf, "ngx_http_lua_module", "ngx_http_wasm_module");
+#endif
+
     return mcf;
 }
 
@@ -313,6 +341,8 @@ ngx_http_wasm_create_loc_conf(ngx_conf_t *cf)
     loc->socket_buffer_reuse = NGX_CONF_UNSET;
     loc->pwm_req_headers_in_access = NGX_CONF_UNSET;
     loc->pwm_lua_resolver = NGX_CONF_UNSET;
+    loc->postpone_rewrite = NGX_CONF_UNSET;
+    loc->postpone_access = NGX_CONF_UNSET;
 
     if (ngx_wasm_main_vm(cf->cycle)) {
         loc->plan = ngx_wasm_ops_plan_new(cf->pool, &ngx_http_wasm_subsystem);
@@ -375,6 +405,12 @@ ngx_http_wasm_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
     ngx_conf_merge_value(conf->pwm_lua_resolver,
                          prev->pwm_lua_resolver, 0);
 
+    ngx_conf_merge_value(conf->postpone_rewrite,
+                         prev->postpone_rewrite, NGX_CONF_UNSET);
+
+    ngx_conf_merge_value(conf->postpone_access,
+                         prev->postpone_access, NGX_CONF_UNSET);
+
     if (conf->plan && !conf->plan->populated) {
         conf->plan = prev->plan;
     }
@@ -665,6 +701,49 @@ ngx_http_wasm_rewrite_handler(ngx_http_request_t *r)
 {
     ngx_int_t                 rc;
     ngx_http_wasm_req_ctx_t  *rctx;
+    static unsigned           pcheck = 0;
+
+    ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                   "wasm rewrite handler, uri:\"%V\" c:%ud",
+                   &r->uri, r->main->count);
+
+    if (!pcheck) {
+        ngx_http_phase_handler_t    tmp;
+        ngx_http_phase_handler_t   *ph, *cur_ph, *last_ph;
+        ngx_http_core_main_conf_t  *cmcf;
+        ngx_http_wasm_loc_conf_t   *loc;
+        unsigned                    pset, p;
+
+        pcheck = 1;  /* postpone check done */
+
+        cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
+        loc = ngx_http_get_module_loc_conf(r, ngx_http_wasm_module);
+        pset = loc->postpone_rewrite != NGX_CONF_UNSET;
+        p = pset && loc->postpone_rewrite;
+
+        ph = cmcf->phase_engine.handlers;
+        cur_ph = &ph[r->phase_handler];
+        last_ph = &ph[cur_ph->next - 1];
+
+        if ((p
+#if (NGX_WASM_LUA)
+             || (last_ph->handler == ngx_http_lua_rewrite_handler
+                 && (!pset || pset && p))  /* && "not explicitly disabled" */
+#endif
+            ) && cur_ph < last_ph)
+        {
+            dd("postponing the rewrite handler...");
+
+            tmp = *cur_ph;
+            ngx_memmove(cur_ph, cur_ph + 1,
+                        (last_ph - cur_ph) * sizeof(ngx_http_phase_handler_t));
+            *last_ph = tmp;
+
+            r->phase_handler--;  /* re-enter the current phase_handler */
+
+            return NGX_DECLINED;
+        }
+    }
 
     dd("enter");
 
@@ -748,6 +827,49 @@ ngx_http_wasm_access_handler(ngx_http_request_t *r)
 {
     ngx_int_t                 rc;
     ngx_http_wasm_req_ctx_t  *rctx;
+    static unsigned           pcheck = 0;
+
+    ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                   "wasm access handler, uri:\"%V\" c:%ud",
+                   &r->uri, r->main->count);
+
+    if (!pcheck) {
+        ngx_http_phase_handler_t    tmp;
+        ngx_http_phase_handler_t   *ph, *cur_ph, *last_ph;
+        ngx_http_core_main_conf_t  *cmcf;
+        ngx_http_wasm_loc_conf_t   *loc;
+        unsigned                    pset, p;
+
+        pcheck = 1;  /* postpone check done */
+
+        cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
+        loc = ngx_http_get_module_loc_conf(r, ngx_http_wasm_module);
+        pset = loc->postpone_access != NGX_CONF_UNSET;
+        p = pset && loc->postpone_access;
+
+        ph = cmcf->phase_engine.handlers;
+        cur_ph = &ph[r->phase_handler];
+        last_ph = &ph[cur_ph->next - 2];  /* -2: also skip the post_access handler */
+
+        if ((p
+#if (NGX_WASM_LUA)
+             || (last_ph->handler == ngx_http_lua_access_handler
+                 && (!pset || pset && p))  /* && "not explicitly disabled" */
+#endif
+            ) && cur_ph < last_ph)
+        {
+            dd("postponing the access handler...");
+
+            tmp = *cur_ph;
+            ngx_memmove(cur_ph, cur_ph + 1,
+                        (last_ph - cur_ph) * sizeof(ngx_http_phase_handler_t));
+            *last_ph = tmp;
+
+            r->phase_handler--;  /* re-enter the current phase_handler */
+
+            return NGX_DECLINED;
+        }
+    }
 
     dd("enter");
 

diff --git a/src/wasm/ngx_wasm.h b/src/wasm/ngx_wasm.h
@@ -97,6 +97,7 @@ typedef struct {
 
     ngx_resolver_t                    *resolver;
     ngx_resolver_t                    *user_resolver;
+
     ngx_flag_t                         pwm_lua_resolver;
 } ngx_wasm_core_conf_t;
 
@@ -120,6 +121,9 @@ ngx_msec_t ngx_wasm_monotonic_time();
 void ngx_wasm_wall_time(void *rtime);
 void ngx_wasm_log_error(ngx_uint_t level, ngx_log_t *log, ngx_err_t err,
     const char *fmt, ...);
+#if (NGX_WASM_DYNAMIC_MODULE && (NGX_WASM_LUA || NGX_WASM_HTTP))
+void swap_modules_if_needed(ngx_conf_t *cf, const char *m1, const char *m2);
+#endif
 
 /* directives */
 char *ngx_wasm_core_wasmtime_block(ngx_conf_t *cf, ngx_command_t *cmd,

diff --git a/src/wasm/ngx_wasm_core_module.c b/src/wasm/ngx_wasm_core_module.c
@@ -260,46 +260,6 @@ ngx_wasm_core_cleanup_pool(void *data)
 }
 
 
-#if (NGX_WASM_DYNAMIC_MODULE && (NGX_WASM_LUA || NGX_WASM_HTTP))
-static unsigned
-get_module_index(ngx_cycle_t *cycle, const char *module_name, ngx_uint_t *out)
-{
-    size_t  i;
-
-    for (i = 0; cycle->modules[i]; i++) {
-        if (ngx_str_eq(cycle->modules[i]->name, -1, module_name, -1)) {
-            *out = i;
-            return 1;
-        }
-    }
-
-    return 0;
-}
-
-
-static void
-swap_modules_if_needed(ngx_conf_t *cf, const char *m1, const char *m2)
-{
-    ngx_uint_t     m1_idx, m2_idx;
-    ngx_cycle_t   *cycle = cf->cycle;
-    ngx_module_t  *tmp;
-
-    if (get_module_index(cycle, m1, &m1_idx)
-        && get_module_index(cycle, m2, &m2_idx)
-        && m1_idx < m2_idx)
-    {
-        ngx_wasm_log_error(NGX_LOG_NOTICE, cf->log, 0,
-                           "swapping modules: \"%s\" (index: %l) and \"%s\" "
-                           "(index: %l)", m1, m1_idx, m2, m2_idx);
-
-        tmp = cycle->modules[m1_idx];
-        cycle->modules[m1_idx] = cycle->modules[m2_idx];
-        cycle->modules[m2_idx] = tmp;
-    }
-}
-#endif
-
-
 static void *
 ngx_wasm_core_create_conf(ngx_conf_t *cf)
 {
@@ -396,6 +356,7 @@ ngx_wasm_core_create_conf(ngx_conf_t *cf)
     wcf->connect_timeout = NGX_CONF_UNSET_MSEC;
     wcf->send_timeout = NGX_CONF_UNSET_MSEC;
     wcf->recv_timeout = NGX_CONF_UNSET_MSEC;
+
     wcf->pwm_lua_resolver = NGX_CONF_UNSET;
 
     wcf->socket_buffer_size = NGX_CONF_UNSET_SIZE;

diff --git a/src/wasm/ngx_wasm_util.c b/src/wasm/ngx_wasm_util.c
@@ -521,3 +521,43 @@ ngx_wasm_log_error(ngx_uint_t level, ngx_log_t *log, ngx_err_t err,
 
     ngx_log_error_core(level, log, err, "[wasm] %*s", p - errstr, errstr);
 }
+
+
+#if (NGX_WASM_DYNAMIC_MODULE && (NGX_WASM_LUA || NGX_WASM_HTTP))
+static unsigned
+get_module_index(ngx_cycle_t *cycle, const char *module_name, ngx_uint_t *out)
+{
+    size_t  i;
+
+    for (i = 0; cycle->modules[i]; i++) {
+        if (ngx_str_eq(cycle->modules[i]->name, -1, module_name, -1)) {
+            *out = i;
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
+
+void
+swap_modules_if_needed(ngx_conf_t *cf, const char *m1, const char *m2)
+{
+    ngx_uint_t     m1_idx, m2_idx;
+    ngx_cycle_t   *cycle = cf->cycle;
+    ngx_module_t  *tmp;
+
+    if (get_module_index(cycle, m1, &m1_idx)
+        && get_module_index(cycle, m2, &m2_idx)
+        && m1_idx < m2_idx)
+    {
+        ngx_wasm_log_error(NGX_LOG_NOTICE, cf->log, 0,
+                           "swapping modules: \"%s\" (index: %l) and \"%s\" "
+                           "(index: %l)", m1, m1_idx, m2, m2_idx);
+
+        tmp = cycle->modules[m1_idx];
+        cycle->modules[m1_idx] = cycle->modules[m2_idx];
+        cycle->modules[m2_idx] = tmp;
+    }
+}
+#endif