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

Implement webdav SEARCH #3360

Merged
merged 6 commits into from
Mar 8, 2017
Merged

Implement webdav SEARCH #3360

merged 6 commits into from
Mar 8, 2017

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Feb 2, 2017

Still WIP, but submitting so the clients can start with it, functionality wise it should be ready for clients to start using the API

Clients can do a search by firing a SEARCH request at nextcloud/remote.php/dav with a search body as described by the rfc

For an example of a somewhat complex query that is possible with this api see this example request

cc @mario

@mention-bot
Copy link

@icewind1991, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MorrisJobke, @butonic and @nickvergessen to be potential reviewers.

@icewind1991
Copy link
Member Author

Fixes #3253

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't work:

$ curl -X SEARCH http://192.168.99.100/server/remote.php/dav -u admin:admin    
<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\NotImplemented</s:exception>
  <s:message>There was no plugin in the system that was willing to handle this SEARCH method.</s:message>
</d:error>

@icewind1991
Copy link
Member Author

icewind1991 commented Feb 15, 2017

@MorrisJobke you need to send a request body

curl -u test:test -X SEARCH  https://localcloud.icewind.me/remote.php/dav -H "content-Type: text/xml" --data '<?xml version="1.0"?>
 <d:searchrequest xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
 <d:basicsearch>
     <d:select>
         <d:prop>
             <oc:fileid/>
             <d:getcontenttype/>
             <d:getetag/>
             <oc:size/>
         </d:prop>
     </d:select>
     <d:from>
         <d:scope>
             <d:href>/files/test</d:href>
             <d:depth>infinity</d:depth>
         </d:scope>
     </d:from>
     <d:where>
         <d:like>
             <d:prop>
                 <d:getcontenttype/>
             </d:prop>
             <d:literal>text/%</d:literal>
         </d:like>
     </d:where>
     <d:orderby/>
</d:basicsearch>
</d:searchrequest>'

@MorrisJobke
Copy link
Member

I send it but it seems I forgot the text/xml header - let me retry then

/** @var Folder $folder $results */
$results = $folder->search($query);

var_dump(array_map(function (Node $node) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected? 😉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving debug code in production saves me from having to create a debug patch in the future 😄

@MorrisJobke
Copy link
Member

If I create a folder files/test I only get this as answer:

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>InvalidArgumentException</s:exception>
  <s:message>Search is only supported on directories</s:message>
</d:error>

@icewind1991
Copy link
Member Author

@MorrisJobke can you paste the full request you made?

@MorrisJobke
Copy link
Member

$ curl -X SEARCH https://example.org/remote.php/dav -u admin:admin -i -H "content-Type: text/xml" --data '<?xml version="1.0"?> 
 <d:searchrequest xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
 <d:basicsearch>
     <d:select>
         <d:prop>
             <oc:fileid/>
             <d:getcontenttype/>
             <d:getetag/>
             <oc:size/>
         </d:prop>
     </d:select>
     <d:from>
         <d:scope>
             <d:href>/files/test</d:href>
             <d:depth>infinity</d:depth>
         </d:scope>
     </d:from>
     <d:where>
         <d:like>
             <d:prop>
                 <d:getcontenttype/>
             </d:prop>
             <d:literal>text/%</d:literal>
         </d:like>
     </d:where>
     <d:orderby/>
</d:basicsearch>
</d:searchrequest>'

HTTP/1.1 404 Not Found
Date: Wed, 15 Feb 2017 16:36:52 GMT
...
Content-Length: 226
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\NotFound</s:exception>
  <s:message>Principal with name test not found</s:message>
</d:error>

@MorrisJobke
Copy link
Member

bildschirmfoto 2017-02-15 um 10 40 43

@icewind1991
Copy link
Member Author

@MorrisJobke the scope paths are relative to the dav root (remote.php/dav), so settings the scope to files/test searches in the root folder of the test user

@MorrisJobke
Copy link
Member

@MorrisJobke the scope paths are relative to the dav root (remote.php/dav), so settings the scope to files/test searches in the root folder of the test user

Tested and works 👍

cc @marinofaggiana I tested it against your test setup and it works 😃

@codecov-io
Copy link

Codecov Report

Merging #3360 into master will increase coverage by 0.54%.
The diff coverage is 68.96%.

@@             Coverage Diff             @@
##             master   #3360      +/-   ##
===========================================
+ Coverage     54.15%   54.7%   +0.54%     
- Complexity    21023   21630     +607     
===========================================
  Files          1306    1316      +10     
  Lines         80264   82610    +2346     
  Branches       1250    1300      +50     
===========================================
+ Hits          43466   45190    +1724     
- Misses        36798   37420     +622
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Search/SearchOrder.php 0% <ø> (ø) 3 <3> (?)
apps/dav/lib/Server.php 45.31% <ø> (-2.63%) 12 <ø> (ø)
lib/private/Lockdown/Filesystem/NullCache.php 90.9% <ø> (-3.44%) 22 <1> (+1)
lib/private/Files/Cache/FailedCache.php 0% <ø> (ø) 26 <1> (+1)
lib/private/Files/Search/SearchComparison.php 100% <100%> (ø) 4 <4> (?)
lib/private/Files/Cache/Wrapper/CacheJail.php 85.48% <100%> (+0.11%) 55 <1> (+18)
lib/private/Files/Cache/Wrapper/CacheWrapper.php 78.08% <100%> (+0.93%) 32 <1> (+1)
lib/private/Files/Node/Folder.php 88.05% <50%> (-0.83%) 49 <ø> (+1)
lib/private/Files/Search/SearchBinaryOperator.php 57.14% <57.14%> (ø) 3 <3> (?)
lib/private/Files/Cache/QuerySearchHelper.php 67.12% <67.12%> (ø) 23 <23> (?)
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd49eb3...7c7abf7. Read the comment docs.

@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 22, 2017

For this body:

<?xml version="1.0"?>
<d:searchrequest xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
<d:basicsearch>
    <d:select>
      <d:prop>
        <d:displayname/>
      </d:prop>
    </d:select>
    <d:from>
      <d:scope>
        <d:href>/files/admin</d:href>
        <d:depth>infinity</d:depth>
      </d:scope>
    </d:from>
    <d:where>
      <d:like>
        <d:prop>
          <d:displayname/>
        </d:prop>
        <d:literal>Documents</d:literal>
      </d:like>
    </d:where>
  </d:basicsearch>
</d:searchrequest>

I get following response:

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\ServiceUnavailable</s:exception>
  <s:message>TypeError: Argument 4 passed to OC\Files\Search\SearchQuery::__construct() must be of the type array, null given, called in /var/www/nextcloud/apps/dav/lib/Files/FileSearchBackend.php on line 177</s:message>
</d:error>

@LukasReschke
Copy link
Member

LukasReschke commented Feb 22, 2017

<s:message>TypeError: Argument 4 passed to OC\Files\Search\SearchQuery::__construct() must be of the type array, null given, called in /var/www/nextcloud/apps/dav/lib/Files/FileSearchBackend.php on line 177</s:message>

General note: Can we avoid leaking exception messages via DAV? They can sometimes contain sensitive content :)

@icewind1991
Copy link
Member Author

@MorrisJobke can you retry with the updated 3rdparty

@MorrisJobke
Copy link
Member

@MorrisJobke can you retry with the updated 3rdparty

I now get:

HTTP/1.1 207 Multi-Status

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:cal="urn:ietf:params:xml:ns:caldav" xmlns:cs="http://calendarserver.org/ns/" xmlns:card="urn:ietf:params:xml:ns:carddav" xmlns:oc="http://owncloud.org/ns" xmlns:nc="http://nextcloud.org/ns"/>

😕

@icewind1991
Copy link
Member Author

@MorrisJobke what is the request you're trying?

@MorrisJobke
Copy link
Member

@MorrisJobke what is the request you're trying?

#3360 (comment)

@mario
Copy link
Contributor

mario commented Feb 28, 2017

@icewind1991 when doing OPTIONS, search doesn't seem to be among supported methods?

@icewind1991
Copy link
Member Author

Works for me:

curl -I -X OPTIONS  -u test:test https://localcloud.icewind.me/remote.php/dav
...
Allow: OPTIONS, GET, HEAD, DELETE, PROPFIND, PUT, PROPPATCH, COPY, MOVE, REPORT, SEARCH

@mario
Copy link
Contributor

mario commented Feb 28, 2017 via email

@mario
Copy link
Contributor

mario commented Feb 28, 2017

@icewind1991 would something like this work with your implementation, since it doesn't look like the current libs supports that draft entirely:

<d:searchrequest xmlns:d="DAV:" dcr:="http://www.day.com/jcr/webdav/1.0" >
dcr:xpath//sv:node[@sv:name='myapp:paragraph'][1]</dcr:xpath>
</d:searchrequest>

cc @AndyScherzinger @tobiasKaminsky

Otherwise we might look into how we can either extend it use or use (possibly?) another lib just for this particular use case if such a thing exists.

@icewind1991 icewind1991 added this to the Nextcloud 12.0 milestone Feb 28, 2017
Signed-off-by: Robin Appelman <robin@icewind.nl>
@marinofaggiana
Copy link
Member

ping @icewind1991

@icewind1991
Copy link
Member Author

@marinofaggiana please describe the error you're getting instead of having us guess

@marinofaggiana
Copy link
Member

curl -u admin:XXXXXXXX -X SEARCH https://ios-demo.weasel.rocks/remote.php/dav -H "content-Type: text/xml" --data '<d:searchrequest xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns"><d:basicsearch><d:select><d:prop>oc:fileid/<d:getcontenttype/><d:getetag/>oc:size/oc:favorite/</d:prop></d:select><d:from><d:scope><d:href>/files/admin</d:href><d:depth>infinity</d:depth></d:scope></d:from><d:where><d:like><d:prop><d:displayname/></d:prop><d:literal>Nextcloud%</d:literal></d:like></d:where><d:orderby/></d:basicsearch></d:searchrequest>'

@marinofaggiana
Copy link
Member

@icewind1991 this is the response :

<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
<s:exception>Sabre\DAV\Exception\NotImplemented</s:exception>
<s:message>There was no plugin in the system that was willing to handle this SEARCH method.</s:message>
</d:error>

@marinofaggiana
Copy link
Member

@icewind1991 @MorrisJobke please info for this issue.

@mario
Copy link
Contributor

mario commented Mar 7, 2017

So search basically works, so please +1 all of you.

After that, I just need it extended to support oc:favorite in a query because filter-files won't work for Android atm.

@marinofaggiana
Copy link
Member

marinofaggiana commented Mar 7, 2017

@mario
"<oc:filter-files xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns\" xmlns:nc="http://nextcloud.org/ns\">oc:filter-rulesoc:favorite1</oc:favorite></oc:filter-rules><d:prop>"

@mario
Copy link
Contributor

mario commented Mar 7, 2017

@marinofaggiana using that our Android lib doesn't want to parse the propstat so I don't get the properties back.

@marinofaggiana
Copy link
Member

Modify the lib ?

@mario
Copy link
Contributor

mario commented Mar 7, 2017

Not something I feel comfortable doing under these kind of deadlines.

@MorrisJobke
Copy link
Member

Sorry - I screwed up the instance. It should work now. :( @marinofaggiana @icewind1991

@icewind1991 icewind1991 merged commit 2a8e922 into master Mar 8, 2017
@icewind1991 icewind1991 deleted the dav-search branch March 8, 2017 12:09
@barrydegraaff
Copy link

barrydegraaff commented Jan 15, 2018

@mario good work. I have been playing around with WebDAV SEARCH on nextcloud 12.0.4 and integrate it in Zimbra Nextcloud Client (https://github.com/Zimbra-Community/owncloud-zimlet)

Mostly I can make it work, but I have issues searching for files with more than 2 search words.
So I would like to search for all files that match: word1, word2 and word3. So far I can only make Nextcloud look for word1 and word2. The 3rd word is ignored. Where can I find good documentation on webdav search?

   curl -u user:password -X SEARCH https://nextcloud.example.com/nextcloud/remote.php/dav -H "content-Type: text/xml" --data '<d:searchrequest xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns"><d:basicsearch><d:select><d:prop>oc:fileid/<d:getcontentlength/><d:getlastmodified/><d:getcontenttype/><d:getetag/>oc:size/oc:favorite/</d:prop></d:select><d:from><d:scope><d:href>/files/user</d:href><d:depth>infinity</d:depth></d:scope></d:from><d:where><d:and><d:like><d:prop><d:displayname/></d:prop><d:literal>%word1%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word2%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word3%</d:literal></d:like></d:and></d:where><d:orderby><d:order><d:prop><d:displayname/></d:prop><d:ascending/></d:order><d:order><d:prop><d:getlastmodified/></d:prop><d:ascending/></d:order></d:orderby></d:basicsearch></d:searchrequest>'


   curl -u user:password -X SEARCH https://nextcloud.domain.com/nextcloud/remote.php/dav -H "content-Type: text/xml" --data '<d:searchrequest xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns"><d:basicsearch><d:select><d:prop>oc:fileid/<d:getcontentlength/><d:getlastmodified/><d:getcontenttype/><d:getetag/>oc:size/oc:favorite/</d:prop></d:select><d:from><d:scope><d:href>/files/user</d:href><d:depth>infinity</d:depth></d:scope></d:from><d:where><d:and><d:like><d:prop><d:displayname/></d:prop><d:literal>%word1%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word2%</d:literal></d:like></d:and></d:where><d:orderby><d:order><d:prop><d:displayname/></d:prop><d:ascending/></d:order><d:order><d:prop><d:getlastmodified/></d:prop><d:ascending/></d:order></d:orderby></d:basicsearch></d:searchrequest>'

So here only the first 2 words are matched, the 3rd is ignored... is there a limit of 2 in AND?

     <d:where><d:and><d:like><d:prop><d:displayname/></d:prop><d:literal>%word1%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word2%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word3%</d:literal></d:like></d:and></d:where>

@barrydegraaff
Copy link

https://www.ietf.org/rfc/rfc5323.txt
5.7. Boolean Operators: DAV:and, DAV:or, and DAV:not

The DAV:and operator performs a logical AND operation on the
expressions it contains.

Can anyone help me to do a WebDAV Search to nextcloud with more than 2 words to look for in displayname, I think the XML example, is according to the spec, but maybe I misread.

  <d:where><d:and><d:like><d:prop><d:displayname/></d:prop><d:literal>%word1%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word2%</d:literal></d:like><d:like><d:prop><d:displayname/></d:prop><d:literal>%word3%</d:literal></d:like></d:and></d:where>

@icewind1991
Copy link
Member Author

How I understood the spec (and implemented it) is that DAV:and and DAV:or only take 2 arguments,
you can nest multiple and/or statements to get your desired effect

@barrydegraaff
Copy link

@icewind1991 thanks, that makes it a bit harder to implement on the client side. It would be nice if the rfc said The operator takes two arguments. instead of The DAV:and operator performs a logical AND operation on the expressions it contains. I would not read 2 in that.

@barrydegraaff
Copy link

@icewind1991 I am thinking about asking julian reschke (his name is on the rfc) and see what he says.

But should the server return a 501 Not Implemented error if I do something that is outside the webdav spec? Aka an AND with 3 criteria = 501. Now it parses only the first 2.

@reschke
Copy link

reschke commented Jan 15, 2018

I agree that RFC 5323 could be clearer, but AFAIR, we would have said "two" if we had intended to restrict it to two operands.

@icewind1991
Copy link
Member Author

I will see if I can extend the implementation to allow for arbitrary number of arguments

@barrydegraaff
Copy link

barrydegraaff commented Jan 16, 2018 via email

@rullzer
Copy link
Member

rullzer commented Jan 16, 2018

I just noticed this as well.

Since and is associative you can just do

a AND b AND c AND d

and transform it into

((a AND b) AND c) AND d)

@icewind1991
Copy link
Member Author

Fix is here

@rullzer
Copy link
Member

rullzer commented Feb 25, 2018

@pinkylabs please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search API
10 participants