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

fix(pdk): response.set_header support header argument with table array of string #12164

Merged
merged 13 commits into from
Dec 25, 2023

Conversation

oowl
Copy link
Member

@oowl oowl commented Dec 6, 2023

Summary

  transform_headers(conf, kong.response.get_headers())
  -> ngx.resp.get_headers(max_headers)

In Openresty, if we set the response header in the access(or rewrite) phase, it will add headers to r->headers_out, and continue to other logic, and then real upstream processing and return headers to Openresty C logic, Openresty will add these response headers (return from real upstream) to r->headers_out, the default logic can not overwrite existing header which set by ngx.header["bala"] = "ball", So if we set the ngx.header["X-ratelimit-limit"] = 10 header in access phase, but real upstream also return “X-ratelimit-limit" header, The Openresty will store these two headers in r->headers_out, And iterate r->headers_out all response header to downstream, But for ngx.resp.get_headers APi, it will return table(not string) which contains two X-ratelimit-limit headers value.

https://github.com/openresty/lua-resty-core/blob/50f73790b6bb1d83f52cb3b4ec7e4ab7f649ab32/lib/resty/core/response.lua#L231

https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_headers.c#L1043

https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_headers_out.c#L552

https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_headers_out.c#L140

So for Kong, if the response header has two same headers (one header set by ngx.req.header), it will merge two headers to a table returned by ngx.resp.get_headers()

In response-header-transformer, it will cause a type error, this fix tries to let response.set_header support the header argument with a table array of string
https://github.com/Kong/kong/blob/master/kong/plugins/response-transformer/header_transformer.lua#L99

https://github.com/Kong/kong/pull/12164/files#diff-c9c2f298c974df55f211340f437466cf70d690dde66ae40dfaf7e1a3c0e3c859R103

Checklist

Issue reference

FTI-5585

@oowl oowl changed the title fix(response-transformer): fix response transformer header rename err… fix(response-transformer): fix response transformer header rename error trace when the same header was set twice Dec 19, 2023
@pull-request-size pull-request-size bot added size/M and removed size/S labels Dec 20, 2023
@oowl oowl changed the title fix(response-transformer): fix response transformer header rename error trace when the same header was set twice fix(pdk): response.set_header support header arguement with table array of string Dec 20, 2023
@oowl oowl changed the title fix(pdk): response.set_header support header arguement with table array of string fix(pdk): response.set_header support header argument with table array of string Dec 20, 2023
Copy link
Member

@windmgc windmgc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Copy link
Contributor

@ms2008 ms2008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kong/pdk/private/checks.lua Outdated Show resolved Hide resolved
t/01-pdk/08-response/05-set_header.t Outdated Show resolved Hide resolved
@oowl oowl force-pushed the fix/response-transformer-header branch from 8994edb to a47bc6f Compare December 25, 2023 03:48
@windmgc windmgc merged commit 2b99ee7 into master Dec 25, 2023
26 checks passed
@windmgc windmgc deleted the fix/response-transformer-header branch December 25, 2023 04:33
@windmgc windmgc added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Dec 25, 2023
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-12164-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-12164-to-master-to-upstream
git checkout -b cherry-pick-12164-to-master-to-upstream
ancref=$(git merge-base 6fe681348bb0a58efbbbc5f2a6ef57828ed61667 a47bc6f43cc9162e63ca498b2d476826840dc2af)
git cherry-pick -x $ancref..a47bc6f43cc9162e63ca498b2d476826840dc2af

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/pdk plugins/response-transformer size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants