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

Fix case-sensitivity in discovery #134

Merged
merged 2 commits into from Jan 10, 2018

Conversation

Projects
None yet
2 participants
@strk
Contributor

strk commented Mar 18, 2017

Fixes #133

@marcoceppi

I don't think this properly addresses the bug and I'm not sure it's exactly what we'd want this method to do.

Show outdated Hide outdated Auth/OpenID/Parse.php
if (!preg_match($regexp, $text, $match)) {
return false;
}
$match = $match[0];

This comment has been minimized.

@marcoceppi

marcoceppi Mar 19, 2017

Member

I'm concerned with removing this. preg_match returns an array the first element is the whole line, the second element is the text capture from $regexp. The purpose of this method is to just return the first line.

@marcoceppi

marcoceppi Mar 19, 2017

Member

I'm concerned with removing this. preg_match returns an array the first element is the whole line, the second element is the text capture from $regexp. The purpose of this method is to just return the first line.

This comment has been minimized.

@strk

strk Mar 22, 2017

Contributor

Did my comment #134 (comment) address your concern ? You can check by yourself that it is expected that the match function returns an array, at least from the function used to extract links (openid rel) from the page.

@strk

strk Mar 22, 2017

Contributor

Did my comment #134 (comment) address your concern ? You can check by yourself that it is expected that the match function returns an array, at least from the function used to extract links (openid rel) from the page.

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Mar 20, 2017

Contributor
Contributor

strk commented Mar 20, 2017

@strk strk referenced this pull request Mar 22, 2017

Merged

Login via OpenID-2.0 #618

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Apr 8, 2017

Contributor

@marcoceppi did I not address your concern ? I'm using this change in production with PHP-5.6 in case it helps accepting it (btw: no CI keeping an eye on the code?)

Contributor

strk commented Apr 8, 2017

@marcoceppi did I not address your concern ? I'm using this change in production with PHP-5.6 in case it helps accepting it (btw: no CI keeping an eye on the code?)

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Apr 8, 2017

Contributor

For the record: this change was also proposed downstream to GNUSocial (after successfully running it in production for some time now): https://git.gnu.io/gnu/gnu-social/merge_requests/140

Contributor

strk commented Apr 8, 2017

For the record: this change was also proposed downstream to GNUSocial (after successfully running it in production for some time now): https://git.gnu.io/gnu/gnu-social/merge_requests/140

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Apr 21, 2017

Contributor

@marcoceppi Wordpress plugin is also affected (surely using your library): diso/wordpress-openid#48

Contributor

strk commented Apr 21, 2017

@marcoceppi Wordpress plugin is also affected (surely using your library): diso/wordpress-openid#48

mattl pushed a commit to foocorp/gnu-social that referenced this pull request Jul 11, 2017

Fix OpenID discovery in pages using uppercase <HEAD> tag
Closes #60

Equivalent change was proposed upstream:
openid/php-openid#134
@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Jan 9, 2018

Contributor

@marcoceppi hello ?

Contributor

strk commented Jan 9, 2018

@marcoceppi hello ?

@marcoceppi marcoceppi merged commit 8c3a0ae into openid:master Jan 10, 2018

@strk strk deleted the strk:case-sensitive-discovery branch Jan 10, 2018

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Jan 10, 2018

Contributor

thanks

Contributor

strk commented Jan 10, 2018

thanks

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