Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(router) preserve_host always fetches the request's Host
Summary ------- It was reported in #2825 that `preserve_host` seemed to behave wrongly upon subsequent call of a same API if that API had no `hosts`. Since matches APIs are cached, and so are their `upstream_host` values, I investigated around the LRU cache of our matches in the router. What I discovered is, considering the following API: { "name": "example", "upstream_url": "http://httpbin.org", "uris": ["/example"] } The router would try to save cycles by *not* reading the `ngx.var.http_host` variable, since no API is configured to match on a Host header. However, the Host is also part of the cache key since the introduction of the LRU cache in the router. But in the above case, considering the following two requests: GET /example HTTP/1.1 Host: domain1.com GET /example HTTP/1.1 Host: domain2.com Both matches would be cached under the **same** key (since the `req_host` argument of `find_api` is `nil` because the router does not read the Host header): GET:/example: Notice the lack of Host part after the last colon. Hence, once this API would be matched after the first request, its cached `upstream_host` would be `domain1.com`, and subsequent calls with `Host: domain2.com` would hit the same key, and yield the same `upstream_host` as the first request. The approach taken by this fix is to **always** retrieve the Host header. This choice is driven by the fact that cache keys should have as high a cardinality as possible, and to ensure we avoid such duplicate keys in the future. Changes ------- * remove `grab_header` optimization from `router.lua` * remove `grab_header` tests from unit test suite * make `find_api()`'s 3rd argument (Host header) mandatory * update related unit tests to reflect the above change * new regression test for this issue --- Fix #2824
- Loading branch information