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

Correct incorrectly typed X-OC-Mtime header #3793

Merged
merged 1 commit into from
Mar 10, 2017

Conversation

stefan-squareweave
Copy link
Contributor

This bug causes file uploads to show an error message when using the Postgres database and a storage backend that uses the filecache system.

The error message you get in the HTTP response is something like:

<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Doctrine\DBAL\Exception\DriverException</s:exception>
  <s:message>An exception occurred while executing 'UPDATE "oc_filecache" SET "path_hash" = ?, "path" = ?, "parent" = ?, "name" = ?, "mimepart" = ?, "mimetype" = ?, "size" = ?, "mtime" = ?, "storage_mtime" = ?, "encrypted" = ?, "etag" = ?, "permissions" = ?, "checksum"=? WHERE ("path_hash" &lt;&gt; ? OR "path" &lt;&gt; ? OR "parent" &lt;&gt; ? OR "name" &lt;&gt; ? OR "mimepart" &lt;&gt; ? OR "mimetype" &lt;&gt; ? OR "size" &lt;&gt; ? OR "mtime" &lt;&gt; ? OR "storage_mtime" &lt;&gt; ? OR "encrypted" &lt;&gt; ? OR "etag" &lt;&gt; ? OR "permissions" &lt;&gt; ? OR "checksum" &lt;&gt; ? OR "path_hash" IS NULL OR "path" IS NULL OR "parent" IS NULL OR "name" IS NULL OR "mimepart" IS NULL OR "mimetype" IS NULL OR "size" IS NULL OR "mtime" IS NULL OR "storage_mtime" IS NULL OR "encrypted" IS NULL OR "etag" IS NULL OR "permissions" IS NULL OR "checksum" IS NULL) AND "fileid" = ? ' with params ["2b720af9543ff5bc9757e518c0de0d43", "files\/some-testfile.txt", 2, "some-testfile.txt", 10, 11, 4, "1489115007.243", 1489124931, 0, "58c23e439fb08", 27, null, "2b720af9543ff5bc9757e518c0de0d43", "files\/some-testfile.txt", 2, "some-testfile.txt", 10, 11, 4, "1489115007.243", 1489124931, 0, "58c23e439fb08", 27, null, 110]:

SQLSTATE[22P02]: Invalid text representation: 7 ERROR:  invalid input syntax for integer: "1489115007.243"</s:message>
</d:error>

As can be seen, the value passed in for the mtime column is a string representation of a floating point number. In postgres this column is an integer, and so the update fails.

@mention-bot
Copy link

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

Signed-off-by: Stefan Schneider <stefan.schneider@squareweave.com.au>
@rullzer
Copy link
Member

rullzer commented Mar 10, 2017

Thnx for the PR!

Wouldn't trying to cast to an int make more sense?

@stefan-squareweave
Copy link
Contributor Author

Hello. Did you mean just replacing the intval($mtimeStr) call with (int)$mtimeStr? I think they should be equivalent. If you mean my is_numeric check, I just put that in as it makes it extra clear for external clients what the required type is.

Before this PR it would have been ambiguous whether the type of the X-OC-Mtime header was just unix timetamp, or both unix timestamp and parseable date string (either would work). To check which was correct this I searched the internet and found two external clients: the owncloud client, and lftp (!). Both of them passed in unix timestamps, so I think making it just a unix timestamp is correct.

@rullzer
Copy link
Member

rullzer commented Mar 10, 2017

Sorry I was to quick (mobile). Looks good and works here!

@LukasReschke LukasReschke merged commit 1045bf4 into nextcloud:master Mar 10, 2017
danxuliu added a commit that referenced this pull request Nov 27, 2017
This commit extends the changes introduced in pull request #3793 also to
chunked uploads.

The "sanitizeMTime" method name is the same used in the equivalent pull
request to this one from ownCloud (28066).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu added a commit that referenced this pull request Nov 27, 2017
This commit extends the changes introduced in pull request #3793 also to
chunked uploads.

The "sanitizeMTime" method name is the same used in the equivalent pull
request to this one from ownCloud (28066).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants