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.

Sign up for GitHub

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

Jump to bottom

fix(plugin): request transformer to preserve header cases #12244

Merged
merged 1 commit into from
Jan 2, 2024
Merged

fix(plugin): request transformer to preserve header cases #12244

merged 1 commit into from
Jan 2, 2024

Conversation

StarlightIbuki
Copy link
Contributor

Summary

Request-transformer used to ignore cases when renaming header. This PR makes it case-sensitive when renaming headers.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md

Issue reference

Fix KAG-2599 #11579

@StarlightIbuki StarlightIbuki force-pushed the fix/case-sensitive-transformer branch from d78ff9d to 3921b49 Compare December 25, 2023 05:51
@StarlightIbuki StarlightIbuki force-pushed the fix/case-sensitive-transformer branch from 3921b49 to bedbda6 Compare December 25, 2023 08:54
ADD-SP
ADD-SP reviewed Dec 26, 2023
View reviewed changes
Copy link
Contributor

@ADD-SP ADD-SP left a comment

Choose a reason for hiding this comment

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

Technically, I think these changes are OK.

But the plugin matches request header case-insenitive, but preserve the case of original value, I think this behavior might confuse customers.

We should change this behavior carefully.

@StarlightIbuki StarlightIbuki force-pushed the fix/case-sensitive-transformer branch from bedbda6 to fb5f420 Compare December 27, 2023 02:53
@ADD-SP ADD-SP force-pushed the fix/case-sensitive-transformer branch from fb5f420 to 47da063 Compare January 2, 2024 02:42
@ADD-SP
Copy link
Contributor

ADD-SP commented Jan 2, 2024 •
edited
Loading

Rebased on master to check if any false green on CI as we found issues on the dynamic tests scheduler.

@windmgc windmgc merged commit 3015421 into master Jan 2, 2024
24 checks passed
@windmgc windmgc deleted the fix/case-sensitive-transformer branch January 2, 2024 06:16
@StarlightIbuki StarlightIbuki added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jan 2, 2024
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

@StarlightIbuki StarlightIbuki mentioned this pull request Jan 2, 2024
Closed
1 task
@ADD-SP ADD-SP mentioned this pull request Jan 4, 2024
Closed
3 tasks
@StarlightIbuki StarlightIbuki restored the fix/case-sensitive-transformer branch January 4, 2024 08:50
@chronolaw chronolaw deleted the fix/case-sensitive-transformer branch January 5, 2024 02:24
@chronolaw
Copy link
Contributor

We should revert This PR.

StarlightIbuki added a commit that referenced this pull request Jan 5, 2024
@StarlightIbuki
Revert "fix(request-transformer): respect letter case of rename heade…
3329f74 
…rs' new names (#12244)"

This reverts commit 3015421.
windmgc pushed a commit that referenced this pull request Jan 5, 2024
@StarlightIbuki @windmgc
Revert "fix(request-transformer): respect letter case of rename heade…
c3abb6a 
…rs' new names (#12244)"

This reverts commit 3015421.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ADD-SP ADD-SP ADD-SP approved these changes

@windmgc windmgc windmgc approved these changes

Assignees

@StarlightIbuki StarlightIbuki

Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee plugins/request-transformer size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@StarlightIbuki @ADD-SP @team-gateway-bot @chronolaw @windmgc