WebDAV servlet returns 500 if files not readable

[Martin Carpenter <mc...@free.fr>]

Hello,

I have an issue with the standard WebDAV servlet bundled with Tomcat 5.5
and 6.0 (tested 5.5.26 and 6.0.20) that is causing me some pain. If a
directory contains a file that is not readable by the tomcat process (eg
file permissions, dangling symlink) then TC throws a NullPointerException
and returns a 500 Internal Server Error to the client. I think this
is incorrect behavior and that it should at least list the files in a
directory even if they are not all accessible to the user.

I couldn't see this issue reported in the BugZilla or on this list.

Stack:

    SEVERE: Servlet.service() for servlet webdav threw exception
    java.lang.NullPointerException
            at org.apache.catalina.servlets.WebdavServlet.parseProperties(Unknown Source)
            at org.apache.catalina.servlets.WebdavServlet.doPropfind(Unknown Source)
            at org.apache.catalina.servlets.WebdavServlet.service(Unknown Source)
            at javax.servlet.http.HttpServlet.service(Unknown Source)
	    ...


Method parseProperties() is in
java/org/apache/catalina/servlets/WebdavServlet.java.org and the NPE
happens when cacheEntry.attributes is null (case FIND_BY_PROPERTY in my
testing and possibly in other cases too).

Following the breadcrumbs via:

    cacheEntry = resources.lookupCache(path);

leads eventually to java/org/apache/naming/resources/FileDirContext.java:

    public Attributes getAttributes(String name, String[] attrIds)
        throws NamingException {
        // Building attribute list
        File file = file(name);
        if (file == null)
            throw new NamingException
                (sm.getString("resources.notFound", name));
    ...
    protected File file(String name) {
        File file = new File(base, name);
        if (file.exists() && file.canRead()) {
            ... // do useful stuff
        } else {
            return null; // ouch
        }


I see two potential fixes (but I'm not at all familiar with the
codebase):

1. Add lots of guard statements into caller parseProperties():

    if(cacheEntry.attributes == null) {
        propertiesNotFound.addElement(property);
    } else {
        ...

This is ugly and repetitive. There is already something like this for
cacheEntry.context and I can make this strategy work but adding more
code like this feels wrong.


2. Fix FileDirContext.file() to make it do as much as possible even
when exists()/canRead() return false.  Alternatively, do something
with/other than throwing NamingException when file() returns null.

This seems like a better solution but I worry about other dependencies on
the existing behaviour. How is test coverage?


Any other advice on how best to solve this appreciated!

Thanks,

Martin.



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

Re: WebDAV servlet returns 500 if files not readable

[Mark Thomas <ma...@apache.org>]

Martin Carpenter wrote:
> Hello,
> 
> I have an issue with the standard WebDAV servlet bundled with Tomcat 5.5
> and 6.0 (tested 5.5.26 and 6.0.20) that is causing me some pain. If a
> directory contains a file that is not readable by the tomcat process (eg
> file permissions, dangling symlink) then TC throws a NullPointerException
> and returns a 500 Internal Server Error to the client. I think this
> is incorrect behavior and that it should at least list the files in a
> directory even if they are not all accessible to the user.
> 
> I couldn't see this issue reported in the BugZilla or on this list.

Please create a new issue.

> This seems like a better solution but I worry about other dependencies on
> the existing behaviour. How is test coverage?

Tomcat unit tests are non-existent for this. I tend to use the litmus
test suite for WebDAV testing.

> Any other advice on how best to solve this appreciated!

I'm tempted to say any inaccessible file should just be ignored.
However, that could cause issues if a user tries to upload a file of
that name. Certainly, any such configuration isn't valid so returning an
error to the user and logging an error isn't too unreasonable. Maybe
better error reporting is the way to go with this one.

Mark




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