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

V3 getBody()->getContents() no longer returns full stream on second call #160

Open
BackEndTea opened this issue May 8, 2023 · 3 comments

Comments

@BackEndTea
Copy link

BackEndTea commented May 8, 2023

BC Break Report

Q A
Version 3.0.0

Summary

In v2, the default the default behaviour of the ServerRequestyFActory was to return a request object, where you could get the contents of the body multiple times. in v3, every new read of getContents() will result in an empty string. (Casting the body to a string still works as before).

Since the way it currently works is correct according to PSR implementation, it should probably be updated in the migration guide. And it should not be changed back to the v2 implementation

Previous behavior

getBody->getContents would return the full stream every time

Current behavior

getBody->getContents will return the full stream the first time, and then an empty string on consecutive calls.

How to reproduce

Send a post request and call request->getBody()->getContents() multiple times.

@Xerkus Xerkus added Invalid This doesn't seem right and removed BC Break labels May 8, 2023
@Xerkus
Copy link
Member

Xerkus commented May 8, 2023

This is expected behavior as per PSR interface getContents() is stream pointer aware:

Returns the remaining contents in a string

For multiple invocations of getContents() stream must be rewound. It can not be rewound if it is not seekable.

Previous implementation was bugged. See #150

@Xerkus Xerkus closed this as not planned Won't fix, can't repro, duplicate, stale May 8, 2023
@Xerkus Xerkus added Documentation Won't Fix This will not be worked on and removed Invalid This doesn't seem right Won't Fix This will not be worked on labels May 8, 2023
@Xerkus Xerkus reopened this May 8, 2023
@Xerkus
Copy link
Member

Xerkus commented May 8, 2023

Reopening as documentation issue. Mention of behavior fix can be added to migration guide.

@BackEndTea
Copy link
Author

Thanks, ill try snd update the docs tomorrow

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

No branches or pull requests

2 participants