bug: the traffic-split plugin is invalid to bind upstream via upstream_id

[Firstsawyou]

Issue description

When both the routing and traffic-split plugins use upstream_id to bind upstream, the request distribution is not accurate.

1、add route and bind plugin

The upstream_id is 1, the response data is 1980, and the upstream_id is 2 the response data is 1981.

curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "create_time":1614743731,
    "update_time":1614745342,
    "uris":[
        "/*"
    ],
    "name":"luarocks_cn",
    "methods":[
        "GET",
        "HEAD",
        "POST",
        "PUT",
        "DELETE",
        "OPTIONS",
        "PATCH"
    ],   
    "vars":[

    ],
    "plugins":{
        "proxy-rewrite":{
            "disable":false,
            "scheme":"http"
        },
        "traffic-split":{
            "disable":false,
            "rules":[
                {
                    "match":[
                        {
                            "vars":[
                                [
                                    "uri",
                                    "==",
                                    "/hello"
                                ]
                            ]
                        }
                    ],
                    "weighted_upstreams":[
                        {
                            "upstream_id":"2"
                        }
                    ]
                }
            ]
        }
    },
    "upstream_id":"1",
    "status":1
}
'

2、Test plugin

The vars rule passes:

$ curl http://127.0.0.1:9080/hello
1981

The vars rule failed：

$ curl http://127.0.0.1:9080/hello12
1980

The vars rule passes:

$ curl http://127.0.0.1:9080/hello
1981

The vars rule failed (an exception occurred)：

$ curl http://127.0.0.1:9080/hello12
1981

[tzssangglass]

"weighted_upstreams":[
                        {
                            "upstream_id":"2"
                        }
                    ]

should be

"weighted_upstreams":[
                        {
                            "upstream_id":2
                        }
                    ]

the type of upstream_id is number but not string.

by the way, it is an interesting bug
I added a debug log

core.log.warn("route.value.upstream_id: ", core.json.encode(route.value.upstream_id))

between the line 71 and the line 72

apisix/apisix/http/route.lua

Lines 70 to 73 in a97ba0d

	handler = function (api_ctx, match_opts)
	api_ctx.matched_params = nil
	api_ctx.matched_route = route
	api_ctx.curr_req_matched = match_opts.matched

I found that when the bug appeared, the upstream_id changed from 1 to 2.

I don't know what caused the change.

[tzssangglass]

I was wrong, according to the documentation, the type of upstream_id can be string or integer

[tzssangglass]

here are the reasons for the bugs I found:

1. the first time execute curl http://127.0.0.1:9080/hello, the match rule is verified, and will go to this line

   apisix/apisix/plugins/traffic-split.lua

   Line 323 in 0d40f81

   ctx.matched_route.value.upstream_id = upstream

2. then upstream_id changed from 1 to 2
3. this change modifies the ctx.matched_route table, which is not held by api_ctx, so when api_ctx is released with the end of request, matched_route still exists, I think matched_route is the radixtree_uri.lua's module-level variable, which has a lifecycle across requests.
4. on the second request, execute curl http://127.0.0.1:9080/hello12, the match rule verification fails, but matched_route.value.upstream_id has already been changed to 2` by the previous request.

[tokers]

@tzssangglass Would you like to submit a PR to solve this issue? Thanks!

[tzssangglass]

@tzssangglass Would you like to submit a PR to solve this issue? Thanks!

assigned to me

[starsz]

@tzssangglass Would you like to submit a PR to solve this issue? Thanks!

assigned to me

Thank you very much.