parsivori / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

Custom HTTP codes in SimpleSAML_Error_Error #566

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Purpose of code changes on this branch:

Add support for custom HTTP codes in SimpleSAML_Error_Error.

When reviewing my code changes, please focus on:

-

After the review, I'll merge this branch into:
/trunk

Original issue reported on code.google.com by comel...@gmail.com on 29 Aug 2013 at 1:59

Attachments:

GoogleCodeExporter commented 8 years ago
- I think $httpCode should be an integer, and then provide a dictionary with 
the mappings from the most common status codes to the status text. This ensures 
compatibility with the http_response_code()-function which is available from 
PHP 5.4.
- You provide a parameter to set the status code in the constructor, but 
instead of using it, you override a member-variable. I think you should 
probably either just use the constructor, or just add «$this->httpCode = 
400;» after calling the parent constructor.
- If you want to add a line feed to the trailing ?> in the file, you should 
rather remove it.

Best regards,
Olav Morken
UNINETT / Feide

Original comment by olavmrk@gmail.com on 2 Sep 2013 at 11:18

GoogleCodeExporter commented 8 years ago
OK, here is the modified patch.

Original comment by comel...@gmail.com on 2 Sep 2013 at 12:00

Attachments:

GoogleCodeExporter commented 8 years ago
Parameter $httpCode is added to constructor for override purposes, e.g. throw 
new SimpleSAML_Error_Error('example:FORBIDDEN', $e, 403).

Original comment by comel...@gmail.com on 3 Sep 2013 at 12:54

GoogleCodeExporter commented 8 years ago
OK, but in that case, you may as well use it when the code looks like:

    parent::__construct(array('BADREQUEST', '%REASON%' => $this->reason));
    $this->httpCode = 400;

Other than that, this patch looks good. Please commit it.

Original comment by olavmrk@gmail.com on 3 Sep 2013 at 1:18

GoogleCodeExporter commented 8 years ago
OK, committed as r3267.

Original comment by comel...@gmail.com on 3 Sep 2013 at 1:37