You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2021/07/09 18:08:08 UTC
[Bug 65443] New: Allow subclassing CorsFilter
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
Bug ID: 65443
Summary: Allow subclassing CorsFilter
Product: Tomcat 9
Version: unspecified
Hardware: PC
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: ekkelenkamp@gmail.com
Target Milestone: -----
I would like to subclass the the CorsFilter to be able to provide a custom list
of allowed origins, instead of configure them from the init parameters.
Unfortunately the method isOriginAllowed uses the instance variable
allowedOrigns directly. This makes it impossible to override the available
function: getAllowedOrigins and isAnyOriginAllowed.
So i would propose the change the isOriginAllowed function from the CorsFilter
class as follows:
/**
* Checks if the Origin is allowed to make a CORS request.
*
* @param origin
* The Origin.
* @return <code>true</code> if origin is allowed; <code>false</code>
* otherwise.
*/
private boolean isOriginAllowed(final String origin) {
if (isAnyOriginAllowed()) {
return true;
}
// If 'Origin' header is a case-sensitive match of any of allowed
// origins, then return true, else return false.
return getAllowedOrigins().contains(origin);
}
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #4 from ekkelenkamp@gmail.com ---
Indeed I would prefer if there is a configuration method available for those
settings. I just assumed the state was immutable.
The setAllowedOrigins would have to set the anyOriginAllowed variable as well.
Changing the mutable Map that currently is returned from the getAllowedOrigins
will not do, since the anyOriginAllowed variable needs to be updated as well in
case a wildcard domain is set.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #2 from ekkelenkamp@gmail.com ---
Just created a pull request: https://github.com/apache/tomcat/pull/432
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #3 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to ekkelenkamp from comment #2)
> Just created a pull request: https://github.com/apache/tomcat/pull/432
I am -0.
Wouldn't it be better to provide configuration methods for those settings (e.g.
setAllowedOrigins(boolean, Collection<String>) instead of overriding getter
methods?
BTW, the map returned by `getAllowedOrigins()` is mutable, though its mutations
are not thread-safe.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #6 from ekkelenkamp@gmail.com ---
Thanks for the feedback.
The requirement is indeed to be able to configure the allowed origins at
runtime.
Should I provide a pull request for this enhancement or is this kind of
refactoring done by the core developers?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #5 from Mark Thomas <ma...@apache.org> ---
The initial implementation was clearly on the basis that the state was set
during init() and then never changed but if you have a requirement to change it
then I don't see a good reason not to refactor the code and provide setters.
I'll note that other built-in filters have getters and setters.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #10 from ekkelenkamp@gmail.com ---
Thanks for providing the change. I'll use it in my code once available in the
new releases.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #9 from Mark Thomas <ma...@apache.org> ---
I've made the CorsFilter class sufficiently extensible to meet this
requirement. I opted not to provide setters as that opens up additional
concurrency issues.
Fixed in:
- 10.1.x for 10.1.0-M3 onwards
- 10.0.x for 10.0.9 onwards
- 9.0.x for 9.0.51 onwards
- 8.5.x for 8.5.70 onwards
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #7 from ekkelenkamp@gmail.com ---
Just to let you know, I found a convenient way to get the job done without
changing the code.
Simply extending the getInitParameter method, does the job. As a reference,
this is the pseudo setup of my code:
/**
* extended to allow configuration at runtime.
*/
public final class MyCorsFilter extends CorsFilter {
private static String getCorsAllowedOrigins() {
return config.getCorsAllowedOrigins(); // get config from database
}
@Override
public String getInitParameter(String name) {
if (name.equals("cors.allowed.origins")) {
return getCorsAllowedOrigins();
}
return super.getInitParameter(name);
}
}
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #8 from Christopher Schultz <ch...@christopherschultz.net> ---
This violates the servlet spec, but hey if you want to do that in your code,
it's fine.
Honestly, I was hoping you'd end up writing a proper patch because I think this
is a good idea to have in the base class.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 65443] Allow subclassing CorsFilter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
--- Comment #1 from Christopher Schultz <ch...@christopherschultz.net> ---
Sounds like a perfect opportunity for you to supply a patch (attached here) or
a pull-request via GitHub. I would support its inclusion into Tomcat.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org