From f1577bf092715bfd9b72437094d34e9965464e83 Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 9 Aug 2026 19:38:44 +0800 Subject: [PATCH] =?UTF-8?q?fix(openresty):=20=E4=BF=AE=E5=A4=8D=E6=BA=90?= =?UTF-8?q?=E7=AB=99=E9=94=99=E8=AF=AF=E9=A1=B5=E3=80=8C=E4=BB=85=E9=92=88?= =?UTF-8?q?=E5=AF=B9=20GET=20=E8=AF=B7=E6=B1=82=E3=80=8D=E8=A6=86=E7=9B=96?= =?UTF-8?q?=E9=9D=9E=20GET=20=E5=8E=9F=E5=A7=8B=E6=8A=A5=E9=94=99=E6=95=B0?= =?UTF-8?q?=E6=8D=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit proxy_intercept_errors 会在 Lua 判断前丢弃源站错误响应体，POST/PUT 等 请求收到 503 时被 OpenResty 自带错误页覆盖原始报错数据。现改为在代理 location 内用 Lua header/body 过滤器仅对 GET 请求替换错误页，非 GET 请求完整透传源站原始状态码与响应体；非仅 GET 模式继续使用命名 location 承载错误页。 --- docs/changelog/index.md | 2 +- docs/reference/configuration.md | 2 +- pkg/render/openresty/origin_error_page.go | 105 +++++++++++++----- .../openresty/origin_error_page_test.go | 55 +++++---- 4 files changed, 114 insertions(+), 50 deletions(-) diff --git a/docs/changelog/index.md b/docs/changelog/index.md index 203e6148..7c1c1c8a 100644 --- a/docs/changelog/index.md +++ b/docs/changelog/index.md @@ -19,7 +19,7 @@ sidebar: false ## [Unreleased] ### 🛠 修复 -- 修复源站错误页「仅针对 GET 请求」未生效：`error_page` 内部重定向会把请求方法改写成 GET，导致内部 Lua 无法识别 POST/PUT 等原始方法、仍返回自定义错误页；现改为命名 location（`@__openflare_origin_error`）承载错误页，保留原始请求方法与错误状态码，非 GET 请求不再返回自定义错误页。 +- 修复源站错误页「仅针对 GET 请求」未真正透传非 GET 响应：`proxy_intercept_errors` 会在 Lua 判断前丢弃源站错误响应体，POST/PUT 等请求收到 503 时被 OpenResty 自带错误页覆盖原始报错数据；现改为在代理层用 Lua 过滤器（`header_filter`/`body_filter`）仅对 GET 请求替换错误页，非 GET 请求完整透传源站原始状态码与响应体；非仅 GET 模式继续使用命名 location（`@__openflare_origin_error`）承载错误页，保留原始请求方法与错误状态码。 - 修复 PostgreSQL 作为日志库时节点访问日志/可观测指标/用户访问日志批量写入失败：GORM 对零值 `uint64` 主键会省略 `id` 列，而 PG 日志表 `id` 无默认值，导致持续报「null value in column id violates not-null constraint」；现于落库前为零 ID 行生成雪花 ID（与 ClickHouse 写入路径一致），并新增回归测试覆盖六张日志表。 ## [v3.5.1] - 2026-08-09 diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 1f35429e..46fb1dff 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -264,7 +264,7 @@ Server 的所有核心基础配置定义在 `config.yaml` 中，且均支持环 | 配置键 (Key) | 数据类型 | 作用说明 | 默认值 | | --- | --- | --- | --- | | `origin_error_page_enabled` | `bool` | 是否启用全局源站错误页。开启后，源站或网关返回的匹配状态码由自定义/默认 HTML 替换，**HTTP 状态码保持原值**；关闭后不生成相关指令，恢复透传。修改后需发布配置版本生效 | `true` | -| `origin_error_page_get_only` | `bool` | 是否仅对 **GET** 请求生效。开启后仅 GET 的匹配错误状态码返回自定义错误页；POST/PUT 等其它方法不返回自定义错误页（保留原始错误状态码） | `false` | +| `origin_error_page_get_only` | `bool` | 是否仅对 **GET** 请求生效。开启后仅 GET 的匹配错误状态码返回自定义错误页；POST/PUT 等其它方法**透传源站响应**（原始状态码与响应体不变） | `false` | | `origin_error_page_status_codes` | `json` | 触发错误页的状态码标签 JSON 数组。支持单码（如 `522`）与闭区间（如 `500-599`）；单码与区间两端均须在 **400–599**，且 `lo ≤ hi`。启用时展开结果不能为空 | `["500-599"]` | | `origin_error_page_html` | `string` | 错误页自定义 HTML。空字符串表示使用内置 OpenFlare 默认模板（极简白底）；支持占位符 `{{status}}`（与 HTTP 状态码一致）、`{{host}}`（请求 Host）。最大 **256 KiB**（按字节）。勿嵌入不可信第三方脚本 | 空 | diff --git a/pkg/render/openresty/origin_error_page.go b/pkg/render/openresty/origin_error_page.go index 4bcf7c6e..29b18ae5 100644 --- a/pkg/render/openresty/origin_error_page.go +++ b/pkg/render/openresty/origin_error_page.go @@ -11,12 +11,16 @@ const ( OriginErrorPageSupportPath = "error_pages/origin_error.html.tmpl" // OriginErrorPageInternalLocation is the named nginx location that serves the error body. - // Must be a NAMED location (@...), not a URI internal redirect: error_page URI redirects - // rewrite the request method to GET, so the get_only Lua check (ngx.req.get_method() ~= "GET") - // would never fire and POST/PUT would still receive the custom HTML. Named locations preserve - // the original request method and the original error status (without the `=` form). + // OriginErrorPageInternalLocation is the named nginx location that serves the error body + // for the all-methods mode (get_only disabled). Must be a NAMED location (@...), not a + // URI internal redirect: error_page URI redirects rewrite the request method to GET, so a + // method check inside the location could never distinguish POST/PUT. Named locations keep + // the original method and (without the `=` form) the original error status. + // + // When get_only is enabled this location is NOT emitted: GET-only mode replaces the body + // via Lua header/body filters inside the proxy location, so non-GET responses pass through + // with their original status and body. OriginErrorPageInternalLocation = "@__openflare_origin_error" - defaultOriginErrorPageStatusTag = "500-599" ) @@ -131,25 +135,83 @@ func renderOriginErrorPageIntercept(cfg ConfigSnapshot) string { if !cfg.OriginErrorPageEnabled { return "" } - if _, err := ExpandStatusCodeTags(effectiveOriginErrorPageStatusTags(cfg)); err != nil { + codes, err := ExpandStatusCodeTags(effectiveOriginErrorPageStatusTags(cfg)) + if err != nil || len(codes) == 0 { return "" } + if cfg.OriginErrorPageGetOnly { + // GET-only mode must NOT use proxy_intercept_errors: interception discards + // the upstream error body, so non-GET requests could never receive the + // original response (nginx would serve its own default error page instead). + // The body is replaced by Lua header/body filters that only fire for GET; + // non-GET responses pass through with status, headers and body untouched. + return renderOriginErrorPageLuaFilterBlock(codes) + } // Intercept at the proxy level for all methods. nginx does not allow // proxy_intercept_errors inside limit_except (only allow/deny are valid - // there), so GET-only is enforced in the internal error location's Lua: - // non-GET requests exit with the original status and no custom HTML. + // there), so the custom HTML is served by the named error location. return " proxy_intercept_errors on;\n" } -// renderOriginErrorPageServerBits emits server-level error_page + named error location. -// Returns empty string when disabled, expand fails, or no codes remain. +// renderOriginErrorPageLuaFilterBlock emits the GET-only body replacement inside the +// proxy location. header_filter decides whether the response should be replaced and +// reads the template once into ngx.ctx; body_filter swaps the upstream body for the +// custom HTML and forces end-of-body so remaining upstream chunks are discarded. +// Non-GET requests (or statuses outside the configured set) are never touched. +func renderOriginErrorPageLuaFilterBlock(codes []int) string { + codeList := make([]string, len(codes)) + for i, code := range codes { + codeList[i] = strconv.Itoa(code) + } + return fmt.Sprintf(` header_filter_by_lua_block { + local codes = {%s} + local function match(code) + for _, c in ipairs(codes) do + if c == code then + return true + end + end + return false + end + local status = ngx.status + if match(status) and ngx.req.get_method() == "GET" then + ngx.header.content_length = nil + ngx.header["Content-Type"] = "text/html; charset=utf-8" + local f = io.open("%s", "r") + local body = f and f:read("*a") + if f then + f:close() + end + if not body then + body = "" .. tostring(status) .. "

" .. tostring(status) .. "

" + end + body = body:gsub("{{status}}", function() return tostring(status) end) + body = body:gsub("{{host}}", function() return ngx.var.host or "" end) + ngx.ctx.openflare_error_html = body + end + } + body_filter_by_lua_block { + local html = ngx.ctx.openflare_error_html + if html then + ngx.arg[1] = html + ngx.arg[2] = true + ngx.ctx.openflare_error_html = nil + end + } +`, strings.Join(codeList, ", "), ErrorPageTmplPlaceholder) +} + +// renderOriginErrorPageServerBits emits server-level error_page + named error location +// for the all-methods mode. Returns empty string when disabled, expand fails, no codes +// remain, or get_only is enabled (GET-only mode replaces the body via Lua filters inside +// the proxy location, see renderOriginErrorPageIntercept). // // IMPORTANT: do NOT use `error_page CODE = @name` (equals without response code). // That form adopts the status returned by the error URI; content_by_lua defaults // to 200 and ngx.status is often 0, so clients saw 200 with body "{{status}}"→"0". // Without `=`, nginx keeps the original error status for the redirect. func renderOriginErrorPageServerBits(cfg ConfigSnapshot) string { - if !cfg.OriginErrorPageEnabled { + if !cfg.OriginErrorPageEnabled || cfg.OriginErrorPageGetOnly { return "" } codes, err := ExpandStatusCodeTags(effectiveOriginErrorPageStatusTags(cfg)) @@ -163,11 +225,11 @@ func renderOriginErrorPageServerBits(cfg ConfigSnapshot) string { var builder strings.Builder // No `=` — preserve original error status (502 stays 502). fmt.Fprintf(&builder, " error_page %s %s;\n", strings.Join(parts, " "), OriginErrorPageInternalLocation) - builder.WriteString(renderOriginErrorPageInternalLocation(cfg.OriginErrorPageGetOnly)) + builder.WriteString(renderOriginErrorPageInternalLocation()) return builder.String() } -func renderOriginErrorPageInternalLocation(getOnly bool) string { +func renderOriginErrorPageInternalLocation() string { // Resolve status from $status (set by error_page redirect), then // upstream_status, then ngx.status. Force ngx.status so the client receives // the real error code. Use function replacers so host/status with `%` are safe. @@ -176,18 +238,12 @@ func renderOriginErrorPageInternalLocation(getOnly bool) string { // written as `%%` so Sprintf does not treat them as format verbs. // // The location is NAMED (@...), not a URI internal redirect: URI redirects - // (location = /uri) rewrite the request method to GET, which defeats the - // get_only gate below. Named locations keep the original method, so a POST - // that reaches this location exits with the original status and no HTML body. - getOnlyLua := "false" - if getOnly { - getOnlyLua = "true" - } + // (location = /uri) rewrite the request method to GET. Named locations keep + // the original method and (without `=`) the original error status. return fmt.Sprintf(` location %s { default_type text/html; charset utf-8; content_by_lua_block { - local get_only = %s local function resolve_error_status() local code = tonumber(ngx.var.status) if code and code >= 400 then @@ -210,11 +266,6 @@ func renderOriginErrorPageInternalLocation(getOnly bool) string { local code = resolve_error_status() ngx.status = code - if get_only and ngx.req.get_method() ~= "GET" then - -- Non-GET: do not replace with HTML; exit with status only. - return ngx.exit(code) - end - local f = io.open("%s", "r") if not f then ngx.header["Content-Type"] = "text/html; charset=utf-8" @@ -232,5 +283,5 @@ func renderOriginErrorPageInternalLocation(getOnly bool) string { ngx.say(body) } } -`, OriginErrorPageInternalLocation, getOnlyLua, ErrorPageTmplPlaceholder) +`, OriginErrorPageInternalLocation, ErrorPageTmplPlaceholder) } diff --git a/pkg/render/openresty/origin_error_page_test.go b/pkg/render/openresty/origin_error_page_test.go index af9fdd2a..466c91d6 100644 --- a/pkg/render/openresty/origin_error_page_test.go +++ b/pkg/render/openresty/origin_error_page_test.go @@ -90,33 +90,46 @@ func TestRenderOriginErrorPageGetOnly(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(out, "proxy_intercept_errors on") { - t.Fatal("missing intercept on") + // Regression: GET-only must NOT intercept at the proxy level. + // proxy_intercept_errors discards the upstream error body, so non-GET requests + // would receive nginx's own default error page instead of the original + // response (this was the reported bug: POST 503 returned OpenResty's page). + if strings.Contains(out, "proxy_intercept_errors") { + t.Fatal("get_only must not emit proxy_intercept_errors (it discards the upstream body for non-GET)") + } + // The body replacement must happen in Lua filters that only fire for GET. + if !strings.Contains(out, "header_filter_by_lua_block") { + t.Fatal("get_only must emit header_filter_by_lua_block inside the proxy location") + } + if !strings.Contains(out, "body_filter_by_lua_block") { + t.Fatal("get_only must emit body_filter_by_lua_block inside the proxy location") + } + if !strings.Contains(out, `ngx.req.get_method() == "GET"`) { + t.Fatal("Lua filter must replace the body only for GET requests") + } + if !strings.Contains(out, `ngx.ctx.openflare_error_html`) { + t.Fatal("Lua filter must stash the error HTML in ngx.ctx for the body filter") + } + if !strings.Contains(out, `local codes = {500`) { + t.Fatal("Lua filter must carry the expanded status codes") + } + if !strings.Contains(out, ErrorPageTmplPlaceholder) { + t.Fatal("missing error page template placeholder") + } + // No error_page / named location machinery in GET-only mode. + if strings.Contains(out, "error_page") { + t.Fatal("get_only must not emit error_page (named-location path can only serve HTML or an empty status, never the original body)") + } + if strings.Contains(out, "@__openflare_origin_error") { + t.Fatal("get_only must not emit the named error location") } // nginx rejects proxy_intercept_errors inside limit_except (only allow/deny - // are valid there), which made the generated config fail `openresty -t` and - // caused apply rollback. GET-only must rely on the internal location's Lua. + // are valid there); GET-only must rely on Lua filters instead. if strings.Contains(out, "limit_except") { t.Fatal("get_only must not emit limit_except (proxy_intercept_errors is not allowed there)") } - if strings.Contains(out, "proxy_intercept_errors off") { - t.Fatal("get_only must not emit proxy_intercept_errors off") - } - if !strings.Contains(out, `get_only = true`) { - t.Fatal("internal location must set get_only = true") - } - if !strings.Contains(out, `ngx.req.get_method() ~= "GET"`) { - t.Fatal("internal location must skip HTML for non-GET") - } - // Regression: error_page must target the NAMED location. URI internal redirects - // (location = /uri) rewrite the request method to GET, so ngx.req.get_method() - // would always return "GET" and the get_only gate would never skip HTML for - // POST/PUT. Named locations preserve the original method. - if !strings.Contains(out, "error_page 500") || !strings.Contains(out, " @__openflare_origin_error;") { - t.Fatalf("error_page must target the named location, got:\n%s", out) - } if strings.Contains(out, "location = /__openflare_origin_error") { - t.Fatal("get_only must not use URI internal redirect (rewrites method to GET, breaking the gate)") + t.Fatal("must not use URI internal redirect (rewrites method to GET, breaking the GET gate)") } }