PROPFIND to get list of files doesn't properly encode @ character #33594

davigonz · 2018-11-20T12:07:50Z

We've been facing some issues with usernames containing 'At' character (@) in mobile apps so I had a deep look into it and noticed a weird behaviour. It seems that the PROPFIND to get the files for a specific user is not properly retrieving the encoded path for files when using @.

Steps to reproduce

Curl command to list a directory:

Being %40 the @ character encoded, therefore username user@whatever is user%40whatever

curl -X PROPFIND -H "Content-Type: text/xml" -H 'User-Agent:Mozilla/5.0 (Android) ownCloud-android/2.9.2' -H 'Authorization:Basic base64Credentials' -d "<?xml version='1.0' encoding='utf-8' ?><D:propfind xmlns:D='DAV:'><D:allprop/></D:propfind>" 'url:port/remote.php/dav/files/user%40name/' | xmllint --format -

Expected behaviour

Server should include the path for every file properly encoded, i.e. /remote.php/dav/files/user%40name/whatever

Expected curl result:

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>/remote.php/dav/files/user%40name/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>5630922</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9f264d46"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user%40name/Documents/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>36227</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9f240a04"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user%40name/Photos/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>678556</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9f264d46"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user%40name/ownCloud%20Manual.pdf</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:getcontentlength>4916139</d:getcontentlength>
        <d:resourcetype/>
        <d:getetag>"ae4602f56f897e638ad678c57e5404ca"</d:getetag>
        <d:getcontenttype>application/pdf</d:getcontenttype>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

Actual behaviour

Server includes @ instead of %40, e.g. /remote.php/dav/files/user@name/whatever

Actual curl result:

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>/remote.php/dav/files/user@name/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>5630922</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9f264d46"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user@name/Documents/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>36227</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9f240a04"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user@name/Photos/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>678556</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9f264d46"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user@name/ownCloud%20Manual.pdf</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:03:14 GMT</d:getlastmodified>
        <d:getcontentlength>4916139</d:getcontentlength>
        <d:resourcetype/>
        <d:getetag>"ae4602f56f897e638ad678c57e5404ca"</d:getetag>
        <d:getcontenttype>application/pdf</d:getcontenttype>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

Server configuration

Web server: Apache 2.4.25

Database: MySQL

PHP version: php7.2.12

ownCloud version: 10.0.10 (stable)

CC / @michaelstingl @DeepDiver1975 @PVince81

The text was updated successfully, but these errors were encountered:

davigonz · 2018-11-20T12:25:56Z

With other characters like space (%20) works as expected.

Curl command:
With the username user name , being encoded as user%20name.

curl -X PROPFIND -H "Content-Type: text/xml" -H 'User-Agent:Mozilla/5.0 (Android) ownCloud-android/2.9.2' -H 'Authorization:Basic base64Credentials' -d "<?xml version='1.0' encoding='utf-8' ?><D:propfind xmlns:D='DAV:'><D:allprop/></D:propfind>" 'url:port/remote.php/dav/files/user%20name/' | xmllint --format -

Result:

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>/remote.php/dav/files/user%20name/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:02:27 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>5630922</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9c3df9af"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user%20name/Documents/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:02:27 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>36227</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9c3b95b3"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user%20name/Photos/</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:02:27 GMT</d:getlastmodified>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:quota-used-bytes>678556</d:quota-used-bytes>
        <d:quota-available-bytes>-3</d:quota-available-bytes>
        <d:getetag>"5bf3e9c3df9af"</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
  <d:response>
    <d:href>/remote.php/dav/files/user%20name/ownCloud%20Manual.pdf</d:href>
    <d:propstat>
      <d:prop>
        <d:getlastmodified>Tue, 20 Nov 2018 11:02:27 GMT</d:getlastmodified>
        <d:getcontentlength>4916139</d:getcontentlength>
        <d:resourcetype/>
        <d:getetag>"6086aa0d80c0f3f57ae262a54a200055"</d:getetag>
        <d:getcontenttype>application/pdf</d:getcontenttype>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

ownclouders · 2018-11-20T12:33:24Z

GitMate.io thinks possibly related issues are #22326 (PROPFIND answer doesn't include encoding.), #12112 (ownCloud File Mapper Not Encoding UTF-8 Properly), #11724 (filename with bad encoding not listed), and #27089 (Server >9.0 is not responding with 403 Forbidden when some firewall rules are enabled).

phil-davis · 2018-11-20T13:39:59Z

I made issue #33596 to consider adding a more general variety of acceptance tests for user IDs.

phil-davis · 2018-11-20T13:40:48Z

I am surprised that "space" in a user ID works at all. I didn't think that user IDs containing the "space" char could be created in the first place.

davigonz · 2018-11-20T14:45:19Z

I didn't think that user IDs containing the "space" char could be created in the first place.

I thought the same the first time I saw it 😄

jesmrec · 2018-11-20T15:14:52Z

DeepDiver1975 · 2018-11-21T09:51:34Z

I could not find any detailed specs in the RFC regarding what to decode and what not.

Tests with default Apache+mod_dav show the same behavior:

I created a folder at@user and a folder "a b" and this is the propfind response:

DeepDiver1975 · 2018-11-21T09:55:05Z

See this test - only spaces need to be decoded in href sabre-io/dav#719 https://github.com/sabre-io/http/blob/4bfb3834209de0a4c658e0dbe69bda794bf42e3a/lib/functions.php#L385

davigonz · 2018-11-21T11:36:42Z

Thanks @DeepDiver1975 for having a look into this. I am going to give you more context about what is done in the Android app after performing a PROPFIND operation to get all the files from a specific folder.

Once we get the PROPFIND response, we try to create a list of RemoteFile objects so we can save them into the database. This RemoteFile objects should contain a relative remote path, e.g. /Documents/text1, being the whole remote path /remote.php/dav/files/username/Documents/text1.

How is this relative remote path built? By comparing two paths:

/remote.php/dav/files/username/ composed of /remote.php/dav/files/ stored in an app constant and username retrieved in a previous GET user info operation, properly encoded and saved.
/remote.php/dav/files/username/Documents/text1 read from href in PROPFIND response.
Relative remote path: /Documents/text1 because both paths are the same before this relative path.

What happens when the username contains space? Which paths are being compared?

/remote.php/dav/files/user%20name/ composed of /remote.php/dav/files/ constant and the user%20name properly encoded.
/remote.php/dav/files/user%20name/Documents/text1 read from href in PROPFIND response.
Relative remote path: /Documents/text1 because both paths are the same before this relative path.

What happens when the username contains @? Which paths are being compared?

/remote.php/dav/files/user%40name/ composed of /remote.php/dav/files/ constant and the user%40name properly encoded.
/remote.php/dav/files/user@name/Documents/text1 read from href in PROPFIND response.
Current relative remote path: /remote.php/dav/files/user@name/Documents/text1 because both paths are not the same before this relative path.
Expected relative remote path: /Documents/text1

DeepDiver1975 · 2018-11-21T11:44:01Z

The files app work almost identical - with one exception:

path comparison is done on the decoded paths and user names

double decode is not hurting - while double encode hurts for sure

davigonz · 2018-11-21T12:57:15Z

Good news, applying the decoding approach seems to work:

Comparing next paths:

Username with @
- /remote.php/dav/files/user@name/ composed of /remote.php/dav/files/ constant and user@name, obtained from a previous GET user info operation, with no modification since is already retrieved as decoded.
- /remote.php/dav/files/user@name/Documents/text1 read from href in PROPFIND response, decoded would the same.
- Relative remote path: /Documents/text1 because both paths are the same before this relative path.
Username with space
- /remote.php/dav/files/user name/ composed of /remote.php/dav/files/ constant and user name, obtained from a previous GET user info operation, with no modification since is already retrieved as decoded.
- /remote.php/dav/files/user%20name/Documents/text1 read from href in PROPFIND response, decoded would be /remote.php/dav/files/user name/Documents/text1
- Relative remote path: /Documents/text1 because both paths are the same before this relative path.

Thanks for your help @DeepDiver1975 😉

Anyway, I was expecting to receive the @ character encoded because I read the next paragraph in RFC 1738

After reading this, I understand that @ character should be encoded, not sure why sabre dav is not doing it.

DeepDiver1975 · 2018-11-21T13:08:50Z

Just for the sake of completness ... http://sabre.io/dav/character-encoding/

davigonz · 2018-11-21T14:27:07Z

Just for the sake of completness ... http://sabre.io/dav/character-encoding/

I would highly recommend using characters from this list.

A-Z // uppercase

a-z // lowerchase

0-9 // numbers

// dash

_ // underscore

. // full stop

These characters are pretty much guaranteed to work correctly across the board. Everything else may be percent encoded, depending on the client.

Then I don't understand they are not considering @ in their tests 😕

PVince81 · 2018-11-23T15:11:00Z

I think I observed similar issues before when working on having the UI use the new DAV endpoint. See c99f5b4

I didn't know back then that it might be a Sabre problem and simply adjusted the JS side to send whatever works.

DeepDiver1975 · 2018-11-23T15:15:24Z

@PVince81 this was fixed in andoid - we can close this from my pov

davigonz mentioned this issue Nov 20, 2018

"Folder doesn't exist anymore" on OwnCloud account with a space owncloud/android#2347

Closed

phil-davis mentioned this issue Nov 20, 2018

Add acceptance tests for more variety of user Ids #33596

Closed

2 tasks

ChrisEdS added the blue-ticket label Nov 21, 2018

jesmrec mentioned this issue Nov 22, 2018

Blanks in username causes 404 in PROPFIND owncloud/ios-sdk#33

Closed

PVince81 added this to the backlog milestone Nov 23, 2018

PVince81 added the bug-analysis label Nov 23, 2018

PVince81 closed this as completed Nov 23, 2018

lock bot locked as resolved and limited conversation to collaborators Nov 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PROPFIND to get list of files doesn't properly encode @ character #33594

PROPFIND to get list of files doesn't properly encode @ character #33594

davigonz commented Nov 20, 2018 •

edited

Loading

davigonz commented Nov 20, 2018

ownclouders commented Nov 20, 2018

phil-davis commented Nov 20, 2018

phil-davis commented Nov 20, 2018

davigonz commented Nov 20, 2018

jesmrec commented Nov 20, 2018

DeepDiver1975 commented Nov 21, 2018

DeepDiver1975 commented Nov 21, 2018

davigonz commented Nov 21, 2018 •

edited

Loading

DeepDiver1975 commented Nov 21, 2018 •

edited

Loading

davigonz commented Nov 21, 2018

DeepDiver1975 commented Nov 21, 2018

davigonz commented Nov 21, 2018 •

edited

Loading

PVince81 commented Nov 23, 2018

DeepDiver1975 commented Nov 23, 2018

PROPFIND to get list of files doesn't properly encode @ character #33594

PROPFIND to get list of files doesn't properly encode @ character #33594

Comments

davigonz commented Nov 20, 2018 • edited Loading

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

davigonz commented Nov 20, 2018

ownclouders commented Nov 20, 2018

phil-davis commented Nov 20, 2018

phil-davis commented Nov 20, 2018

davigonz commented Nov 20, 2018

jesmrec commented Nov 20, 2018

DeepDiver1975 commented Nov 21, 2018

DeepDiver1975 commented Nov 21, 2018

davigonz commented Nov 21, 2018 • edited Loading

DeepDiver1975 commented Nov 21, 2018 • edited Loading

davigonz commented Nov 21, 2018

DeepDiver1975 commented Nov 21, 2018

davigonz commented Nov 21, 2018 • edited Loading

PVince81 commented Nov 23, 2018

DeepDiver1975 commented Nov 23, 2018

davigonz commented Nov 20, 2018 •

edited

Loading

davigonz commented Nov 21, 2018 •

edited

Loading

DeepDiver1975 commented Nov 21, 2018 •

edited

Loading

davigonz commented Nov 21, 2018 •

edited

Loading