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

users cannot download multiple files as a single zip/tar file in filesender 2.43 #1733

Open
StCyr opened this issue Dec 22, 2023 · 6 comments · Fixed by #1735
Open

users cannot download multiple files as a single zip/tar file in filesender 2.43 #1733

StCyr opened this issue Dec 22, 2023 · 6 comments · Fixed by #1735

Comments

@StCyr
Copy link

StCyr commented Dec 22, 2023

It seems that you ran composer on php-8 and that you've dropped the following in filesender 2.43:lib/vendor/composer/platform_check.php:

if (!(PHP_VERSION_ID >= 80100)) {
$issues[] = 'Your Composer dependencies require a PHP version ">= 8.1.0". You are running ' . PHP_VERSION . '.';
}

for some reason it prevents users from downloading multiple files as a single zip/tar file

@monkeyiq
Copy link
Contributor

It seems that the first issue can be gotten around with a blunt axe using the following.

 composer update  --ignore-platform-req=php

As there are two fairly distinct paths in the code for downloading multiple files as a single zip/tar I assume we are talking about the case here for not encrypted files. Archives for not encrypted files are generated on the server and will likely be effected by this php 8.x issue if the code has troubles running on 7.x.

I have made a branch with the php version check ignored at https://github.com/monkeyiq/filesender/tree/2023/dec/php8-server-archives I have not tested that on a php7 system. I am wondering if that branch solves the archive problem in your environment?

If not if there are any messages in /opt/filesender/log that might shine some light on a next move.

@madsi1m
Copy link
Contributor

madsi1m commented Dec 27, 2023

I don't think it is a php version bug as i see it in php 8.3, above MR #1735 fixes it for me.

@StCyr
Copy link
Author

StCyr commented Jan 1, 2024

composer update --ignore-platform-req=php

we don't have composer installed on our filesender server

I don't think it is a php version bug as i see it in php 8.3, above MR #1735 fixes it for me.

I don't think your PR solves this particular issue: Our filesender server works perfectly well with php7 after we've replaced the lib/vendor/composer/platform_check.php file with the one from version 2.42

The problem of this particular issue is that some part of your code say that they require php8 while your documentation doesn't say so.

The solutions are either (a) filesender requires php8 and you should update your documentation accordingly, or (b) filesender doesn't require php7 and you should fix your lib/vendor/composer/platform_check.php file

@madsi1m
Copy link
Contributor

madsi1m commented Jan 1, 2024

Looking at that code it seems it was added when composer updated guzzle. Looking at guzzle lib it says it needs php: ^7.2.5 || ^8.0 so i guess in theory we could remove that if statement >= 70250 or revert it back to 70300 ?

@madsi1m madsi1m reopened this Jan 1, 2024
@monkeyiq
Copy link
Contributor

monkeyiq commented Jan 1, 2024

composer update --ignore-platform-req=php

we don't have composer installed on our filesender server

I mentioned above that I have run this composer command and pushed that code to a branch in my repository at
https://github.com/monkeyiq/filesender/tree/2023/dec/php8-server-archives

I have not tested that on a php7 system. I am wondering if that branch solves the archive problem in your environment? If it does solve the problem I will make a composer-run.sh with this in it and that will be the official way to update with composer until such time as we officially drop a support for php 7.x. This will mean that you can just install a release on a php7 system without any special commands or need for composer etc.

I don't think it is a php version bug as i see it in php 8.3, above MR #1735 fixes it for me.

I don't think your PR solves this particular issue: Our filesender server works perfectly well with php7 after we've replaced the lib/vendor/composer/platform_check.php file with the one from version 2.42

Ah ok, so the above branch will very likely succeed too! There is no platform_check in that branch https://github.com/monkeyiq/filesender/tree/2023/dec/php8-server-archives/lib/vendor/composer

The problem of this particular issue is that some part of your code say that they require php8 while your documentation doesn't say so.

The solutions are either (a) filesender requires php8 and you should update your documentation accordingly, or (b) filesender doesn't require php7 and you should fix your lib/vendor/composer/platform_check.php file

I think for now the option (b) is what I have been pursuing, at least for FileSender 2.x releases. I may roll the changes from my branch to development shortly if that makes it easier to test on your system.

@monkeyiq
Copy link
Contributor

@StCyr Do downloads still not work properly in php 8+ with the above merged update from #1735 ?

I think there is a lot of other information in this report about php 7 vs php 8. It seems reasonable that we only want to support php 8.1+ in FileSender 3.0 as php 8.1 is the minimal version that still has some security support.

If you are still seeing issues with 1735 mentioned above and file download I am very happy to investigate those.

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 a pull request may close this issue.

3 participants