ramondeklein / nwebdav

.NET implementation of the WebDAV protocol
MIT License
155 stars 42 forks source link

Problem with move request for non-existing source #52

Closed vishwas-trivedi closed 5 years ago

vishwas-trivedi commented 5 years ago

Problem Description

Move request of a non-existing files returns Status: 207(Multi-Status) instead of 404(Not Found). Though I was able to confirm that Multi-Status response contains 404(Not Found) as an inner response as following :

<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n
<D:multistatus xmlns:D=\"DAV:\" xmlns:Z=\"urn:schemas-microsoft-com:\">\r\n
  <D:response>\r\n
    <D:href>https://172.21.2.220:5003/bbb</D:href>\r\n
    <D:status>HTTP/1.1 404 Not Found</D:status>\r\n
  </D:response>\r\n
</D:multistatus>

Reproduction Procedure

Make a move request where source file does not exist.

Question

Is this intended behaviors or is it a bug?

ramondeklein commented 5 years ago

Section 9.9.4 of RFC4918 states that MOVE should return either 201, 204, 207, 403, 409, 412, 423 or 502. It can also return one of the general status codes, so the spec is not completely clear. Do you encounter issues with this result?

vishwas-trivedi commented 5 years ago

Hi Ramon, Thank you for the quick response!

Yes, I am facing problem with this, I would be grateful if you could let me know about following:

  1. When moving file from A → B, A does not exist but 207 MultiStatus has returned, which has resulted in success response on the client side. (OSS Clients like WebDAV.Client consider 207 as success response but actually its is not)
  2. If you test the same scenario in IIS, it returns 404 error code.
  3. I've checked the source of NWebDAVServer's MoveHandler and found it strange that you are checking if parent folder exists instead of actual file or folder that I need to mode. Please check the following code of yours :
    // We should always move the item from a parent container
    var splitSourceUri = RequestHelper.SplitUri(request.Url);
    var sourceCollection = await store.GetCollectionAsync(splitSourceUri.CollectionUri, httpContext).ConfigureAwait(false); 
    if (sourceCollection == null)
    {
       // Source not found
        response.SetStatus(DavStatusCode.NotFound);
        return true;
    }
ramondeklein commented 5 years ago

I think it's better to return 404 if the source file doesn't exist. The reason why I explicitly check for the source collection is to avoid for null reference issues. If the parent doesn't exists, then the source collection would be null and I cannot attempt to fetch the moved item.

ramondeklein commented 5 years ago

See also pull request #53

vishwas-trivedi commented 5 years ago

Hello Ramon, Thank you for the quick response.

I've confirmed the operation with the branch's code, the problem has been resolved. Could you please merge it to master?

ramondeklein commented 5 years ago

It has been merged to master and v0.1.34 has been pushed to NuGet.