Go: x/net/webdav: fails on broken links

[enoodle]

When trying to list a directory that contains a broken link I get a non informative error:

$ cadaver webdav://localhost:5555/api/v1/content                                                                                                                                         
dav:/api/v1/content/> ls etc
Listing collection `/api/v1/content/etc/': failed:
XML parse error at line 1: junk after document element

This is what I get from the webdav server:

2016/06/27 14:52:42 http: multiple response.WriteHeader calls

The error comes from file.go [1] where it calls stat on the broken link file but I think that maybe it should be handled in webdav.go walkFn [2].

I am not sure myself what should be printed / returned but I would expect to get the full directory listing (ie webdav shouldn't crush on this in my opinion) with this link marked a broken somehow .

[1] https://github.com/golang/net/blob/master/webdav/file.go#L779
[2] https://github.com/golang/net/blob/master/webdav/webdav.go#L527

/cc @simon3z

[skritchz]

I just encounter this issue to. I fixed it locally by just returning nil when getting an error here https://github.com/golang/net/blob/master/webdav/webdav.go#L557.

I tested the same use case with Apache implementation and in the case of a broken symblink, it is silently discarded.

I don't know if it is a good enough solution but today's behavior is incorrect too.
Here is a wireshark trace of the faulty response

Host: 10.21.59.204
Depth: 1
Content-Type: application/xml
Apply-To-Redirect-Ref: T
Accept-Encoding: gzip, deflate
User-Agent: gvfs/1.28.2
Accept-Language: en-us, en;q=0.9
Connection: Keep-Alive
Content-Length: 235

<?xml version="1.0" encoding="utf-8" ?>
 <D:propfind xmlns:D="DAV:">
  <D:prop>
<D:creationdate/>
<D:displayname/>
<D:getcontentlength/>
<D:getcontenttype/>
<D:getetag/>
<D:getlastmodified/>
<D:resourcetype/>
  </D:prop>
 </D:propfind>HTTP/1.1 207 status code 207
Content-Type: text/xml; charset=utf-8
Date: Wed, 15 Mar 2017 09:00:40 GMT
Content-Length: 610

<?xml version="1.0" encoding="UTF-8"?><D:multistatus xmlns:D="DAV:"><D:response><D:href>/dav</D:href><D:propstat><D:prop><D:displayname></D:displayname><D:getlastmodified>Wed, 15 Mar 2017 08:59:55 GMT</D:getlastmodified><D:resourcetype><D:collection xmlns:D="DAV:"/></D:resourcetype></D:prop><D:status>HTTP/1.1 200 OK</D:status></D:propstat><D:propstat><D:prop><D:creationdate></D:creationdate><D:getcontentlength></D:getcontentlength><D:getcontenttype></D:getcontenttype><D:getetag></D:getetag></D:prop><D:status>HTTP/1.1 404 Not Found</D:status></D:propstat></D:response></D:multistatus>Internal Server Error

In the Webdav Spec it is said that

In the case of allprop and propname, if a principal does not have the
right to know whether a particular property exists then the property
should be silently excluded from the response.

I don't know if this apply in our use case

[bingliu221]

I think a 4XX status code should be return in the case of accessing broken links, more specifically, 403 Forbidden for links linked to out of permission targets, and 404 Not Found for links linked to nonexistent targets.

Currently errors returned by FileSystem.OpenFileor File.Stat transfer into http.StatusInternalServerError and cause "http: superfluous response.WriteHeader call from golang.org/x/net/webdav.(*Handler).ServeHTTP (webdav.go:74)"

[gopherbot]

Change https://golang.org/cl/249797 mentions this issue: webdav: ignore os.PathError in PROPFIND