Bug 69360 - Inconsistent DELETE behavior between DefaultServlet and WebdavServlet
Summary: Inconsistent DELETE behavior between DefaultServlet and WebdavServlet
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.95
Hardware: All All
: P2 major (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-10-02 07:51 UTC by Michael Osipov
Modified: 2024-10-02 11:53 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2024-10-02 07:51:18 UTC
If a resource cannot be deleted by the DefaultServlet https://github.com/apache/tomcat/blob/c50b8aa2588c85df86c0c6e0cd03e79700822360/java/org/apache/catalina/servlets/DefaultServlet.java#L699-L700 METHOD_NOT_ALLOWED is returned. Either the resource is readOnly or java.io.File#delete() returns false. Reasonable. But the WebdavServlet says https://github.com/apache/tomcat/blob/c50b8aa2588c85df86c0c6e0cd03e79700822360/java/org/apache/catalina/servlets/WebdavServlet.java#L1703-L1714 INTERNAL_SERVER_ERROR which is inconsistent and not necessarily true. We can only return this if we use Files#delete(Path).

If no one objects I will return METHOD_NOT_ALLOWED for both servlets. Took me some time to search for the exception in the catalina.out which I haven't found until I have found this inconsistency.
Comment 1 Michael Osipov 2024-10-02 07:54:26 UTC
Moreover, I'd even use the method sendNotAllowed() for consistency reasons.
Comment 2 Michael Osipov 2024-10-02 08:04:08 UTC
(In reply to Michael Osipov from comment #1)
> Moreover, I'd even use the method sendNotAllowed() for consistency reasons.

Nope, won't work because when a collection is hit it could be partially deleted. I will stick to the status code for now. This change must be reviewed separately.
Comment 3 Remy Maucherat 2024-10-02 08:22:27 UTC
METHOD_NOT_ALLOWED is not a bad status to return for WebDAV DELETE if it fails, although 500 is ok as well since the cause is not really known.
Comment 4 Michael Osipov 2024-10-02 08:33:22 UTC
(In reply to Remy Maucherat from comment #3)
> METHOD_NOT_ALLOWED is not a bad status to return for WebDAV DELETE if it
> fails, although 500 is ok as well since the cause is not really known.

Well, for read-only it is false. The likelyhood for FS failures is smaller here, I guess. Are you OK with METHOD_NOT_ALLOWED consistency for both servlet?
Comment 5 Remy Maucherat 2024-10-02 08:37:30 UTC
(In reply to Michael Osipov from comment #4)
> (In reply to Remy Maucherat from comment #3)
> > METHOD_NOT_ALLOWED is not a bad status to return for WebDAV DELETE if it
> > fails, although 500 is ok as well since the cause is not really known.
> 
> Well, for read-only it is false. The likelyhood for FS failures is smaller
> here, I guess. Are you OK with METHOD_NOT_ALLOWED consistency for both
> servlet?

Yes, as I was saying, I verified in some docs that METHOD_NOT_ALLOWED is a good return status code in that situation for WebDAV DELETE.
Comment 6 Michael Osipov 2024-10-02 08:38:41 UTC
(In reply to Remy Maucherat from comment #5)
> (In reply to Michael Osipov from comment #4)
> > (In reply to Remy Maucherat from comment #3)
> > > METHOD_NOT_ALLOWED is not a bad status to return for WebDAV DELETE if it
> > > fails, although 500 is ok as well since the cause is not really known.
> > 
> > Well, for read-only it is false. The likelyhood for FS failures is smaller
> > here, I guess. Are you OK with METHOD_NOT_ALLOWED consistency for both
> > servlet?
> 
> Yes, as I was saying, I verified in some docs that METHOD_NOT_ALLOWED is a
> good return status code in that situation for WebDAV DELETE.

Alright, thanks.
Comment 7 Michael Osipov 2024-10-02 09:08:50 UTC
Fixed in:
- main for 12.0.0-M1 and onwards
- 11.0.x for 11.0.0 and onwards
- 10.1.x for 10.1.31 and onwards
- 9.0.x  for 9.0.96 and onwards
Comment 8 Michael Osipov 2024-10-02 11:53:09 UTC
Incorporated Mark's objection.