Skip to content

fix: manually set modified time to SFTP files after editing#58970

Open
rikled wants to merge 4 commits intomasterfrom
fix/sftp-mtime
Open

fix: manually set modified time to SFTP files after editing#58970
rikled wants to merge 4 commits intomasterfrom
fix/sftp-mtime

Conversation

@rikled
Copy link
Copy Markdown

@rikled rikled commented Mar 16, 2026

Summary

This PR fixes SFTP file writes. When uploading a file to the SFTP server, the files original modified date is not preserved. The SFTP server typically sets the modified date to the time of upload. This leads to issues, for example, with Nextcloud Office, which expects the mtime to match the mtime of the file. Nextcloud office then repeatedly throws errors indicating that the file has been modified externally. In this PR the SFTP implementation explicitely sets the modified date of the file.

Caution

I am not sure about the knownMTimes. I set them everywhere the data is written to the SFTP server, but I'm not sure if that's correct, since I'm not entirely sure what the purpose of this memory cache is.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@juliusknorr juliusknorr added bug 3. to review Waiting for reviews labels Mar 17, 2026
@artonge artonge added this to the Nextcloud 34 milestone Mar 25, 2026
Copy link
Copy Markdown
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Looks good, but no idea if that works in practice. Have you tested it?

@rikled rikled marked this pull request as ready for review March 25, 2026 18:10
@rikled rikled requested a review from a team as a code owner March 25, 2026 18:10
@rikled rikled requested review from ArtificialOwl, come-nc and provokateurin and removed request for a team March 25, 2026 18:10
@rikled
Copy link
Copy Markdown
Author

rikled commented Mar 25, 2026

Yes, I tested it locally with Collabora. Collabora no longer shows that the file has been modified, even though it hasn't.

Copy link
Copy Markdown
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Agree with @susnux

@rikled rikled force-pushed the fix/sftp-mtime branch 2 times, most recently from 21cc6d8 to f71c382 Compare March 26, 2026 17:00
@rikled
Copy link
Copy Markdown
Author

rikled commented Mar 26, 2026

Thank you for the feedback. I've updated the code.

@rikled rikled requested review from provokateurin and susnux March 26, 2026 17:54
@github-actions
Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

rikled and others added 4 commits March 31, 2026 12:35
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
…tead

Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
Copy link
Copy Markdown
Contributor

susnux commented Mar 31, 2026

(rebased and updated psalm baseline entry)

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.

5 participants