Summary: | CORS filter incorrectly implements same/local origin check | ||
---|---|---|---|
Product: | Tomcat 9 | Reporter: | Michael Osipov <michaelo> |
Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | major | CC: | michaelo |
Priority: | P2 | ||
Version: | 9.0.x | ||
Target Milestone: | ----- | ||
Hardware: | All | ||
OS: | All |
Description
Michael Osipov
2019-11-19 13:09:55 UTC
The CORS specification references RFC 6454 for the definition of the origin header. RFC 6454 states that the port should only be included in serialized form (which is the form used in the HTTP header) if the port differs from the default port. Tomcat's same origin test is, therefore, correct in this respect. As far as the case sensitivity of the check (or not) goes, both the scheme and host are case insensitive although lower case is the convention for both. It really should not make any difference at all whether this check is case sensitive or not. That said, this is the CORS filter and the CORS spec says the check should be performed in a case sensitive manner so I do think we should change this. We can change the method name at the same time. I suspect this will be part of some slightly wider refactoring in support of fixing bug 63637. (In reply to Mark Thomas from comment #1) > The CORS specification references RFC 6454 for the definition of the origin > header. > > RFC 6454 states that the port should only be included in serialized form > (which is the form used in the HTTP header) if the port differs from the > default port. Tomcat's same origin test is, therefore, correct in this > respect. You are referring to https://tools.ietf.org/html/rfc6454#section-6, granted. But the example from https://tools.ietf.org/html/rfc6454#section-3.2.1 claim that w/ and w/o default port is the same origin, so this the Spring's CORS filter: https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/web/cors/CorsUtils.java#L50-L52 Would you both consider to be wrong, or just lax? Because RFC 6454 refers in the ABNF to RFC 3986 and that likely will compare them equal. (In reply to Mark Thomas from comment #2) > As far as the case sensitivity of the check (or not) goes, both the scheme > and host are case insensitive although lower case is the convention for > both. It really should not make any difference at all whether this check is > case sensitive or not. That said, this is the CORS filter and the CORS spec > says the check should be performed in a case sensitive manner so I do think > we should change this. We can change the method name at the same time. Sound reasonable, I agree. If the client does not provide a clean serialized form, it is the client's fault. > I suspect this will be part of some slightly wider refactoring in support of > fixing bug 63637. This issue refers to JMeter? Typo? The bug requiring refactoring is bug 63937. The RFC 6454 inconsistency is unhelpful but the intention seems clear from section 3.2. The Spring reference was useful as it suggested we should include ws/wss when calculating default ports. I have a fix for this locally. Just want to run a few more tests. Fixed in: - master for 9.0.30 onwards - 8.5.x for 8.5.50 onwards - 7.0.x for 7.0.99 onwards |