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

Fixing HTTP status code definition #105

Merged
merged 7 commits into from
Jan 30, 2018
Merged

Fixing HTTP status code definition #105

merged 7 commits into from
Jan 30, 2018

Conversation

SebScoFr
Copy link

@SebScoFr SebScoFr commented Jan 10, 2018

In oauth2-php/lib/OAuth2.php, status code should be changed from

const HTTP_FOUND = '302 Found';
const HTTP_BAD_REQUEST = '400 Bad Request';
const HTTP_UNAUTHORIZED = '401 Unauthorized';
const HTTP_FORBIDDEN = '403 Forbidden';
const HTTP_UNAVAILABLE = '503 Service Unavailable';

to

const HTTP_FOUND = Response::HTTP_FOUND;
const HTTP_BAD_REQUEST = Response::HTTP_BAD_REQUEST;
const HTTP_UNAUTHORIZED = Response::HTTP_UNAUTHORIZED;
const HTTP_FORBIDDEN = Response::HTTP_FORBIDDEN;
const HTTP_UNAVAILABLE = Response::HTTP_SERVICE_UNAVAILABLE;

The reason for that is that the lib now fails with SF4 with
Notice: A non well formed numeric value encountered

This happens here, oauth2-php/lib/OAuth2ServerException.php

public function getHttpResponse()
{
    return new Response(
        $this->getResponseBody(),
        $this->getHttpCode(),
        $this->getResponseHeaders()
    );
}

And Symfony Response class is expecting this

public function __construct($content = '', int $status = 200, array $headers = array())
{
    $this->headers = new ResponseHeaderBag($headers);
    $this->setContent($content);
    $this->setStatusCode($status);
    $this->setProtocolVersion('1.0');
}

$status must be a well-formed int.

EDIT: I followed @Spomky suggestion and updated the PR.

@SebScoFr SebScoFr changed the title Fixing status code definition Fixing HTTP status code definition Jan 10, 2018
@Spomky
Copy link

Spomky commented Jan 10, 2018

Best would be to mark those constants as deprecated and directly use Response object ones

@SebScoFr
Copy link
Author

Thanks @Spomky, done.

@andythorne
Copy link

Any news on getting this merged? It seems to fix it...

@GuilhemN
Copy link
Member

The composer.json should then be update to remove symfony/http-foundation old versions support. (imo we can remove 2.x support)

@recons
Copy link

recons commented Jan 30, 2018

@GuilhemN will it be merged in the near future?

@GuilhemN
Copy link
Member

GuilhemN commented Jan 30, 2018

@recons same answer. This is not ready to be merged. Please submit a corrected PR if you want it to go faster.

@SebScoFr
Copy link
Author

@GuilhemN Thanks for the comment. I've updated the PR.

@recons
Copy link

recons commented Jan 30, 2018

@SebScoFr you need to update required composer php version and .travis.yml also

@SebScoFr
Copy link
Author

Should be all good now

@GuilhemN GuilhemN merged commit a41fef6 into FriendsOfSymfony:master Jan 30, 2018
@GuilhemN
Copy link
Member

Great, thank you @SebScoFr :)

@dinamic dinamic mentioned this pull request Feb 9, 2018
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.

5 participants