Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable DEPTH:infinity in PROPFIND is only applicable to dav/spaces path #7359

Closed
saw-jan opened this issue Sep 26, 2023 · 3 comments · Fixed by #7746
Closed

Disable DEPTH:infinity in PROPFIND is only applicable to dav/spaces path #7359

saw-jan opened this issue Sep 26, 2023 · 3 comments · Fixed by #7746

Comments

@saw-jan
Copy link
Member

saw-jan commented Sep 26, 2023

Describe the bug

It is possible to do PROPFIND request with Depth: infinity header using old (/webdav) and new (/dav/files) endpoints in the oCIS server started with default configs (mainly disable depth:infinity).

But the same request using spaces endpoint gives expected error message.

Steps to reproduce

  1. Start oCIS server with default configs (that has OCDAV_ALLOW_PROPFIND_DEPTH_INFINITY=false)

    PROXY_ENABLE_BASIC_AUTH=true \
    OCIS_ASYNC_UPLOADS=true \
    ./ocis/bin/ocis server
  2. Add some nested folders/files

  3. Do PROPFIND request using old/new dav paths with Depth: infinity (Possible - ❓)

    curl -XPROPFIND "https://localhost:9200/remote.php/dav/files/admin" -uadmin:admin -H"Depth: infinity" -vk
    Response
    <?xml version="1.0"?>
    <d:multistatus xmlns:s="http://sabredav.org/ns" xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
      <d:response>
        <d:href>/remote.php/dav/files/admin/</d:href>
        <d:propstat>
          <d:prop>
            ...
          </d:prop>
          <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
      </d:response>
      <d:response>
        <d:href>/remote.php/dav/files/admin/New%20folder/</d:href>
        <d:propstat>
          <d:prop>
            ...
          </d:prop>
          <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
      </d:response>
      <d:response>
        <d:href>/remote.php/dav/files/admin/New%20folder/file.txt</d:href>
        <d:propstat>
          <d:prop>
            ...
          </d:prop>
          <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
      </d:response>
      <d:response>
        <d:href>/remote.php/dav/files/admin/Shares/</d:href>
        <d:propstat>
          <d:prop>
            ...
          </d:prop>
          <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
          <d:prop>
            <oc:file-parent/>
          </d:prop>
          <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
      </d:response>
    </d:multistatus>
  4. Do PROPFIND request using spaces dav path with Depth: infinity (Error)

    curl -XPROPFIND "https://localhost:9200/remote.php/dav/spaces/<personal-space-id>" -uadmin:admin -H"Depth: infinity" -vk
    <?xml version="1.0" encoding="UTF-8"?>
    <d:error xmlns:d="DAV" xmlns:s="http://sabredav.org/ns">
      <s:exception>Sabre\DAV\Exception\BadRequest</s:exception>
      <s:message>Invalid Depth header value: infinity</s:message>
    </d:error>

NOTE: other endpoints unaffected by OCDAV_ALLOW_PROPFIND_DEPTH_INFINITY are:

  • Public link share /remote.php/dav/public-files/<token>
  • Trashbin /remote.php/dav/spaces/trash-bin/<personal-space-id>

Expected behavior

all endpoints yield same response

Actual behavior

disable depth:infinity work for spaces but not for old/new dav paths

@phil-davis phil-davis changed the title Disable DEPTH:inifinity in PROPFIND is only applicable to dav/spaces path Disable DEPTH:infinity in PROPFIND is only applicable to dav/spaces path Sep 26, 2023
@micbar micbar added the Priority:p3-medium Normal priority label Sep 27, 2023
@micbar micbar moved this from Qualification to Prio 3 or less in Infinite Scale Team Board Sep 27, 2023
@micbar
Copy link
Contributor

micbar commented Sep 27, 2023

@case0sh @kobergj This could be a candidate.

@2403905
Copy link
Contributor

2403905 commented Oct 24, 2023

@saw-jan I made the PR and a lot of tests failed. Please take a look if it is expected.

@2403905
Copy link
Contributor

2403905 commented Nov 16, 2023

@saw-jan fix already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants