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

Empty header check breaking change #8

Closed
weierophinney opened this issue Dec 31, 2019 · 3 comments
Closed

Empty header check breaking change #8

weierophinney opened this issue Dec 31, 2019 · 3 comments
Labels
Invalid This doesn't seem right

Comments

@weierophinney
Copy link
Member

The following commit causes breaking issues and probably should not have been implemented in a minor version.

zendframework/zend-diactoros@ee4bcdc#diff-5abca4d9d5693da46d10a54795b1192d

Specifically this change breaks laravel/passport ^3.0 which depends on ~1.0 of this library.

I've opened a ticket for them to patch branch 3.x, but that patch may introduce new problems. That's why I'd suggest this update be reverted from 1.x of the zend framework.


Originally posted by @soundsgoodsofar at zendframework/zend-diactoros#356

@weierophinney
Copy link
Member Author

This is problematic to back out of the 1.8 series of zend-diactoros.

The patch that introduced it, #325, did so in order to make the library comply correctly with the PSR-7 specification. Essentially, by not checking it, we had introduced a bug in our implementation.

That also means that, on the flip side, if you were relying on that behavior previously, you were relying on a bug.

If laravel/passport requires the original behavior, one solution is to do the following in your composer.json:

"require": {
    "zendframework/zend-diactoros": "~1.0"
},
"conflict": {
    "zendframework/zend-diactoros": "1.8.6"
}

Pinging @Ocramius — thoughts on reverting #325 for the 1.8 series?


Originally posted by @weierophinney at zendframework/zend-diactoros#356 (comment)

@weierophinney
Copy link
Member Author

Hey everyone. Seems that the problem lies with Symfony HttpFoundation's ServerBag class and not with Passport itself. See laravel/passport#992


Originally posted by @driesvints at zendframework/zend-diactoros#356 (comment)

@weierophinney
Copy link
Member Author

My 2 cents on the issue: I wouldn't revert the change if it was a bug fix to comply to PSR-7 more. Implementing libraries were relying on faulty behavior like @weierophinney noted.


Originally posted by @driesvints at zendframework/zend-diactoros#356 (comment)

@Xerkus Xerkus added the Invalid This doesn't seem right label May 1, 2023
@Xerkus Xerkus closed this as not planned Won't fix, can't repro, duplicate, stale May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants