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: The request-id plugin may have a bug #4460

Closed
zuiyangqingzhou opened this issue Jun 22, 2021 · 7 comments · Fixed by #4479
Closed

bug: The request-id plugin may have a bug #4460

zuiyangqingzhou opened this issue Jun 22, 2021 · 7 comments · Fixed by #4479

Comments

@zuiyangqingzhou
Copy link
Contributor

Issue description

When I enable the global request-id plugin and the request-id plugin on the route at the same time, there will be a BUG

Bug report without environment information will be ignored or closed.

  • apisix version (cmd: apisix version): 2.6
  • OS (cmd: uname -a): Darwin Kernel Version 20.5.0
  • OpenResty / Nginx version (cmd: nginx -V or openresty -V): nginx version: openresty/1.19.3.2
  • etcd version, if have (cmd: run curl http://127.0.0.1:9090/v1/server_info to get the info from server-info API): etcd Version: 3.4.16
  • apisix-dashboard version, if have:
  • luarocks version, if the issue is about installation (cmd: luarocks --version):

Minimal test code / Steps to reproduce the issue

Bug report without steps to reproduce will be ignored or closed.

  1. I enable the request-id plugin on a specific route, the default header_name is X-Request-Id, and I get the following result, which looks normal
$ curl -i http://127.0.0.1:9080/hello -H 'Authorization: eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJrZXkiOiJ0ZXN0LWp3dC1kZW1vLTEiLCJleHAiOjE2MjQzNTk3MjZ9.TF2o2KngjIHMYO2CqPurPr1f2UnZVry76kN2XOyKEIU'

HTTP/1.1 200 OK
Content-Type: text/plain; charset=utf-8
Content-Length: 408
Connection: keep-alive
Date: Tue, 22 Jun 2021 02:25:37 GMT
Server: APISIX/2.6
X-Request-Id: f02ac2c2-5333-4c03-a4ba-9b81fafdcdab

# my upstream get headers from apisix
Header全部数据: map[Accept:[*/*] Authorization:[eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJrZXkiOiJ0ZXN0LWp3dC1kZW1vLTEiLCJleHAiOjE2MjQzNTk3MjZ9.TF2o2KngjIHMYO2CqPurPr1f2UnZVry76kN2XOyKEIU] User-Agent:[curl/7.64.1] X-Forwarded-For:[127.0.0.1] X-Forwarded-Host:[127.0.0.1] X-Forwarded-Port:[9080] X-Forwarded-Proto:[http] X-Real-Ip:[127.0.0.1] X-Request-Id:[f02ac2c2-5333-4c03-a4ba-9b81fafdcdab]]
  1. I enable a global request-id plug-in, header_name is X-My-Request-id, request again, it seems that there is a problem

Environment

$ curl -i http://127.0.0.1:9080/hello -H 'Authorization: eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJrZXkiOiJ0ZXN0LWp3dC1kZW1vLTEiLCJleHAiOjE2MjQzNTk3MjZ9.TF2o2KngjIHMYO2CqPurPr1f2UnZVry76kN2XOyKEIU'

HTTP/1.1 200 OK
Content-Type: text/plain; charset=utf-8
Content-Length: 463
Connection: keep-alive
Date: Tue, 22 Jun 2021 02:28:22 GMT
Server: APISIX/2.6
X-My-Request-Id: 0c547cd1-8789-4369-9bca-f185f3e20309
X-Request-Id: 0c547cd1-8789-4369-9bca-f185f3e20309

# my upstream get header from apisix
Header全部数据: map[Accept:[*/*] Authorization:[eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJrZXkiOiJ0ZXN0LWp3dC1kZW1vLTEiLCJleHAiOjE2MjQzNTk3MjZ9.TF2o2KngjIHMYO2CqPurPr1f2UnZVry76kN2XOyKEIU] User-Agent:[curl/7.64.1] X-Forwarded-For:[127.0.0.1] X-Forwarded-Host:[127.0.0.1] X-Forwarded-Port:[9080] X-Forwarded-Proto:[http] X-My-Request-Id:[cbe7a2c9-8667-4016-8298-9104b0829ab4] X-Real-Ip:[127.0.0.1] X-Request-Id:[0c547cd1-8789-4369-9bca-f185f3e20309]]

image

As you can see, the X-My-Request-Id in the header returned by the request is different from the X-My-Request-Id obtained by upstream.

What's the actual result? (including assertion message & call stack if applicable)

What's the expected result?

@tokers
Copy link
Contributor

tokers commented Jun 23, 2021

@zuiyangqingzhou What is the exact logic of your upstream? Just reflecting the request headers?

The request-id plugin just adds the id header on your demands. It won't change other things. Make sure your upstream doesn't contain its own X-My-Request-Id, as request-id only adds the id header to response if the header doesn't exist.

@zuiyangqingzhou
Copy link
Contributor Author

This is the upstream logic, it just gets the header passed from apisix

 package main

 import (
     "fmt"
     "log"
     "net/http"
 )
 func HelloServer(w http.ResponseWriter, req *http.Request) {
     _, _ = fmt.Fprintln(w,"Header全部数据:",req.Header)
 }

 func main() {
     http.HandleFunc("/hello", HelloServer)
     err := http.ListenAndServe(":9999", nil)
     if err != nil {
         log.Fatal("ListenAndServe: ", err)
     }
 }

You may not understand what I mean. What I mean is that the request-id passed by apisix to upstream is different from the request-id it returns to curl. Shouldn't this be a bug? I have a picture above you can look at it

@tzssangglass
Copy link
Member

tzssangglass commented Jun 23, 2021

@zuiyangqingzhou hi, I repeated your steps, cc @tokers

step 1: set request-id on route

curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri":"/get",
    "plugins": {
        "request-id": {
            "header_name":"X-Request-Route-Id",
            "include_in_response": true
        }
    },
    "upstream":{
        "nodes":{
            "httpbin.org:80":1
        },
        "type":"roundrobin"
    }
}'

step 2: set request-id on global_rule

curl http://127.0.0.1:9080/apisix/admin/global_rules/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "plugins": {
        "request-id": {
            "header_name":"X-My-Request-id",
            "include_in_response": true
        }
    }
}'

step 3: send request

curl -i 127.0.0.1:9080/get
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 429
Connection: keep-alive
Date: Wed, 23 Jun 2021 17:00:57 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Server: APISIX/2.6
X-My-Request-id: 2a3c2d84-7e8f-4dcd-8d85-d175f0bb9d7a
X-Request-Route-Id: 2a3c2d84-7e8f-4dcd-8d85-d175f0bb9d7a

{
  "args": {}, 
  "headers": {
    "Accept": "*/*", 
    "Host": "127.0.0.1", 
    "User-Agent": "curl/7.71.1", 
    "X-Amzn-Trace-Id": "Root=1-60d368c9-7ef6036b6b2944281ae48465", 
    "X-Forwarded-Host": "127.0.0.1", 
    "X-My-Request-Id": "8509eca0-be8c-4e1f-9186-8427569adfd1", 
    "X-Request-Route-Id": "2a3c2d84-7e8f-4dcd-8d85-d175f0bb9d7a"
  }, 
  "origin": "127.0.0.1, 58.152.81.86", 
  "url": "http://127.0.0.1/get"
}

I get the same error.

by adding the log I found that the ctx.x_request_id

if not headers[conf.header_name] then
core.response.set_header(conf.header_name, ctx.x_request_id)
is store the value generated by the request-id on the route, this is because the request-id on route is executed later than the one on global_rule.

I think it should be explicitly stated that request-id plugin should not be configured on both global_rule and route, it doesn't make much sense.

@tokers
Copy link
Contributor

tokers commented Jun 24, 2021

@zuiyangqingzhou Oh, gotcha, configuring request-id plugin twice will cause APISIX generating two UUID values.

I'd like to know your scenario that need two request-id.

@tzssangglass Though it seems that configuring two request-id plugins don't make sense, I think the request-id header in response should be consistent with the one in request headers to upstream.

@zuiyangqingzhou
Copy link
Contributor Author

@tzssangglass Though it seems that configuring two request-id plugins don't make sense, I think the request-id header in response should be consistent with the one in request headers to upstream.

Yes, it is very correct. This is what I want to express. Although the two request-ids are meaningless, it should be ensured that the request-id in the response is the same as the one passed to upstream.

@tokers
Copy link
Contributor

tokers commented Jun 24, 2021

@zuiyangqingzhou Would you like to submit a PR to fix this? Thanks!

@zuiyangqingzhou
Copy link
Contributor Author

I haven't mentioned a PR to apisix yet, I am a little nervous, but I can give it a try

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 a pull request may close this issue.

3 participants