Skip to content

Conversation

hertz1
Copy link

@hertz1 hertz1 commented Jun 23, 2025

Hi there! 👋

There was a need for FTP stream support in a project I'm working on, so I thought I'd upstream the work I've done.

As of now, I've only implemented support for addFromDisk(...), add('ftp://...') is not currently supported, you need to have the disk configured in your config/filesystems.php file.

Let me know your thoughts.

@jszobody
Copy link
Member

This is nice, I like it. I would probably merge this if it gets built out a bit more. This could probably support a writeable stream as well, yeah?

@hertz1
Copy link
Author

hertz1 commented Jul 22, 2025

@jszobody thanks for the feedback. Writable streams are used for the save to disk feature, is that right?

I'm trying to clarify how the saveToDisk() feature for writable streams is supposed to work. My understanding is that it saves the zip to disk. Is it also intended to trigger a download to the browser?

Currently, it seems that if I call saveToDisk(), I also need to stream the zip to php://output for it to function correctly. This feels a bit unexpected, especially since other file models appear to stream directly to the intended disk (local or S3) without this extra step. However, those also don't work without streaming to php://output.

For example, the following snippet will trigger a download, but it will fail:

Route::get('test', function () {
    $zip = \STS\ZipStream\Facades\Zip::create('package.zip', ['/path/to/Some File.pdf']);
    $zip->saveTo('path/to/folder');

    return response();
});

Am I missing something about the intended workflow here?

@jszobody
Copy link
Member

No it's not supposed to trigger a browser download when you save to disk. I just fixed that issue and released v5.6.

You should now be able to save to a path or disk, with no browser download triggered. I had to suppress the HTTP headers that were automatically being sent by the underlying zipstream package.

@hertz1 hertz1 force-pushed the master branch 2 times, most recently from eadd84c to 4eab6df Compare July 27, 2025 20:18
@hertz1
Copy link
Author

hertz1 commented Jul 29, 2025

@jszobody I believe this should be feature complete now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants