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

Release 1.6.0 #218

Merged
merged 1 commit into from
Apr 9, 2023
Merged

Release 1.6.0 #218

merged 1 commit into from
Apr 9, 2023

Conversation

nicolas-grekas
Copy link
Collaborator

No description provided.

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me. suggested linking to the RFC for the encoded characters.

as the userinfo encoding is a change in behaviour, and a potential BC break in some super edge cases, i wonder if we should wait a few days if php-fig/fig-standards#1298 coalesces into an agreement and if we change behaviour only change it once...

CHANGELOG.md Outdated Show resolved Hide resolved
@nicolas-grekas
Copy link
Collaborator Author

I have zero confidence in the speed of anything when the FIG is involved (except maybe for negative speed ;) )
About the BC break, there can't be any, because the resulting URLs are invalid right now, so this is only a bugfix (as discussed on php-http/discovery#222)

@nicolas-grekas
Copy link
Collaborator Author

I think we should instead cover those edge cases in https://github.com/php-http/psr7-integration-tests/, and encourage mainstream implementations to adopt this test suite. On this we can have a leverage as a community. Not on the FIG.

@dbu
Copy link
Contributor

dbu commented Apr 1, 2023

About the BC break, there can't be any, because the resulting URLs are invalid right now, so this is only a bugfix (as discussed on php-http/discovery#222)

in the discussion in php-fig/fig-standards#1298 it was mentioned that encoding immediately changes what is returned in getUserInfo. that said, as that method concatenates username and password, separated by :, if somebody was going that route they would still have had a bug when the password contains a :...

i guess the changelog is good enough in case somebody hits that edge case. i think my concern is more about xkcd than anything relevant... so lets go.

@nicolas-grekas
Copy link
Collaborator Author

Thanks for jumping into the topic and for the reviews!

@Nyholm
Copy link
Owner

Nyholm commented Apr 9, 2023

Thank you

@Nyholm Nyholm merged commit bf4aebd into Nyholm:master Apr 9, 2023
12 of 13 checks passed
@nicolas-grekas nicolas-grekas deleted the changelog branch April 9, 2023 08:48
@nicolas-grekas
Copy link
Collaborator Author

Thanks for merging and tagging!

@dbu
Copy link
Contributor

dbu commented Apr 10, 2023

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

Successfully merging this pull request may close these issues.

None yet

4 participants