Skip to content

add AddResponseHeadersIfNotPresentGatewayFilterFactory#3756

Closed
JoeCqupt wants to merge 4 commits intospring-cloud:mainfrom
JoeCqupt:feat1
Closed

add AddResponseHeadersIfNotPresentGatewayFilterFactory#3756
JoeCqupt wants to merge 4 commits intospring-cloud:mainfrom
JoeCqupt:feat1

Conversation

@JoeCqupt
Copy link
Copy Markdown
Contributor

@JoeCqupt JoeCqupt commented Apr 10, 2025

Signed-off-by: joecqupt <[email protected]>
@JoeCqupt
Copy link
Copy Markdown
Contributor Author

PTAL @spencergibb @ryanjbaxter



This listing adds 2 headers `X-Response-Color-1:blue` and `X-Response-Color-2:green` to the downstream response's headers for all matching requests.
This is similar to how `AddResponseHeader` works, but unlike `AddResponseHeader` it will do it only if the header is not already there.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be useful to also add the option to override the header if it is present? The default could be to do nothing if the header is there but we could add a configuration option to allow the override behavior

cc: @spencergibb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I take back my comment here, I think we should add the option to override the header or not to AddResponseHeader

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not resolved...

Copy link
Copy Markdown
Contributor Author

@JoeCqupt JoeCqupt May 15, 2025

Choose a reason for hiding this comment

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

We should not add 'ResponseHeadersIfNotPresentGatewayFilter'. Instead, we should modify 'ResponseHeaderGatewayFilter' to support the override option.

You mean this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes exactly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants