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

Activity message on file upload (dropbox target): Server does not support X-OC-MTime #6797

Closed
mmattel opened this issue Oct 7, 2018 · 6 comments

Comments

@mmattel
Copy link
Contributor

mmattel commented Oct 7, 2018

Expected behaviour

Upload file

Actual behaviour

File not synced with error message: Server does notsupport X-OC-MTime
Shown in activity, not synced
Files shows red crossed sync button
After some minutes, the file gets green and the client error message is gone
The file is also visible via the browser

Steps to reproduce

  1. Create a dropbox mount on the server and let the mount sync to the client
  2. Add a file via local filesystem eg test.txt (no upper / lower case issue as it is already lower case)
  3. Watch the client activity

Server configuration

Operating system: Ubuntu 16.04

Web server: nginx

Database: mysql

PHP version: 7.0

ownCloud version: 10.0.10 (Stable)

Storage backend (external storage): local, smb, ftp, dropbox

Client configuration

Client version: 2.5.0

Operating system: W10x64

OS language: DE

Logs

Please use Gist (https://gist.github.com/) or a similar code paster for longer
logs.

  1. Client logfile:

  2. Web server error log:
    no issues

  3. Server logfile: ownCloud log (data/owncloud.log):
    no issues

@ogoffart
Copy link
Contributor

ogoffart commented Oct 8, 2018

The server must return the X-OC-MTime: accepted header.
@PVince81 Any reason this header would not be present?

@PVince81
Copy link
Contributor

PVince81 commented Oct 8, 2018

As far as I remember, the upload code tries to do a touch() on the target storage to adjust the mtime. If the target server doesn't support touch() and returns false, it assumes the mtime was not accepted and doesn't return that header.

Looking at https://github.com/owncloud/core/blob/v10.0.10/lib/private/Files/Storage/Flysystem.php#L224 which is the base class from the Dropbox implementation, it looks like setting mtime isn't supported by the Flysystem API at all.

@PVince81
Copy link
Contributor

PVince81 commented Oct 8, 2018

@ogoffart wasn't the older client behavior different, where if the header isn't returned it would try to do a PROPPATCH with the mtime afterwards ? Now even if it did, the mtime would not be settable due to the same reasons stated above.

@PVince81
Copy link
Contributor

PVince81 commented Oct 8, 2018

also see https://github.com/owncloud/core/blob/v10.0.10/apps/dav/lib/Connector/Sabre/File.php#L99

now this feels like a design inconsistency: in oc_filecache we have two columns, "storage_mtime" and "mtime" and the purpose of "mtime" is to be able to hold whatever mtime the client sent while the "storage_mtime" is whatever value the storage has now. In an ideal case they are identical but for cases where the storage does not support "touch", "storage_mtime" would be whatever value the storage assigns the file after writing is completed.

A possible fix would be to change the current behavior to always accept the mtime and simply store it there. Need to think of potential side effects though.

@ogoffart
Copy link
Contributor

ogoffart commented Oct 9, 2018

@ogoffart wasn't the older client behavior different, where if the header isn't returned it would try to do a PROPPATCH with the mtime afterwards ?

We do no longer call propatch since we got rid of the neon dependency in mirall 2.1. (At the time already, we were no longer supporting server which did not support the X-OC-MTime. (And I thought that it would be always accepted if the server supports it)

In other words, this is an error since 2.1, and nobody who had ownCloud > 6.0 complained about it (that i know of)

But maybe we can just remove this error as it does not really need to be an error in the first place.

ogoffart added a commit that referenced this issue Oct 9, 2018
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

#6797
ogoffart added a commit that referenced this issue Oct 12, 2018
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

#6797
guruz pushed a commit that referenced this issue Oct 15, 2018
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

#6797
@mmattel
Copy link
Contributor Author

mmattel commented Jun 18, 2019

closing as #6800 has been merged

@mmattel mmattel closed this as completed Jun 18, 2019
er-vin pushed a commit to nextcloud/desktop that referenced this issue Nov 27, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Nov 27, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 2, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 8, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 10, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 10, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
github-actions bot pushed a commit to nextcloud/desktop that referenced this issue Dec 11, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
github-actions bot pushed a commit to nextcloud/desktop that referenced this issue Dec 14, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 14, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 14, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
er-vin pushed a commit to nextcloud/desktop that referenced this issue Dec 15, 2020
…cepted header

If the server does not set the mtime, it is not a big problem for the
synchronisation.

The test was used before so we could do a PROPPATCH for server that did not
support this header. But now that all server supports that we don't need to
to the check. (We do not do the PROPPATCH since we got rid of the neon
dependency)

Apparently, it may happen that some backend don't support setting mtime
and this can lead to this error.

owncloud/client#6797
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants