Bug 67472 - CorsFilter erroneously adds CORS headers in responses to Non-CORS requests
Summary: CorsFilter erroneously adds CORS headers in responses to Non-CORS requests
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 10
Classification: Unclassified
Component: Util (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal (vote)
Target Milestone: ------
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-19 23:01 UTC by lmedini
Modified: 2023-09-22 17:12 UTC (History)
0 users



Attachments
First request without Origin headers and response with CORS headers (in French, sorry) (82.96 KB, image/jpeg)
2023-09-19 23:01 UTC, lmedini
Details

Note You need to log in before you can comment on or make changes to this bug.
Description lmedini 2023-09-19 23:01:53 UTC
Created attachment 39049 [details]
First request without Origin headers and response with CORS headers (in French, sorry)

Hi,

Problem experienced in Tomcat 10.1.13.

When CorsFilter configuration is added to Tomcat's web.xml, all requests are treated as CORS requests. I mean that the response to any request will contain CORS headers (see attachment), causing the client to send the server's origin in further requests, which are then blocked by the filter if they are not in the cors.allowed.origins list.

According to the flowchart at https://tomcat.apache.org/tomcat-10.1-doc/images/cors-flowchart.png, responses to non-CORS request should not be added any additional header.

An easy workaround would be to add the server's origin to this list, but:

- it adds useless header exchanges and processing in same-origin transactions,
- it is much harder to do when the server is installed on VMs that are instanciated by scripts.

I think what causes this bug is line 325 in CorsFilter.java:
    addStandardHeaders(request, response);
This method should not be called in handleNonCors().

Can you remove this instruction from the next versions, please?

Thanks in advance and best regards.

L. Medini.
Comment 1 Christopher Schultz 2023-09-20 13:55:40 UTC
Please post more of the request details (e.g. HTTP method, URL, etc.) as well as your CORDSFilter configuration. Feel free to mask-out sensitive parts such as domain name and/or application name.

The CORDFilter contains this comment just before these header values:

// CORS requests (SIMPLE, ACTUAL, PRE_FLIGHT) set the following headers
// although non-CORS requests do not need to. The headers are always set
// as a) they do no harm in the non-CORS case and b) it allows the same
// response to be cached for CORS and non-CORS requests.

This comment asserts that there is no harm done by setting them, but you are claiming otherwise. Can you please explain what is happening to you in more detail?

The note about caching seems very relevant to HTTP.

Note that this code has remained unchanged for the past 5 years, and no other complaints have been filed that I know of.
Comment 2 lmedini 2023-09-21 18:53:18 UTC
I am aware that (1) it is not common to use the same server for both server-side applications and CORS-enabled web APIs, and (2) most developers will configure CORS on a per-application basis, which is probably why nobody has had this problem before. In my case, I am teaching the servlet API (the basics) and web APIs to master degree students, and wish to provide them with an infrastructure that they will be able to use during all this class. That is why I chose this solution and need a CORS filter that only operates for CORS requests. Even if this is not a common need, my claim is that it is the spec's normal behavior. ;-)

Regarding the note about caching: I am not sure that many clients will send the same requests (for non-static resources, otherwise, they should not be served by an application server) in both same-origin and CORS modes. Anyhow, it is the role of the Vary header to specify if CORS headers should be taken into account for caching resources, so removing "Origin" from this header should allow caching resources across CORS and non-CORS requests.

--------

Despite the code comment, I confirm that this behavior does harm in some cases. From my experience, the CORS filter blocks requests with proxied URIs and POST method. I am unsure of its behavior with other preflight requests, but it is likely to be the same.

So that you can test and reproduce it, kindly check https://github.com/lmedini/cors-war-test

You will need a reverse proxy, in addition to Tomcat. You will find detailed explanations about configuration, installation and unexpected behavior in the readme of this repo.

Thank you in advance.
Comment 3 Christopher Schultz 2023-09-21 20:51:23 UTC
This seems patently incorrect to me:

  <init-param>
    <param-name>cors.allowed.origins</param-name>
    <param-value>https://another.origin.than.that.of.the.proxy</param-value>
  </init-param>

Why are you only allowing an origin that does NOT go through the proxy? Isn't the point of using the proxy, thus the cors.allowed.origins should be EXACTLY THAT OF THE PROXY and not anything else?

Are you intending to use this setup to make requests both through the proxy and NOT through the proxy? If so, then you should have BOTH origins in your config and not just one.
Comment 4 lmedini 2023-09-21 21:33:58 UTC
Well, this is precisely the point of Cross-Origin Resource Sharing (CORS). See for instance https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

CORS has been created so that servers can control how they respond to requests made by scripts from other origins than their own. Otherwise, they are same-origin requests and don't need the CORS mechanism. Abd this holds whether the application was obtained through a proxy or not.
Comment 5 Christopher Schultz 2023-09-21 21:39:56 UTC
Applications must explicitly trust ALL proxies for such requests.

Unless I very much misunderstand what you are trying to accomplish, here, I believe this is not a legitimate bug report.
Comment 6 Christopher Schultz 2023-09-22 15:36:37 UTC
It may be possible to tighten this up a bit to help you out.
Comment 7 Christopher Schultz 2023-09-22 16:49:47 UTC
We've chosen to reduce the header noise on non-CORS requests.

Added in 11.x in e610e313765a9724bbba9ca8ceb6f14af9ae9782
Added in 10.x in 46dde2dfa7cb84b99882c45a11045adab8217fd2
Added in 9.0.x in 4dd9c16dd1da0eea0a82a9a2f9ba27daf67eda6d
Added in 8.5.x in af4ee918319c566c43eb8c4e48cdef198ecefc60

Hopefully this change will fix your issue.
Comment 8 lmedini 2023-09-22 17:12:19 UTC
Great, it sure will!

Thx.