Skip to content

fix(files_external): reset EOF when seeking in SFTPReadStream - #66002

Merged
provokateurin merged 1 commit into
nextcloud:masterfrom
tcarrondo:fix/sftp-read-stream-seek-after-eof
Nov 3, 2026
Merged

provokateurin merged 1 commit into
nextcloud:masterfrom
tcarrondo:fix/sftp-read-stream-seek-after-eof

Conversation

@tcarrondo

Copy link
Copy Markdown
Contributor

Summary

SFTPReadStream::seekTo() resets the positions and the buffer, but not $this->eof. After a read has reached the end of the remote file, a seek reports success, yet every later read returns an empty string.

OC\Preview\Movie::getSparseFile() runs into this on SFTP storage: it reads the moov atom at the end of the video, seeks back to 0 and copies the start of the file, which copies 0 bytes. ffmpeg then gets a sparse file of zeros, and the preview fails on every request.

This PR clears the flag in seekTo() and adds unit tests for SFTPReadStream, with a fake server stubbed at the SFTPReflection seam, as in SFTPWriteStreamTest. testSeekBackAfterEof fails without the fix.

Checked on a 35.0.2 instance with an SFTP mount. Before: the movie preview failed for two MP4s (11 and 18 MB, moov at the end), and the sparse file was all zeros. With the fixed class loaded: both previews are generated.

TODO

  • Backport to stable35 (and older supported branches)

Checklist

AI (if applicable)

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

🤖 Generated with Claude Code

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

makes sense

@susnux
susnux requested review from a team, icewind1991, leftybournes and salmart-dev and removed request for a team October 29, 2026 12:40
@susnux susnux added this to the Nextcloud 36 milestone Oct 29, 2026
@susnux

susnux commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

/backport to stable35

@tcarrondo

Copy link
Copy Markdown
Contributor Author

The remaining failure in "No DB unit tests (PHP 8.3)" is in core tests (Test\Avatar\GuestAvatarTest::testGet, plus errors in Test\Memcache\APCuTest and the Files\Storage\Wrapper tests), not in files_external. The same suite passes on PHP 8.5. Could someone re-run that job? I can't trigger it from the fork.

@provokateurin

Copy link
Copy Markdown
Member

The failing preview test was fixed on master. Could you rebase your PR?

Once a read reached the end of the remote file, the eof flag stayed set
after a seek, and every later read returned an empty string. The movie
preview reads the moov atom at the end of a video and then copies the
file from the start, so the sparse file was all zeros and ffmpeg failed.

Fixes nextcloud#66001

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tiago Carrondo <tiago@carrondo.pt>
@tcarrondo
tcarrondo force-pushed the fix/sftp-read-stream-seek-after-eof branch from 48581e1 to c39dcd5 Compare November 3, 2026 00:48
@tcarrondo
tcarrondo requested a review from a team as a code owner November 3, 2026 00:48
@provokateurin
provokateurin merged commit 5b89730 into nextcloud:master Nov 3, 2026
181 checks passed
@welcome

welcome Bot commented Nov 3, 2026

Copy link
Copy Markdown

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

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.

[Bug]: SFTPReadStream returns no data after seeking back from EOF (movie previews fail on SFTP storage)

3 participants