Re: [PR] fix(proxy-rewrite): transform header names to lower case before removal [apisix]

["Baoyuantop (via GitHub)" <gi...@apache.org>]

Baoyuantop commented on PR #13816:
URL: https://github.com/apache/apisix/pull/13816#issuecomment-6359077740

   Thanks for the PR.
   
   We tried to reproduce the reported issue (uppercase names in `proxy-rewrite` `headers.remove` being ignored) on **APISIX 3.19.0** (Docker image `apache/apisix:3.19.0`, standalone / YAML config provider) with `httpbin` as the upstream, and **could not reproduce it**.
   
   ### Environment
   - APISIX `3.19.0`, `deployment.role: data_plane`, `config_provider: yaml`
   - `enable_http2: true`
   - Upstream: httpbin `/headers` (echoes request headers)
   
   ### Route / plugin config (`apisix.yaml`)
   
   ```yaml
   routes:
     - id: 1
       uri: /case-mixed
       plugins:
         proxy-rewrite:
           uri: /headers
           headers:
             remove:
               - Authorization
               - X-Api-Test
       upstream:
         type: roundrobin
         nodes:
           "127.0.0.1:80": 1
     - id: 2
       uri: /case-lower
       plugins:
         proxy-rewrite:
           uri: /headers
           headers:
             remove:
               - authorization
               - x-api-test
       upstream:
         type: roundrobin
         nodes:
           "127.0.0.1:80": 1
     - id: 3
       uri: /no-remove
       plugins:
         proxy-rewrite:
           uri: /headers
       upstream:
         type: roundrobin
         nodes:
           "127.0.0.1:80": 1
   #END
   ```
   
   ### Requests
   
   ```bash
   # Mixed-case remove (HTTP/1.1)
   curl --http1.1 \
     -H 'Authorization: Bearer MIXED' \
     -H 'X-Api-Test: should-drop' \
     -H 'X-Keep: yes' \
     http://127.0.0.1:9080/case-mixed
   
   # Lower-case remove (HTTP/1.1)
   curl --http1.1 \
     -H 'Authorization: Bearer LOWER' \
     -H 'X-Api-Test: should-drop' \
     -H 'X-Keep: yes' \
     http://127.0.0.1:9080/case-lower
   
   # Baseline without remove
   curl --http1.1 \
     -H 'Authorization: Bearer BASE' \
     -H 'X-Api-Test: visible' \
     -H 'X-Keep: yes' \
     http://127.0.0.1:9080/no-remove
   
   # Same cases over HTTP/2
   curl --http2-prior-knowledge \
     -H 'Authorization: Bearer H2MIX' \
     -H 'X-Api-Test: should-drop' \
     -H 'X-Keep: yes' \
     http://127.0.0.1:9080/case-mixed
   
   curl --http2-prior-knowledge \
     -H 'Authorization: Bearer H2LOW' \
     -H 'X-Api-Test: should-drop' \
     -H 'X-Keep: yes' \
     http://127.0.0.1:9080/case-lower
   ```
   
   ### Observed results
   | Case | `Authorization` at upstream | `X-Api-Test` | `X-Keep` |
   |------|----------------------------|--------------|----------|
   | `/case-mixed` HTTP/1.1 | removed | removed | kept |
   | `/case-lower` HTTP/1.1 | removed | removed | kept |
   | `/no-remove` HTTP/1.1 | present | present | kept |
   | `/case-mixed` HTTP/2 | removed | removed | kept |
   | `/case-lower` HTTP/2 | removed | removed | kept |
   
   On this version, `headers.remove` with mixed-case names (`Authorization`, `X-Api-Test`) already removes the headers correctly for both HTTP/1.1 and HTTP/2. The current plugin path uses `core.request.set_header(..., nil)` / `ngx.req` without an extra `:lower()` call, and header name matching is already case-insensitive at that layer.
   
   Without a failing reproduction (or a regression test that fails on current master), we are closing this PR for now. If you have a concrete setup / version where uppercase `remove` entries are ignored, please share the exact config and we can reopen.
   
   Closing as not reproducible on APISIX 3.19.0.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org