FriendsOfSymfony / FOSOAuthServerBundle

A server side OAuth2 Bundle for Symfony
1.09k stars 450 forks source link

HttpFoundation 4 - "A non well formed numeric value encountered" #528

Open cvele opened 6 years ago

cvele commented 6 years ago

OAuth2ServerException::getHttpResponse is attempting to pass the result of OAuth2ServerException::getHttpCode() to Symfony Response object - which results in a Uncaught PHP Exception ErrorException: "Notice: A non well formed numeric value encountered"

Quick debug:

Symfony\Component\HttpFoundation\Response::__construct in it's method signature requires http status argument to be integer. In the case of this bundle http status codes are held in OAuth2\OAuth2 as constants with string assigned values eg. HTTP_UNAUTHORIZED = '401 Unauthorized'.

I'm using Symfony HttpFoundation at version 4.0 and OAuth bundle is fetched from master (tried 1.6 branch too).

Would gladly send a PR that would set those constant codes to pure integers, but I'm not sure if there will be any other repercussions (are they used anywhere else?).

cvele commented 6 years ago

Looking at the issue again, I'm wondering if there is a special reason why are the http status codes held in OAuth2 class and not fetched from HttpFoundation Response class, seeing how the bundle has a clear dependency on mentioned package?

dinamic commented 6 years ago

The problem seems to lie with a dependency of this project, rather than the project itself.

Maybe a more appropriate place to open an issue would be under friendsofsymfony/oauth2-php.

GuilhemN commented 6 years ago

I just tagged oauth2-php 1.2.3, these constants have been deprecated in it, does it fix your issue ?

dinamic commented 6 years ago

ping @cvele

tomasjav commented 6 years ago

friendsofsymfony/oauth2-php v1.2.3 doesn't resolve this issue. My debugging showed the problem is right here: https://github.com/FriendsOfSymfony/FOSOAuthServerBundle/blob/1.6/Security/EntryPoint/OAuthEntryPoint.php#L32

I guess it should be changed to Symfony\Component\HttpFoundation\Response::HTTP_UNAUTHORIZED

Edit: Actually, OAuth2::HTTP_UNAUTHORIZEDconstant is used in a few more places:

GuilhemN commented 6 years ago

@tomasjav could you submit a PR doing the change please? :)

cvele commented 6 years ago

@dinamic sorry went off the grid for a few days.

@tomasjav had exact issue as I did.

GuilhemN commented 6 years ago

Should be fixed by https://github.com/FriendsOfSymfony/FOSOAuthServerBundle/pull/539

dinamic commented 6 years ago

539 has been merged and should have fixed the problem. We don't have this tagged, however, so whomever needs to have the fixed version should use 1.6-dev, until there's 1.6.

francoispluchino commented 6 years ago

The PR #539 fix this issue and it was merged. So, this issue can be closed.