nginxinc / kubernetes-ingress

NGINX and NGINX Plus Ingress Controllers for Kubernetes
https://docs.nginx.com/nginx-ingress-controller
Apache License 2.0
4.58k stars 1.96k forks source link

client-max-body-size is not always enforced when applied to a virtual server upstream #5859

Open kate-osborn opened 1 week ago

kate-osborn commented 1 week ago

Describe the bug The client-max-body-size set on a virtual server upstream is not enforced when the following is true: (1) the directive is set in an internal location block, and (2) the value is greater than the default value of 1m for client_max_body_size.

To Reproduce Steps to reproduce the behavior:

  1. Deploy the ingress controller
  2. Follow the Advanced Routing example but add client_max_body_size to the tea-post upstream. Set it to "2m".
apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: cafe
spec:
  host: cafe.example.com
  upstreams:
  - name: tea-post
    service: tea-post-svc
    port: 80
    client-max-body-size: "2m"
  - name: tea
    service: tea-svc
    port: 80
  - name: coffee-v1
    service: coffee-v1-svc
    port: 80
  - name: coffee-v2
    service: coffee-v2-svc
    port: 80
  routes:
  - path: /tea
    matches:
    - conditions:
      - variable: $request_method
        value: POST
      action:
        pass: tea-post
    action:
      pass: tea
  - path: /coffee
    matches:
    - conditions:
      - cookie: version
        value: v2
      action:
        pass: coffee-v2
    action:
      pass: coffee-v1
  1. Send a POST request to /tea with a payload > 1m Screenshot 2024-06-25 at 3 00 58 PM

Expected behavior The client-max-body-size for the tea-post upstream should be enforced instead of the default client-max-body-size.

Your environment

Additional context This bug was initially found in NGINX Gateway Fabric and may apply to more than just the client_max_body_size directive. The underlying issue is that nginx checks the body size of the request twice -- once in the external location block /tea and again in the internal location block that actually proxies the request to tea-post. If the request body exceeds the max body size in the external location block, a 413 is returned without nginx ever redirecting the request to the internal location block with the greater client_max_body_size. Even if client_max_body_size is not specified in the external location block, nginx will still check that the request body is under the default value of "1m". This is why this bug only applies when the internal client_max_body_size is greater than "1m".

Related: https://github.com/nginxinc/nginx-gateway-fabric/issues/2079 and https://github.com/nginxinc/nginx-gateway-fabric/issues/2105

github-actions[bot] commented 1 week ago

Hi @kate-osborn thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this :slightly_smiling_face:

Cheers!

haywoodsh commented 6 days ago

This seems to be relevant to #5317