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

Remove PhpInputStream #154

Merged

Conversation

weierophinney
Copy link
Member

Q A
Documentation yes
Bugfix no
BC Break yes
New Feature no
RFC no
QA no

Description

The main PhpInputStream existed was because prior to PHP 5.6, php://input was read-once and non-seekable.
As of 5.6, it's reusable AND seekable, which means there's no reason to keep the special implementation, particularly as it causes bugs (see #150)

This patch:

  • Removes the class
  • Updates the ServerRequest to, when a body of php://input is detected, create a read-only Stream instance with that value.

Fixes #150

The main reason this existed was because prior to PHP 5.6, `php://input` was read-once and non-seekable.
As of 5.6, it's reusable AND seekable, which means there's no reason to keep the special implementation, particularly as it causes bugs (see laminas#150)

This patch:

- Removes the class
- Updates the `ServerRequest` to, when a body of `php://input` is detected, create a read-only `Stream` instance with that value.

Signed-off-by: Matthew Weier O'Phinney <[email protected]>
Signed-off-by: Matthew Weier O'Phinney <[email protected]>
@weierophinney weierophinney force-pushed the fix/php-input-stream-get-contents branch from 34d919c to bfbeacd Compare May 2, 2023 22:24
@Xerkus Xerkus linked an issue May 2, 2023 that may be closed by this pull request
@Xerkus Xerkus merged commit bc59ac9 into laminas:3.0.x May 2, 2023
@Xerkus
Copy link
Member

Xerkus commented May 2, 2023

Thank you.

@weierophinney weierophinney deleted the fix/php-input-stream-get-contents branch May 3, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PhpInputStream::getContent() inconsistency
2 participants