You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tomcat.apache.org by kdombeck <gi...@git.apache.org> on 2017/12/13 22:16:52 UTC

[GitHub] tomcat pull request #96: Remove PUT and DELETE methods from an OPTIONS reque...

GitHub user kdombeck opened a pull request:

    https://github.com/apache/tomcat/pull/96

    Remove PUT and DELETE methods from an OPTIONS request if readOnly is true

    Currently ```DefaultServlet``` is returning all HTTP methods for an OPTIONS call even when the **readOnly** flag is true.
    
    Example:
    ```
    $ curl -v -X OPTIONS localhost:8080
    ....
    < Allow: GET, HEAD, POST, PUT, DELETE, OPTIONS
    ....
    ```
    It should be the following instead if the **readOnly** flag is **true**.
    ```
    $ curl -v -X OPTIONS localhost:8080
    ....
    < Allow: GET, HEAD, POST, OPTIONS
    ....
    ```
    
    As a side note I don't understand the following.
    * Why is a POST allowed when **readOnly** is true?
    * Why does a POST call GET internally?

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kdombeck/tomcat remove-options

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/tomcat/pull/96.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #96
    
----
commit deb05b2c1c4693b1cb5904e9bba8f46bca18ee7d
Author: Ken Dombeck <kd...@gmail.com>
Date:   2017-12-13T21:55:59Z

    Remove PUT and DELETE methods from an OPTIONS request if readOnly is true

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by markt-asf <gi...@git.apache.org>.

Github user markt-asf commented on the issue:

    https://github.com/apache/tomcat/pull/96
  
    Lack of response == lazy consensus. I plan to look at applying these patches (or possibly a variation of them) before tagging 9.0.x.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by kdombeck <gi...@git.apache.org>.

Github user kdombeck commented on the issue:

    https://github.com/apache/tomcat/pull/96
  
    @markt-asf I haven't received any responses to the [thread](http://tomcat.10.x6.nabble.com/Change-HTTP-status-code-for-DefaultServlet-with-readOnly-set-to-true-td5070373.html) I started on dev@. I know it was the holidays, so what is the best way to move forward to get consensus on this issue?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by michael-o <gi...@git.apache.org>.

Github user michael-o commented on the issue:

    https://github.com/apache/tomcat/pull/96
  
    @markt-asf I'd second the 405 instead of 403.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by markt-asf <gi...@git.apache.org>.

Github user markt-asf commented on the issue:

    https://github.com/apache/tomcat/pull/96
  
    I've fixed this in trunk. Given the behaviour changes (you can bet someone depends on a 403 rather than a 405 response) I'm not going to back-port it.
    The way I ended up tackling this (WebDAV first then Default) meant I didn't work from the patch but I certainly depended heavily on this discussion.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by markt-asf <gi...@git.apache.org>.

Github user markt-asf commented on the issue:

https://github.com/apache/tomcat/pull/96

The inconsistency stems from the different status codes used. If TRACE is disabled at the connector, level then a 405 is returned for TRACE requests. If readOnly is set in the DefaultServlet, a 403 is returned for PUT and DELETE. Methods that return a 405 should not be included in an OPTIONS response.

Doing some mailing list archaeology, the readOnly parameter started out in the WebDAV servlet ( http://markmail.org/message/vls7an5nhf7eigwc ) and was refactored to the Default Servlet shortly afterwards ( http://markmail.org/message/vls7an5nhf7eigwc ). All this was back in 2000.

Looking at RFC 2616 (since this code pre-dates RFC7231) there isn't much to choose between 403 and 405. Both look equally applicable. One obvious difference for this use case is that a 405 MUST include an Allow header so on that basis using 403 looks to be the simpler implementation.

RFC 7231 adds some detail on the differences between 403 and 405. In short 405 means 'method never allowed' whereas 403 means 'request not allowed but might be allowed with different credentials'.

In this case it is arguable that 405 is the better response code since if readOnly is true, PUT and DELETE are never going to be allowed (with a similar argument applied to many of the WebDAV methods). We have made changes in 9.0.x to better align with RFC 7231 so I'm not against considering changing the status code used. That would also mean changing the OPTIONS response.

As I have been researching this I noticed that the is scope to clean up the existing code. HttpServlet.doOptions() could use a StringBuilder and ALLOW_OPTIONS seems unnecessary. DefaultServlet.doOptions() appears to be unnecessary as well. This clean-up assumes the current behaviour is not changed. A switch to using 405 is likely to impact some of that clean-up.

If there is interest in switching the DefaultServlet to use 405 rather than 403 when readOnly is true, I'd suggest that a discussion on the dev@ list is required to ensure there is consensus on this approach.

---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by markt-asf <gi...@git.apache.org>.

Github user markt-asf commented on the issue:

    https://github.com/apache/tomcat/pull/96
  
    The current OPTIONS behaviour is intentional.
    OPTIONS lists the valid methods, not the permitted methods, for a resource.
    
    The GET/POST issue is before my time with the project. There might be some explanation in the mail archive. I agree with Chris it is unlikely to change.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[GitHub] tomcat issue #96: Remove PUT and DELETE methods from an OPTIONS request if r...

Posted by kdombeck <gi...@git.apache.org>.

Github user kdombeck commented on the issue:

    https://github.com/apache/tomcat/pull/96
  
    Thanks for the quick responses.
    
    I modeled this change after the changes for TRACE in the next if statement below. Also looking at [issue](https://bz.apache.org/bugzilla/show_bug.cgi?id=60697) this change seemed to be similar to that one.
    
    It would make more sense to me if a user could actually change the request some how to invoke PUT or DELETE (by authenticating for example), but in this case there is nothing the user could do to ever get these 2 methods to work without changing the readOnly flag to false.
    
    I am not adamant that this change should be made. I am just trying to understand what appears to be an inconsistency to me. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org