Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.15k stars 75 forks source link

Breaking change from 1.5.1 => 1.6.0 due to fseek on new streams? #244

Closed calvinalkan closed 1 year ago

calvinalkan commented 1 year ago

I have read all the related discussions around https://github.com/Nyholm/psr7/pull/217 and can't understand why this was merged.

Take the following test:

/**
 * @test
 */
public function possible_regression_on_version_1_6_0() :void
{
    $stream = Stream::create('foo');

    $stream->write('bar');
    $stream->seek(0);

    $this->assertSame('foobar', $stream->getContents());
}

On 1.5.1 this test passes.

On 1.6.0:

Failed asserting that two strings are identical.
Expected :'foobar'
Actual   :'bar'

What is the point of this code?

if (\is_string($body)) {
    $resource = \fopen('php://temp', 'rw+');
    \fwrite($resource, $body);
    \fseek($resource, 0);
    $body = $resource;
}

Any call to write() overwrites any previous contents on a newly created stream?

nicolas-grekas commented 1 year ago

Thanks for the report. This has been changed in #217. You might also want to check #99 on the topic. TL;DR, your test case was relying on an unspecified behavior of PSR-7. It's thus implementation specific. I'm closing as this change of behavior is expected in v1.6.

calvinalkan commented 1 year ago

You might also want to check #99 on the topic.

I have and understand it's not specified.

It's thus implementation specific

True. We have been using nyholm/psr7 in a private project.

And 1.6.0 changed the specific implementation of nyholm/psr7, causing a complete BC break.

nicolas-grekas commented 1 year ago

Sorry about that. You know the meme: every bugfix is a BC break for someone...