polyfill icon indicating copy to clipboard operation
polyfill copied to clipboard

[PHP 8.1] Add `CURLStringFile` polyfill

Open Ayesh opened this issue 4 years ago • 9 comments

PHP 8.1 adds a new class CURLStringFile, that works similar to CURLFile, but does not require a path to a file, and accepts a string of file contents instead. This can be polyfilled with a CURLFile sub class that uses a data:// URI inside a constructure and calling CURLFile::__construct with that data URI.

Closes #333.

I will leave this as a draft because I'm not sure how to write tests for this. I'm thinking a PHP CLI server that echoes $_FILES, and a test that checks the returned content for file(s) that it uploaded.

Ayesh avatar Feb 17 '21 05:02 Ayesh

If we add a test case (as you, I don't know which one exactly), it should with the native one and with the polyfilled one (when curl is not installed), so that we can prove that this is really a polyfill.

nicolas-grekas avatar Feb 17 '21 07:02 nicolas-grekas

I'm thinking a PHP CLI server that echoes $_FILES, and a test that checks the returned content for file(s) that it uploaded.

Sounds like a plan. You can find an example of using the CLI webserver in tests here:

https://github.com/symfony/symfony/blob/6ae59a9a171695ddef5294898681c060e8c1f3fe/src/Symfony/Component/HttpFoundation/Tests/ResponseFunctionalTest.php#L22-L29

derrabus avatar Feb 17 '21 08:02 derrabus

Thanks a lot for the feedback.

I totally forgot about the CURLStringFile::$data mutability, and like @nicolas-grekas said, we might (unfortunately) need to use magic methods here. I updated with __get, __set and __isset methods to update the CURLStringFile::$name if $data was modified. It feels a bit... yucky... to do this, but I'm looking for any suggestions.

Ayesh avatar Feb 17 '21 16:02 Ayesh

Thank you so much for the help @nicolas-grekas - I have made the changes. I will also push a test. It passes on PHP >= 7.4, and I'm still pulling my hair on why it fails on older PHP versions.

Ayesh avatar Mar 12 '21 14:03 Ayesh

I will also push a test. It passes on PHP >= 7.4, and I'm still pulling my hair on why it fails on older PHP versions.

that's because your implementation relies on the fact that curl knows how to deal with a CurlFile object. But that CurlFile won't exist there.

stof avatar Mar 12 '21 14:03 stof

Thank you @stof. Looking at CURLFile doc, it says CURLFile should available in PHP >= 5.5. My direction is that the CLI server failed to start at all in older PHP versions.

Ayesh avatar Mar 12 '21 14:03 Ayesh

I tested with a PHP 7.3 setup, it indeed turns out that the problem is that CURLFile only supports streams in PHP >= 7.4, just like @stof mentioned. I updated the tests and CURLStringFile to be only available in PHP >= 7.4, so I can mark this PR for review. All tests pass.

For PHP 7.0 through 7.3, I think we could get it working by temporarily writing the contents to a file, but I'm not sure if the effort is worth it. I will open the PR for review either way, and will happily work on support for older PHP versions if you think it's a worthwhile idea. 🤞🏼

Thank you.

Ayesh avatar Mar 12 '21 17:03 Ayesh

Can you please rebase?

nicolas-grekas avatar Apr 19 '21 10:04 nicolas-grekas

@Ayesh can you please rebase and check @stof's comments?

nicolas-grekas avatar Oct 26 '21 17:10 nicolas-grekas

Thank you @Ayesh.

nicolas-grekas avatar Jan 26 '23 09:01 nicolas-grekas