Restrict automatic CORS configuration to UrlBasedCorsConfigurationSource#15444
Conversation
|
Hi @baezzys, thanks for the PR. Can you please target the |
| Map<String, CorsConfigurationSource> corsConfigurationSources = this.context | ||
| .getBeansOfType(CorsConfigurationSource.class); | ||
|
|
||
| boolean hasUrlBasedCorsConfigurationSource = corsConfigurationSources.values() |
There was a problem hiding this comment.
Would you please adopt another strategy instead of using Stream?
There was a problem hiding this comment.
I have refactored the code to avoid using Streams. PTAL. Thanks.
|
Hi @marcusdacoregio, I have completed the rebase and retargeted the PR to the 6.2.x branch. |
marcusdacoregio
left a comment
There was a problem hiding this comment.
Thanks @baezzys, I've left some feedback inline.
|
|
||
| for (CorsConfigurationSource source : corsConfigurationSources.values()) { | ||
| if (source instanceof UrlBasedCorsConfigurationSource) { | ||
| http.cors(withDefaults()); |
There was a problem hiding this comment.
I wonder if we should check if the instance is UrlBasedCorsConfigurationSource and if the bean name is corsConfigurationSource, since this is the bean name used by the CorsConfigurer, to avoid picking up the wrong CorsConfigurationSource.
There was a problem hiding this comment.
I have updated the code to check if there are any beans of type UrlBasedCorsConfigurationSource using getBeanNamesForType. If such beans exist, CORS configuration is applied.
If this is not what you intended, please feel free to provide further feedback.
|
|
||
| this.mockMvc.perform(formLogin()).andExpect(header().doesNotExist("Access-Control-Allow-Origin")); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you please add a test that verifies if the header Vary is not present? In summary, simulate the problem reported in #15378 and assert that it is fixed.
You can add the issue number in the test, like so:
// gh-15378
@Test
void ...() {
}There was a problem hiding this comment.
I have updated the existing test code to verify if the Vary header is not present. Thank you for your feedback!
…onSource - Update CORS configuration logic to automatically enable .cors() only if a UrlBasedCorsConfigurationSource bean is present. - Modify applyCorsIfAvailable method to check for UrlBasedCorsConfigurationSource instances.
|
Thanks @baezzys, this is now merged into |
Closes gh-15378