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

remove DefaultCookieDecoder #10659

Merged
merged 8 commits into from
Mar 28, 2024
Merged

remove DefaultCookieDecoder #10659

merged 8 commits into from
Mar 28, 2024

Conversation

n0tl3ss
Copy link
Member

@n0tl3ss n0tl3ss commented Mar 28, 2024

#10435 remove DefaultCookieDecoder from services because its broken for parsing request cookies

@yawkat yawkat added the type: bug Something isn't working label Mar 28, 2024
@sdelamo
Copy link
Contributor

sdelamo commented Mar 28, 2024

should not we fix it if it is broken?

@sdelamo
Copy link
Contributor

sdelamo commented Mar 28, 2024

if it is bug it should have 4.3.x as base.

@sdelamo sdelamo self-assigned this Mar 28, 2024
@yawkat
Copy link
Member

yawkat commented Mar 28, 2024

this is related to #10435 . we have to fix it properly later, this PR just fixes the precedence of the netty decoder

@n0tl3ss n0tl3ss force-pushed the remove-default-cookie-decoder branch from 536a200 to b63ab09 Compare March 28, 2024 10:15
@n0tl3ss n0tl3ss changed the base branch from 4.4.x to 4.3.x March 28, 2024 10:15
@sdelamo
Copy link
Contributor

sdelamo commented Mar 28, 2024 •
edited
Loading

I created an alternative fix #10661 which allows users to override the implementation.

@sdelamo sdelamo mentioned this pull request Mar 28, 2024
Closed
sdelamo
sdelamo approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

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

I have removed the following:

http/src/main/resources/META-INF/services/io.micronaut.http.cookie.ClientCookieEncoder
http/src/main/resources/META-INF/services/io.micronaut.http.cookie.ServerCookieDecoder
http/src/main/resources/META-INF/services/io.micronaut.http.cookie.ServerCookieEncoder

and added tests.

@yawkat check it out.

@sdelamo sdelamo force-pushed the remove-default-cookie-decoder branch from 3a485ae to a695e84 Compare March 28, 2024 12:31
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@sdelamo sdelamo merged commit 8ea2045 into 4.3.x Mar 28, 2024
15 checks passed
@sdelamo sdelamo deleted the remove-default-cookie-decoder branch March 28, 2024 13:45
@sdelamo sdelamo mentioned this pull request Apr 23, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdelamo sdelamo sdelamo approved these changes

@yawkat yawkat yawkat approved these changes

Assignees

@sdelamo sdelamo

Labels
type: bug Something isn't working
Projects
No open projects
4.3.8 Release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@n0tl3ss @sdelamo @yawkat