mirror of
https://github.com/Rain-kl/OpenFlare.git
synced 2026-09-29 22:06:38 +08:00
fix(openresty): 修复源站错误页「仅针对 GET 请求」未生效
error_page 的 URI 内部重定向会把请求方法改写成 GET,导致内部 Lua 中 ngx.req.get_method() 恒为 GET,get_only 判断永不命中, POST/PUT 等请求仍返回自定义错误页。 改为命名 location(@__openflare_origin_error)承载错误页: 命名 location 保留原始请求方法与原始错误状态码,非 GET 请求 直接以原状态码退出、不再注入自定义 HTML。附带回归断言,禁止 回退到 URI 内部重定向形式。
This commit is contained in:
@@ -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
|
||||
|
||||
### 新增
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user