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

[wip/proposal] fix(pdk): service.response.get_headers behavior #13827

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Commits on Nov 3, 2024

  1. fix(pdk): service.response.get_headers behavior

    This commit attempts to fix several issues in the
    `kong.service.response.get_headers()` module.
    
    Issues
    ======
    
    1. Said method must only return response headers set by the upstream
       service. However, since its introcution, it is broken; in the
       original implementation, it added an `__index` metamethod to the full headers
       table (both Kong and upstream headers) to ensure a given header being indexed
       was found in a `ngx.var.upstream_http_<header_name> variable (ie, it was
       set by the upstream). However, it responded with the table itself, which
       contained the headers, so the `__index` metamethod would never be called.
       Summarizing, since its introduction, the method has been returning
       both Kong and upstream headers. Tests were also broken due to a
       case-sensitiveness issue (implementation described above bypased the
       original headers table metatable, effectively breaking
       case-insensitiveness implemented by the `ngx.resp.get_headers` API).
    
    2. Buffered proxying, later added, introduced new logic to handle
       buffered headers. This logic corretly implemented the metamethod by
       using a so-called "proxy table"; this is an empty table to which the
       `__index` metamethod is added. Since it's empty, `__index` is
       correctly invoked and ensures only upstream headers are accessible
       (as described by 1.). However, being empty it does not allow the
       headers table to be iterated on, leading to inconsistent and
       unexpected behavior -- a source of bug reports (e.g., #11546, KAG-4866,
       to name two).
    
    What does it change?
    ====================
    
    1. Standardize the interface: both buffered and unbuffered respond
       with an iterable table.
    2. Add more tests, for both buffered and unbuffered modes
        - test that returned table is indeed iterable
        - test that returned table only has upstream headers
    
    Caveats
    =======
    
    - Buffered proxying already reads all headers; adding additional logic
      to limit by `max_headers` arguably does not make sense.
    - Calls to this method will always respond with less than `max_headers`,
      as the call to `ngx.resp.get_headers` includes both Kong and upstream
      headers and we filter out non-upstream headers after said call. The
      ideal solution would use a native API that reads upstream only headers
      directly, and has a similar interface as `ngx.resp.get_headers`,
      taking a `max_headers` argument.
    gszr committed Nov 3, 2024
    Configuration menu
    Copy the full SHA
    659bb66 View commit details
    Browse the repository at this point in the history