Summary: | CGIServlet should handle additional HTTP methods, for example MKCALENDAR, MKCOL, PROPFIND, PROPPATCH | ||
---|---|---|---|
Product: | Tomcat 9 | Reporter: | jm009 <jan0michael> |
Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | enhancement | ||
Priority: | P2 | ||
Version: | 9.0.1 | ||
Target Milestone: | ----- | ||
Hardware: | All | ||
OS: | All | ||
Attachments: | Allow additional HTTP methods by configuration |
Description
jm009
2017-10-28 16:25:00 UTC
I don't like this patch (-1) for a number of reasons. 1. It uses many regular expressions during each request: poor performance 2. It requires configuration for what should be pass-through semantics Why not simply replace doGet, doPost, etc. with service(Request,Response) and pass everything through to the underlying CGI without all that overhead? (In reply to Christopher Schultz from comment #1) > I don't like this patch (-1) for a number of reasons. > > 1. It uses many regular expressions during each request: poor performance Patterns are compiled in init(), not for each request. Let Pattern p = Pattern.compile("A|B|C|D"). Then p.matcher(String).matches should be as efficient as "A".equals(String) || "B".equals(String) || "C".equals(String) || "D".equals(String). > 2. It requires configuration for what should be pass-through semantics > > Why not simply replace doGet, doPost, etc. with service(Request,Response) > and pass everything through to the underlying CGI without all that overhead? Yep, I mentioned that as alternative solution. Just to make sure, I got your point: What do you mean by "pass-through semantics"? How to determine in this case, if request body data (a.k.a. "POST data") should be forwarded to the servlet? - Forward any time? or - Check HttpServletRequest.getInputStream() != null && !HttpServletRequest.getInputStream().isFinished()? This is, what Apache HTTPD does (simply accapts all method names and forwards them to the CGI script). I am wondering, if there might be cases, where forwarding request body data to a CGI script, that is not prepared for it, might result in security problems. (In reply to jm009 from comment #2) > How to determine in this case, if request body data (a.k.a. "POST data") > should be forwarded to the servlet? If I get it right, this reverse proxy servlet https://github.com/mitre/HTTP-Proxy-Servlet decides to handle request body data, if either http header CONTENT_LENGTH or TRANSFER_ENCODING is set. Currently, the CGI servlet only handles a request body if the method is POST and the Content-Length header is set. Unless there are objections then I propose the following: - allow any HTTP method and let the CGI script decide how to handle it - except HEAD, TRACE and OPTIONS which will still be handled by HttpServlet - pass the request body for any method if Content-Length or Transfer-Encoding is set Since this is a change in functionality, I propose to only make this change in 9.0.x. > Unless there are objections then I propose the following: > - allow any HTTP method and let the CGI script decide how to handle it Sounds good. > - except HEAD, TRACE and OPTIONS which will still be handled by HttpServlet Shouldn't the servlet handle OPTIONS, because Tomcat doesn't know which methods are handled by the servlet? I would say yes, but to be honest, I have not much experience with details of HTTP traffic. Good point. Fixing that starts heading in the direction of the original patch again. More thought required. I've been thinking about this some more and experimenting with different ideas. Trying to reconcile the combination of maximising backwards compatibility, supporting any HTTP method and having the correct response for OPTIONS is not easy. Something has to give. My current thinking is to add a single new option, enableAllHttpMethods. If this is enabled, every request regardless of method will be passed to the CGI script and it will be expected to respond appropriately. This maximises backwards compatibility while enabling scripts to be deployed that support any methods. The downside is that if you enable this option the script has to handle OPTIONS, TRACE and any unrecognised methods. That seems a reasonable compromise to me. For the request body, I plan to go with the option of passing the request body for any method if Content-Length or Transfer-Encoding is set. This approach has the advantage that it can be applied to all supported Tomcat versions. I'll leave this a few days for folks to comment and if the consensus is favourable, I'll implement this approach for the next round of releases. (In reply to Mark Thomas from comment #7) > My current thinking is to add a single new option, enableAllHttpMethods. If > this is enabled, every request regardless of method will be passed to the > CGI script and it will be expected to respond appropriately. Alternative idea: - a regexp option that is applied as a filter to method names. If "OPTIONS" method matches the regexp filter then it means that OPTIONS method is processed by underlying CGI script. If "OPTIONS" method does not match the regexp filter, we can implement support for OPTIONS method in Java in the following way: 1. Iterate over a list of known HTTP methods - see IANA registry [1] 2. Retain only those that match the regexp filter. [1] https://www.iana.org/assignments/http-methods/http-methods.xhtml Another idea. Add a new option cgiMethods which is a comma separated list of methods to pass to the CGI script (or * for all methods). Requests using one of those methods are always passed to the script. Default value is "GET,POST" to match current behaviour. The request body is passed to the script for any method if Content-Length or Transfer-Encoding is set. Requests for HEAD, OPTIONS and TRACE are handled by the servlet (which may delegate to the super class) unless the method is listed in cgiMethods. The value of cgiMethods will be used to help populate the OPTIONS response. Requests using any other method gets a 405. I think this gives: - backwards compatible by default - support for arbitrary HTTP methods - the ability to pass any sub-set of HTTP methods to the CGi script I hadn't forgotten about this. I just hadn't found the time to come back to it and implement it. Fixed in: - trunk for 9.0.13 onwards - 8.5.x for 8.5.35 onwards - 7.0.x for 7.0.91 onwards |