Skip to content
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

bug: forward-auth with request_method GET causes timeouts #10927

Closed
BrandonArp opened this issue Feb 7, 2024 · 13 comments
Closed

bug: forward-auth with request_method GET causes timeouts #10927

BrandonArp opened this issue Feb 7, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@BrandonArp
Copy link

Current Behavior

The request to the forward-auth request contains a Content-Length header and the GET request has no body.

Expected Behavior

The forward-auth request has no Content-Length header.

Error Logs

apisix-1  | 2024/02/07 20:27:39 [error] 242#242: *648462 [lua] forward-auth.lua:134: phase_func(): failed to process forward auth, err: timeout, client: 172.27.0.1, server: _, request: "POST /test HTTP/1.1", host: "localhost:9080"
apisix-1  | 2024/02/07 20:27:39 [warn] 242#242: *648462 [lua] plugin.lua:1159: run_plugin(): forward-auth exits with http status code 403, client: 172.27.0.1, server: _, request: "POST /test HTTP/1.1", host: "localhost:9080"

Steps to Reproduce

  1. Run APISIX
  2. Deploy a service that can consume forward-auth requests
  3. Create a route that has forward-auth enabled, with a GET request_method
    You can use the following plugin config:
    forward-auth:
         keepalive: true
         keepalive_pool: 100
         keepalive_timeout: 2000
         request_headers:
           - Authorization
         request_method: GET
         uri: http://some_auth_service/auth```
  4. Send 2 requests to the protected route
  5. Observe that the second request will fail with a timeout

Environment

  • APISIX version 3.8.0
@BrandonArp
Copy link
Author

I believe that the issue was introduced in #10589 where some headers were added unconditionally here.

@moonming
Copy link
Member

moonming commented Feb 8, 2024

welcome PR to fix it @BrandonArp

@shreemaan-abhishek shreemaan-abhishek added the bug Something isn't working label Feb 8, 2024
@pshettyk
Copy link

I am also facing the same issue in 3.8.0. Works fine in 3.6.0.

@moonming
Copy link
Member

@shreemaan-abhishek please take a look

@shreemaan-abhishek
Copy link
Contributor

shreemaan-abhishek commented Mar 4, 2024

I have found the cause of this bug. I will fix this later.

Case 1:

  • request contains payload
  • original request method: GET
  • fw auth request method: GET

In this case, APISIX unconditionally adds the Content-Length header to the request to be sent to fw-auth.uri but the request body (to fw-auth.uri) is empty. This causes the fw-auth server to wait endlessly to read the content.

Solution:

  • add content-length header only if the fw-auth.request_method is POST.

Case 2:

  • request contains payload
  • original request method: POST
  • fw auth request method: POST

In this case, APISIX passes client-body-reader by default. (this was added to support passing large request body to fw-auth). client-body-reader reads the request body and makes it empty. But since the content-length is non zero the upstream server again waits endlessly to read the request body which is no longer available.

Solution:

  1. Use the client-body-reader only if the request body is too large. (How much is too large is unknown)
  2. If using client-body-reader store the req-body in a var before sending the request to fw-auth server. After request from fw-auth is successful, restore the request body.

@suryaprabhakark
Copy link
Contributor

Hi, I raised a PR for the fix. I faced the same issue and we are running the fixed code in our prod for sometime now, not seen any issues yet. Please let me know if i missed anything.

#11021

@motongxue
Copy link
Contributor

I have found the cause of this bug. I will fix this later.

Case 1:

  • request contains payload
  • original request method: GET
  • fw auth request method: GET

In this case, APISIX unconditionally adds the Content-Length header to the request to be sent to fw-auth.uri but the request body (to fw-auth.uri) is empty. This causes the fw-auth server to wait endlessly to read the content.

Solution:

  • add content-length header only if the fw-auth.request_method is POST.

Case 2:

  • request contains payload
  • original request method: POST
  • fw auth request method: POST

In this case, APISIX passes client-body-reader by default. (this was added to support passing large request body to fw-auth). client-body-reader reads the request body and makes it empty. But since the content-length is non zero the upstream server again waits endlessly to read the request body which is no longer available.

Solution:

  1. Use the client-body-reader only if the request body is too large. (How much is too large is unknown)
  2. If using client-body-reader store the req-body in a var before sending the request to fw-auth server. After request from fw-auth is successful, restore the request body.

Hello abhishek. I have submitted a #11023 regarding your issue, and I hope it can be of help to you. At the same time, I would greatly appreciate your assistance in reviewing and finalizing the test cases.

@OnGoingLzy
Copy link

I had the same problem with how to get the body to pass upstream correctly

@zhoujiexiong
Copy link
Contributor

I had the same problem with how to get the body to pass upstream correctly

@OnGoingLzy FYI: #11350

@masri-k
Copy link

masri-k commented Aug 21, 2024

In your roadmap, this fix of this bug will be released when? in which version?

Thank you all

@zhoujiexiong
Copy link
Contributor

In your roadmap, this fix of this bug will be released when? in which version?

Thank you all

Hi @masri-k
The bug of this issue should had been fixed by this merge #11021.
Which version you are using?

Hi @BrandonArp
If the issue was fixed, pls. close it. :D

@masri-k
Copy link

masri-k commented Aug 23, 2024

K. Thanks! using 3.9.1

@BrandonArp
Copy link
Author

Yes, this issue is fixed and was released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

9 participants