tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Carpenter <mcarpen...@free.fr>
Subject WebDAV servlet returns 500 if files not readable
Date Tue, 01 Dec 2009 12:17:02 GMT

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


Mime
View raw message