Skip to content

Upstream: Fix handling of malformed status lines #723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ac000
Copy link
Member

@ac000 ac000 commented Jun 6, 2025

Upstream: Fix handling of malformed status lines

If an upstream returned a malformed status line, e.g.

  HTTP/1.1 404: Not Found

(Note the extraneous ':')

nginx would for HTTP 1.1 connections cause the client to hang until the
proxy timeout was reached while nginx was waiting for more headers from
the upstream that were never going to come.

The client would then see an empty reply from the server.

With HTTP 2 connections nginx would return a status of 000 to the
client. E.g.

  ...
  < HTTP/2 000
  < server: nginx/1.29.0
  < date: Fri, 06 Jun 2025 15:31:08 GMT
  <
  * Unsupported response code in HTTP response
  * Connection #0 to host localhost left intact
  curl: (1) Unsupported response code in HTTP response

Both cases would log a 499 status code to the access log. E.g.

  ::1 - - [06/Jun/2025:16:31:08 +0100] "GET / HTTP/2.0" 499 0 "-" "curl/8.11.1"

This was due to returning NGX_OK from
ngx_http_proxy_process_status_line() in ngx_http_proxy_module.c rather
than NGX_HTTP_UPSTREAM_INVALID_HEADER.

We also don't want to set r->http_version to NGX_HTTP_VERSION_9 as for
HTTP 1.1 connections this would now result in

  > GET / HTTP/1.1
  > Host: localhost:4000
  > User-Agent: curl/8.11.1
  > Accept: */*
  >
  * Request completely sent off
  * Received HTTP/0.9 when not allowed
  * closing connection #0
  curl: (1) Received HTTP/0.9 when not allowed

Which seems wrong.

With this fix both the above cases return a 502 Bad Gateway, e.g.

  > GET / HTTP/2
  > Host: localhost:4000
  > User-Agent: curl/8.11.1
  > Accept: */*
  >
  * Request completely sent off
  < HTTP/2 502
  < server: nginx/1.29.0
  < date: Fri, 06 Jun 2025 15:33:06 GMT
  < content-type: text/html
  < content-length: 157
  ...

A 502 is now logged to the access log, e.g.

  ::1 - - [06/Jun/2025:16:33:15 +0100] "GET / HTTP/1.1" 502 157 "-" "curl/8.11.1"

For good measure we remove the setting of u->state->status to
NGX_HTTP_OK which doesn't seem to have any effect here.

Closes: https://github.com/nginx/nginx/issues/378
Signed-off-by: Andrew Clayton <[email protected]>

@ac000 ac000 linked an issue Jun 6, 2025 that may be closed by this pull request
2 tasks
@ac000 ac000 marked this pull request as ready for review June 6, 2025 16:34
@ac000 ac000 force-pushed the proxy-status-fix branch from 0282a9f to c3ae342 Compare June 23, 2025 13:11
@ac000
Copy link
Member Author

ac000 commented Jun 23, 2025

  • Add missing period on subject line
  • Remove s-o-b
$ git range-diff 0282a9f4b...c3ae3426c
1:  0282a9f4b ! 1:  c3ae3426c Upstream: Fix handling of malformed status lines
    @@ Metadata
     Author: Andrew Clayton <[email protected]>
     
      ## Commit message ##
    -    Upstream: Fix handling of malformed status lines
    +    Upstream: Fix handling of malformed status lines.
     
         If an upstream returned a malformed status line, e.g.
     
    @@ Commit message
         NGX_HTTP_OK which doesn't seem to have any effect here.
     
         Closes: https://github.com/nginx/nginx/issues/378
    -    Signed-off-by: Andrew Clayton <[email protected]>
     
      ## src/http/modules/ngx_http_proxy_module.c ##
     @@ src/http/modules/ngx_http_proxy_module.c: ngx_http_proxy_process_status_line(ngx_http_request_t *r)

If an upstream returned a malformed status line, e.g.

  HTTP/1.1 404: Not Found

(Note the extraneous ':')

nginx would for HTTP 1.1 connections cause the client to hang until the
proxy timeout was reached while nginx was waiting for more headers from
the upstream that were never going to come.

The client would then see an empty reply from the server.

With HTTP 2 connections nginx would return a status of 000 to the
client. E.g.

  ...
  < HTTP/2 000
  < server: nginx/1.29.0
  < date: Fri, 06 Jun 2025 15:31:08 GMT
  <
  * Unsupported response code in HTTP response
  * Connection #0 to host localhost left intact
  curl: (1) Unsupported response code in HTTP response

Both cases would log a 499 status code to the access log. E.g.

  ::1 - - [06/Jun/2025:16:31:08 +0100] "GET / HTTP/2.0" 499 0 "-" "curl/8.11.1"

This was due to returning NGX_OK from
ngx_http_proxy_process_status_line() in ngx_http_proxy_module.c rather
than NGX_HTTP_UPSTREAM_INVALID_HEADER.

We also don't want to set r->http_version to NGX_HTTP_VERSION_9 as for
HTTP 1.1 connections this would now result in

  > GET / HTTP/1.1
  > Host: localhost:4000
  > User-Agent: curl/8.11.1
  > Accept: */*
  >
  * Request completely sent off
  * Received HTTP/0.9 when not allowed
  * closing connection #0
  curl: (1) Received HTTP/0.9 when not allowed

Which seems wrong.

With this fix both the above cases return a 502 Bad Gateway, e.g.

  > GET / HTTP/2
  > Host: localhost:4000
  > User-Agent: curl/8.11.1
  > Accept: */*
  >
  * Request completely sent off
  < HTTP/2 502
  < server: nginx/1.29.0
  < date: Fri, 06 Jun 2025 15:33:06 GMT
  < content-type: text/html
  < content-length: 157
  ...

A 502 is now logged to the access log, e.g.

  ::1 - - [06/Jun/2025:16:33:15 +0100] "GET / HTTP/1.1" 502 157 "-" "curl/8.11.1"

For good measure we remove the setting of u->state->status to
NGX_HTTP_OK which doesn't seem to have any effect here.

Closes: nginx#378
@ac000 ac000 force-pushed the proxy-status-fix branch from c3ae342 to 229d8ae Compare June 23, 2025 13:12
@ac000
Copy link
Member Author

ac000 commented Jun 23, 2025

Rebased with master

$ git range-diff c3ae3426c...229d8ae2d
-:  --------- > 1:  ea001feb1 HTTP/2: added function declaration.
-:  --------- > 2:  662c1dd2a Upstream: early hints support.
-:  --------- > 3:  ba917b136 HTTP/3: indexed field line encoding for "103 Early Hints".
-:  --------- > 4:  c370ac8a5 Use NGX_CONF_OK in some function return checks.
-:  --------- > 5:  4eaecc5e8 Use NULL instead of 0 for null pointer constant.
1:  c3ae3426c = 6:  229d8ae2d Upstream: Fix handling of malformed status lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"HTTP/2 000" response when upstream returns invalid status line
1 participant