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

Properly proceed in filter processing when request is replaced #7174

Merged
merged 7 commits into from Apr 5, 2022

Conversation

yawkat
Copy link
Member

@yawkat yawkat commented Apr 1, 2022

When a filter calls chain.proceed with a new request, that request should be used for further processing.
Replacing the request is still not fully working even with this change, so #5491 is not fully fixed. RoutingInBoundHandler still does not consider the new request, because most of the request processing happens before the filter is applied. In the test case of this patch, this manifests as the response text still being "initial" even though the request has been replaced.

When a filter calls chain.proceed with a new request, that request should be used for further processing.
Replacing the request is still not fully working even with this change, so #5491 is not fully fixed. RoutingInBoundHandler still does not consider the new request, because most of the request processing happens before the filter is applied. In the test case of this patch, this manifests as the response text still being "initial" even though the request has been replaced.
@sdelamo
Copy link
Collaborator

sdelamo commented Apr 5, 2022

When a filter calls chain.proceed with a new request, that request should be used for further processing.
Replacing the request is still not fully working even with this change, so #5491 is not fully fixed. RoutingInBoundHandler still does not consider the new request, because most of the request processing happens before the filter is applied. In the test case of this patch, this manifests as the response text still being "initial" even though the request has been replaced.

I assume that in the test scenario what should happen is that the application responds filter2. I have created such a test and annotated with @PendingFeature.

3e72e9f

@sdelamo
Copy link
Collaborator

sdelamo commented Apr 5, 2022

should we point this to 3.4.x or 3.5.x?

@sdelamo sdelamo added this to the 3.4.2 milestone Apr 5, 2022
@yawkat
Copy link
Member Author

yawkat commented Apr 5, 2022

This is a bug, it should be 3.4.x

@sonarcloud
Copy link

sonarcloud bot commented Apr 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@sdelamo sdelamo merged commit fe0bcce into 3.4.x Apr 5, 2022
@sdelamo sdelamo deleted the filter-replace-request branch April 5, 2022 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants