diff --git a/docs/changelog/index.md b/docs/changelog/index.md index b4e6fb6b..fae023e6 100644 --- a/docs/changelog/index.md +++ b/docs/changelog/index.md @@ -18,6 +18,9 @@ sidebar: false ## [Unreleased] +### 🛠 修复 +- 修复源站错误页「仅针对 GET 请求」未生效：`error_page` 内部重定向会把请求方法改写成 GET，导致内部 Lua 无法识别 POST/PUT 等原始方法、仍返回自定义错误页；现改为命名 location（`@__openflare_origin_error`）承载错误页，保留原始请求方法与错误状态码，非 GET 请求不再返回自定义错误页。 + ## [v3.5.1] - 2026-08-09 ### 新增 diff --git a/pkg/render/openresty/origin_error_page.go b/pkg/render/openresty/origin_error_page.go index 9b4e7f16..4bcf7c6e 100644 --- a/pkg/render/openresty/origin_error_page.go +++ b/pkg/render/openresty/origin_error_page.go @@ -10,8 +10,12 @@ const ( // OriginErrorPageSupportPath is the SupportFile path for the origin error HTML template. OriginErrorPageSupportPath = "error_pages/origin_error.html.tmpl" - // OriginErrorPageInternalLocation is the internal nginx location that serves the error body. - OriginErrorPageInternalLocation = "/__openflare_origin_error" + // 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 = "@__openflare_origin_error" defaultOriginErrorPageStatusTag = "500-599" ) @@ -137,13 +141,13 @@ func renderOriginErrorPageIntercept(cfg ConfigSnapshot) string { return " proxy_intercept_errors on;\n" } -// renderOriginErrorPageServerBits emits server-level error_page + internal location. +// renderOriginErrorPageServerBits emits server-level error_page + named error location. // Returns empty string when disabled, expand fails, or no codes remain. // -// IMPORTANT: do NOT use `error_page CODE = /uri` (equals without response code). +// 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 internal redirect. +// Without `=`, nginx keeps the original error status for the redirect. func renderOriginErrorPageServerBits(cfg ConfigSnapshot) string { if !cfg.OriginErrorPageEnabled { return "" @@ -164,21 +168,22 @@ func renderOriginErrorPageServerBits(cfg ConfigSnapshot) string { } func renderOriginErrorPageInternalLocation(getOnly bool) string { - // Resolve status from $status (set by error_page internal redirect), then + // 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. // // Note: fmt.Sprintf is used only for the path placeholders; Lua `%` must be // written as `%%` so Sprintf does not treat them as format verbs. // - // When getOnly is true, non-GET that still hit this location (e.g. nginx-local - // 502 without upstream body) exit with the original status and no HTML body. + // 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" } - return fmt.Sprintf(` location = %s { - internal; + return fmt.Sprintf(` location %s { default_type text/html; charset utf-8; content_by_lua_block { diff --git a/pkg/render/openresty/origin_error_page_test.go b/pkg/render/openresty/origin_error_page_test.go index 9466061d..af9fdd2a 100644 --- a/pkg/render/openresty/origin_error_page_test.go +++ b/pkg/render/openresty/origin_error_page_test.go @@ -24,22 +24,27 @@ func TestRenderOriginErrorPageEnabled(t *testing.T) { if !strings.Contains(out, "proxy_intercept_errors on") { t.Fatal("missing intercept") } - if !strings.Contains(out, "error_page") || !strings.Contains(out, "/__openflare_origin_error") { + if !strings.Contains(out, "error_page") || !strings.Contains(out, "@__openflare_origin_error") { t.Fatal("missing error_page") } if !strings.Contains(out, "error_page 500") { t.Fatalf("expected expanded status codes in error_page, got:\n%s", out) } - // Must NOT use `error_page … = /uri` (adopts error-URI status → often 200). - // `location = /path` is unrelated and expected. + // Must NOT use `error_page … = @name` (adopts error-URI status → often 200). for _, line := range strings.Split(out, "\n") { trimmed := strings.TrimSpace(line) if strings.HasPrefix(trimmed, "error_page ") && strings.Contains(trimmed, " = ") { t.Fatalf("error_page must not use '=' form, got: %s", trimmed) } } - if !strings.Contains(out, "error_page ") || !strings.Contains(out, " /__openflare_origin_error;") { - t.Fatal("error_page must redirect to internal location without '='") + if !strings.Contains(out, "error_page ") || !strings.Contains(out, " @__openflare_origin_error;") { + t.Fatal("error_page must redirect to the named error location without '='") + } + if !strings.Contains(out, "location @__openflare_origin_error {") { + t.Fatal("error location must be a named location (@...) that preserves the request method") + } + if strings.Contains(out, "location = /__openflare_origin_error") { + t.Fatal("error location must NOT be a URI internal redirect (error_page URI redirects rewrite the method to GET, breaking the get_only gate)") } if !strings.Contains(out, "resolve_error_status") || !strings.Contains(out, "ngx.status = code") { t.Fatal("internal location must resolve and set ngx.status to the original error code") @@ -103,6 +108,16 @@ func TestRenderOriginErrorPageGetOnly(t *testing.T) { 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)") + } } func TestRenderOriginErrorPageDisabled(t *testing.T) { @@ -121,8 +136,8 @@ func TestRenderOriginErrorPageDisabled(t *testing.T) { if strings.Contains(out, "proxy_intercept_errors") { t.Fatal("should not intercept when disabled") } - if strings.Contains(out, "/__openflare_origin_error") { - t.Fatal("should not emit internal error location when disabled") + if strings.Contains(out, "@__openflare_origin_error") { + t.Fatal("should not emit error location when disabled") } res, err := Render(doc, nil) if err != nil { @@ -199,7 +214,7 @@ func TestRenderOriginErrorPageCustomHTMLInSupportFile(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(out, "error_page 502 /__openflare_origin_error;") { + if !strings.Contains(out, "error_page 502 @__openflare_origin_error;") { t.Fatalf("expected single 502 error_page without '=', got:\n%s", out) } } @@ -227,7 +242,7 @@ func TestRenderOriginErrorPageSkipsPagesRoutes(t *testing.T) { if strings.Contains(out, "proxy_intercept_errors") { t.Fatal("pages routes must not get proxy_intercept_errors") } - if strings.Contains(out, "/__openflare_origin_error") { + if strings.Contains(out, "@__openflare_origin_error") { t.Fatal("pages routes must not get origin error location") } }