Skip to content

ftp cannot be rewound #11597

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.

Sign up for GitHub

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

Jump to bottom
Open
mvorisek opened this issue Jul 5, 2023 · 3 comments
Open

ftp cannot be rewound #11597

mvorisek opened this issue Jul 5, 2023 · 3 comments
Labels
Bug Extension: ftp Status: Needs Triage

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Jul 5, 2023

Description

The following code:

<?php

$iter = new RecursiveDirectoryIterator('ftp://ftp.belnet.be/', \RecursiveDirectoryIterator::SKIP_DOTS);

var_dump($iter->current());
$iter->next();
var_dump($iter->current());
$iter->rewind();

Resulted in this output:

Warning: FilesystemIterator::rewind(): stream does not support seeking

But I expected this output instead:

support should not be related with seeking support

PHP Version

any

Operating System

any
@KapitanOczywisty
Copy link

What ftp wrapper does is sending NLST command and streaming response buffer and this is not seekable nor rewindable. It's possible to resend NLST command on rewind, though I don't know what side-effects this would have.

https://github.com/php/php-src/blob/master/ext/standard/ftp_fopen_wrapper.c#L724

@mvorisek
Copy link
Contributor Author

mvorisek commented Jul 5, 2023

As any file iterator is not buffered, ie. the results will change if the filesystem changes, there is no reason to not rewind the ftp iterator as well. Basically a rewind operation imply the state after an iterator is created.

@mvorisek mvorisek mentioned this issue Jul 5, 2023
Merged
nicolas-grekas added a commit to symfony/symfony that referenced this issue Jul 6, 2023
@nicolas-grekas
bug #50884 [Finder] Fix initial directory is opened twice (mvorisek)
99fa865 
This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Finder] Fix initial directory is opened twice

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #50851
| License       | MIT
| Doc PR        | no

The issue was introduced in #8120 which was coded wrongly for several reasons:
- `seekable` support is about file/stream content seeking, not about rewind support, non-seekable stream handler can support rewind - it worked by coincidence
- the original code involved additional directory open (syscall), which is/was slow
- 1st rewind of DirectoryIterator is implicit, ie. it is not needed - again, skipping the rewind worked by coincidence
- 2nd+ rewind must fail if it is not supported, the original code silently skipped it

The test from #8120 is kept and is passing.

I also improved the ftp support detection to make the test stricter.

To support rewind for ftp I opened - php/php-src#11597 - but this Symfony PR does not need it.

Commits
-------

d3f03be [Finder] Fix initial directory is opened twice
@nielsdos
Copy link
Member

nielsdos commented Jul 6, 2023

Rewinding is implemented as seeking back to position 0. The entry for seek in php_ftp_dirstream_ops is non-existent. Perhaps an implementation could be made for seeking to 0.
The question then is what the FTP spec says about an NLST being interrupted by another NLST.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Extension: ftp Status: Needs Triage
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mvorisek @nielsdos @Girgias @KapitanOczywisty