-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] [Request] Throw dedicated exceptions. #20389
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
Changes from all commits
9807f06
45e439f
e750da0
9ca7de7
b99a3f9
183d59d
c25ff57
c721ecc
8c9dbb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\HttpFoundation\Exception; | ||
|
||
/** | ||
* The HTTP request contains incorrect host data. | ||
* | ||
* @internal | ||
* | ||
* @author SpacePossum | ||
*/ | ||
abstract class AbstractHostException extends \UnexpectedValueException implements ExceptionInterface | ||
{ | ||
private $host; | ||
|
||
/** | ||
* @param string $host | ||
* @param string $message | ||
*/ | ||
public function __construct($host, $message) | ||
{ | ||
parent::__construct($message); | ||
$this->host = $host; | ||
} | ||
|
||
public function getHost() | ||
{ | ||
return $this->host; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\HttpFoundation\Exception; | ||
|
||
/** | ||
* Base ExceptionInterface for the Request component. | ||
* | ||
* @author SpacePossum | ||
*/ | ||
interface ExceptionInterface | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\HttpFoundation\Exception; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In other components, exceptions implement an interface specific to the component. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which interface would be appropriate in this case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was only one exception but if the component contains more exceptions, they should implement an ExceptionInterface. |
||
|
||
/** | ||
* The HTTP request contains invalid host data. | ||
* | ||
* @author SpacePossum | ||
*/ | ||
final class InvalidHostException extends AbstractHostException | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this one should be internal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 agreed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On second thought, I don't think so, people want to deal with this exception by catching it. Marking it as internal will cause IDE's to complain about using an internal class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed sorry I didn't commented on the right class... I was thinking about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 I've updated that class. |
||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\HttpFoundation\Exception; | ||
|
||
/** | ||
* The HTTP request contains host data which is not trusted. | ||
* | ||
* @author SpacePossum | ||
*/ | ||
final class UntrustedHostException extends AbstractHostException | ||
{ | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not making
$message
optional (if null, default toInvalid host "'.$host.'"
), so you can simply thrownew InvalidHostException($host)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making it
null
able will break with the design of the base exception class itself, which is IIRC something people don't want in the SF project.Besides there is no way this class can generate a useful/correct message based on the value of
$host
alone, the logic that figured out the$host
value is incorrect and throws the exception knows what is wrong and can therefor set a meaningful message only.