Trying to access array offset on value of type bool

[bjora857]

Bug Report

Error when using streams and S3 on php 7.4 Works fine on php 7.3 But breaks on php 7.4

How to reproduce

Upload a file to s3 using fopen

Storage::disk('some-s3-disk');
$disk->put('test.jpg', fopen($imageUrl, "r"));

// Works fine
// $disk->put('test.jpg', file_get_contents($mageUrl));

[frankdejonge]

Hi @bjora857, what's the value of the $imageUrl variable? Are you trying to upload a file by opening a URL?

[GrahamCampbell]

$disk->put('test.jpg', file_get_contents($mageUrl));

Are you actually calling that with the typo in the variable name... That could explain why we are having an array access on "false".

[bjora857]

Yes, imageUrl. something like $imageUrl = 'https://happywall-statix.imgix.net/category/hero/landscapes.jpg';

[bjora857]

$disk->put('test.jpg', file_get_contents($mageUrl));

Are you actually calling that with the typo in the variable name... That could explain why we are having an array access on "false".

Sorry. That was just a typo. it should of course be $imageUrl. The point was, that it works fine if I use file_get_contents, but fails when using fopen. And this is something that stopped working after upgrading to php7.4

[frankdejonge]

It's because PHP can't determine the size (there's no file backing it), I'll make sure it'll check for the existence of the key first. Although, I must say, this is not really what this library was intended for 🤷‍♂

[GrahamCampbell]

Maybe the stream size method should actually throw an exception on all versions, intentionally breaking this sort of usage. I'd say it was not a BC break, since a BC break is a change in the intended behaviour, not just edge cases (otherwise all bug fixes would be BC breaks).

[frankdejonge]

It's not a BC break for sure, totally agree with that. The method should be passed file handles, the fopen call on a url doesn't provide one.

[GrahamCampbell]

I'll send a PR. One sec.

[frankdejonge]

@GrahamCampbell while I agree it's not a BC break, I think supporting this is harmless. It'll just be limited in optimisation for some cases for AWS. I'm working on making the size retrieving work for this case. No PR needed.

[GrahamCampbell]

https://github.com/thephpleague/flysystem/pull/1119 is what I have for now. I suppose some code can go before my throw to handle other cases, like possibly AWS, like you say.

[frankdejonge]

@GrahamCampbell I've added some remarks to the PR.

[GrahamCampbell]

Thanks. Just pulling down the repo to my machine to make the changes.

[frankdejonge]

👍

[GrahamCampbell]

Pushed. Provisionally, the tests are passing (not all of the runs completed yet).

[bjora857]

You guys are awesome. @GrahamCampbell @frankdejonge

[GrahamCampbell]

Just to be clear, your code will now not work on either version now. You need to first download the image to disk, then upload it.

[GrahamCampbell]

Oh, actually, I think it should be fine, looking at the AwsS3Adapter code. ;)

[frankdejonge]

I think his code will work now. It just not 100% clear how the aws sdk uploads it. Perhaps in a not very efficient manner.

[GrahamCampbell]

I did notice some minor errors in the phpdoc though: https://github.com/thephpleague/flysystem-aws-s3-v3/pull/187. ;)

[bjora857]

I can confirm my code works after the fix. My usecase was to upload a large image file to s3 from a url, without being restricted by the php memory limit. Anyway, I'm very happy that it got resolved in less than an hour.

[frankdejonge]

👍 @bjora857 please consider sponsoring open source authors to keep making this possible.

[GrahamCampbell]

My usecase was to upload a large image file to s3 from a url

You could also stream the file to tmp, then stream it back into php as a resource. That way, if the file download ever fails, you don't end up with a partially uploaded file, or some other cryptic errors.