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

fix: cors filter should not store in local variable allowed origins #1773

Open
yurem opened this issue Jan 9, 2023 · 3 comments
Open

fix: cors filter should not store in local variable allowed origins #1773

yurem opened this issue Jan 9, 2023 · 3 comments
Assignees

Comments

@yurem
Copy link
Contributor

yurem commented Jan 9, 2023

CorsFilter in doFilter method get allowed origins based on request and set them in AbstractCorsFilter.allowedOrigins before calling AbstractCorsFilter.doFilter. This is bad idea to pass them in such way because WebFilter is defined with asyncSupported = true. Hence second request can override this variable value. We can use:

request.setAttribute("clientAllowedOrigins", clientAllowedOrigins);

to pass client allowed origins to AbstractCorsFilter.doFilter

@yurem
Copy link
Contributor Author

yurem commented Apr 21, 2023

@mzico It's ready for QA

@mzico
Copy link
Contributor

mzico commented Jun 2, 2023

@yurem : In 4.5 and in 4.3.1.sp1, server responding with 200 even when request is not correct ( oxAuth.log showing error ).

I think we shouldn't allow server to respond with 200 and empty body. Better to make server response with 401 ( which is working in 4.4.2.sp1.

Can we make 4.5 and 4.3.1 same as 4.4.2.sp1?

Here is a GIF attached below to describe the situation:

200_empty_body

@yurem
Copy link
Contributor Author

yurem commented Jun 2, 2023

According to stack trace it's not CORS filter issue It's related to Authentication Filter.

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

No branches or pull requests

2 participants