-
Notifications
You must be signed in to change notification settings - Fork 4
Check code for errors and warnings. #114
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
4ad9a96
284372c
1b4b1f9
e0928fe
becc71a
f892657
3a03c3d
c2397ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,14 +12,50 @@ | |
*/ | ||
class CodeExecutionException extends RuntimeException | ||
{ | ||
|
||
/** | ||
* @var string | ||
*/ | ||
private $actual; | ||
/** | ||
* @var string | ||
*/ | ||
private $errors; | ||
|
||
/** | ||
* CodeExecutionException constructor. | ||
* @param string $reason | ||
* @param string $actual | ||
* @param string $errors | ||
*/ | ||
public function __construct($reason, $actual = null, $errors = null) | ||
{ | ||
$this->message = $reason; | ||
$this->actual = $actual; | ||
$this->errors = $errors; | ||
} | ||
|
||
/** | ||
* @param Process $process | ||
* @return static | ||
*/ | ||
public static function fromProcess(Process $process) | ||
{ | ||
$message = 'PHP Code failed to execute. Error: "%s"'; | ||
$processOutput = $process->getErrorOutput() ? $process->getErrorOutput() : $process->getOutput(); | ||
return new static(sprintf($message, $processOutput)); | ||
$message = "PHP Code failed to execute. Error: \n%s"; | ||
$processOutput = $process->getOutput(); | ||
$processErrorOutput = $process->getErrorOutput(); | ||
return new static( | ||
sprintf($message, $processErrorOutput ?: $processOutput), $processOutput, $processErrorOutput | ||
); | ||
} | ||
|
||
public function getActual() | ||
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. can we add return types on these two methods |
||
{ | ||
return $this->actual; | ||
} | ||
|
||
public function getErrors() | ||
{ | ||
return $this->errors; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,7 +98,7 @@ private function checkRequest(RequestInterface $request, $fileName) | |
|
||
try { | ||
$event = $this->eventDispatcher->dispatch(new CgiExecuteEvent('cgi.verify.user-execute.pre', $request)); | ||
$userResponse = $this->executePhpFile($fileName, $event->getRequest(), 'user'); | ||
list($userResponse, $userWarnings) = $this->executePhpFile($fileName, $event->getRequest(), 'user'); | ||
} catch (CodeExecutionException $e) { | ||
$this->eventDispatcher->dispatch(new Event('cgi.verify.user-execute.fail', ['exception' => $e])); | ||
return Failure::fromNameAndCodeExecutionFailure($this->getName(), $e); | ||
|
@@ -109,8 +109,15 @@ private function checkRequest(RequestInterface $request, $fileName) | |
$solutionHeaders = $this->getHeaders($solutionResponse); | ||
$userHeaders = $this->getHeaders($userResponse); | ||
|
||
if ($solutionBody !== $userBody || $solutionHeaders !== $userHeaders) { | ||
return new CgiOutRequestFailure($request, $solutionBody, $userBody, $solutionHeaders, $userHeaders); | ||
if ($solutionBody !== $userBody || $solutionHeaders !== $userHeaders || $userWarnings) { | ||
return new CgiOutRequestFailure( | ||
$request, | ||
$solutionBody, | ||
$userBody, | ||
$solutionHeaders, | ||
$userHeaders, | ||
$userWarnings | ||
); | ||
} | ||
|
||
return new Success($this->getName()); | ||
|
@@ -153,7 +160,11 @@ private function executePhpFile($fileName, RequestInterface $request, $type) | |
$output = "HTTP/1.0 200 OK\r\n" . $output; | ||
} | ||
|
||
return ResponseSerializer::fromString($output); | ||
return ( | ||
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'm not keen on the mix of return types here, |
||
$type == 'user' ? | ||
[ResponseSerializer::fromString($output), $process->getErrorOutput()] | ||
: ResponseSerializer::fromString($output) | ||
); | ||
} | ||
|
||
/** | ||
|
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.
Can we rename
$actual
to$output
?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.
Params/property renamed. Methods unchanged, to match other similar methods.