Skip to content

Commit

Permalink
Debugger: Validate breakpoint file and line (#834)
Browse files Browse the repository at this point in the history 
* Validate the breakpoint file/line early

* case insensitive file extension validation

* Fix license year on new file

* If the line starts with *, check to see if it's in a multiline comment.

This won't catch comments that are multiline that don't begin with *.

* Cleanup case statements

* Add comments about this validation not being exhaustive
  • Loading branch information
@chingor13 @dwsupplee
chingor13 authored and dwsupplee committed Jan 17, 2018
1 parent 0dc13d2 commit 81b20d0
Show file tree
Hide file tree
Showing 2 changed files with 313 additions and 30 deletions.
173 changes: 143 additions & 30 deletions src/Debugger/Breakpoint.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,16 @@ public function stackFrames()
return $this->stackFrames;
}

/**
* Return the status for this breakpoint
*
* @return StatusMessage|null
*/
public function status()
{
return $this->status;
}

/**
* Returns the VariableTable
*
Expand Down Expand Up @@ -530,7 +540,10 @@ public function addEvaluatedExpressions(array $expressions)

/**
* Validate that this breakpoint can be executed. If not valid, the status
* field will be populated with the corresponding error message.
* field will be populated with the corresponding error message. This
* validation does not guarantee that the breakpoint will be reachable.
* The primary use case is to reject clearly invalid breakpoints and return
* a message to the developer via the Debugger console.
*
* Example:
* ```
Expand All @@ -540,6 +553,29 @@ public function addEvaluatedExpressions(array $expressions)
* @return bool
*/
public function validate()
{
return $this->ensureExtensionLoaded() &&
$this->validateSourceLocation() &&
$this->validateCondition() &&
$this->validateExpressions();
}

private function setError($type, $message, array $parameters = [])
{
$this->status = new StatusMessage(
true,
$type,
new FormatMessage($message, $parameters)
);
}

private function addVariable($name, $value)
{
$this->variableTable = $this->variableTable ?: new VariableTable();
return $this->variableTable->register($name, $value);
}

private function ensureExtensionLoaded()
{
if (!extension_loaded('stackdriver_debugger')) {
$this->setError(
Expand All @@ -548,30 +584,122 @@ public function validate()
);
return false;
}
return true;
}

if ($this->condition()) {
// validate that the condition is ok for debugging
try {
if (!stackdriver_debugger_valid_statement($this->condition())) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_CONDITION,
'Invalid breakpoint condition - Invalid operations: $0.',
[$this->condition]
);
return false;
}
} catch (\ParseError $e) {
/**
* Validate that the source location is a valid. This means that:
*
* - The file exists
* - The file is readable
* - The file looks like a php file (ends in .php)
* - The line has code on it (non-empty and does not look like a comment)
*
* This validation is not perfect as we are not compiling the file to
* actually do the validation. We may miss cases.
*/
private function validateSourceLocation()
{
if (!$this->location) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
'Invalid breakpoint location'
);
return false;
}

$path = $this->location->path();
$info = new \SplFileInfo($path);

// Ensure the file exists and is readable
if (!$info->isReadable()) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
'Invalid breakpoint location - File not found or unreadable: $0.',
[$path]
);
return false;
}

// Ensure the file is a php file
if (strtolower($info->getExtension()) !== 'php') {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
'Invalid breakpoint location - Invalid file type: $0.',
[$info->getExtension()]
);
return false;
}

$file = $info->openFile('r');
$file->seek($this->location->line() - 1);
$line = ltrim($file->current() ?: '');

// Ensure the line exists and is not empty
if ($line === '') {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
'Invalid breakpoint location - Invalid file line: $0.',
[$this->location->line()]
);
return false;
}

// Check that the line is not a comment
if ($line[0] == '/' || ($line[0] == '*' && $this->inMultilineComment($file, $this->location->line() - 1))) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
'Invalid breakpoint location - Invalid file line: $0.',
[$this->location->line()]
);
return false;
}

return true;
}

private function inMultilineComment($file, $lineNumber)
{
if ($lineNumber === 0) {
return false;
}
$file->seek($lineNumber - 1);
$line = ltrim($file->current() ?: '');

return substr($line, 0, 2) == '/*' ||
($line[0] == '*' && $this->inMultilineComment($file, $lineNumber - 1));
}

private function validateCondition()
{
if (!$this->condition) {
return true;
}

try {
if (!@stackdriver_debugger_valid_statement($this->condition())) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_CONDITION,
'Invalid breakpoint condition - Parse error: $0.',
'Invalid breakpoint condition - Invalid operations: $0.',
[$this->condition]
);
return false;
}
} catch (\ParseError $e) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_CONDITION,
'Invalid breakpoint condition - Parse error: $0.',
[$this->condition]
);
return false;
}
return true;
}

private function validateExpressions()
{
foreach ($this->expressions as $expression) {
if (!stackdriver_debugger_valid_statement($expression)) {
if (!@stackdriver_debugger_valid_statement($expression)) {
$this->setError(
StatusMessage::REFERENCE_BREAKPOINT_EXPRESSION,
'Invalid breakpoint expression: $0',
Expand All @@ -582,19 +710,4 @@ public function validate()
}
return true;
}

private function setError($type, $message, array $parameters = [])
{
$this->status = new StatusMessage(
true,
$type,
new FormatMessage($message, $parameters)
);
}

private function addVariable($name, $value)
{
$this->variableTable = $this->variableTable ?: new VariableTable();
return $this->variableTable->register($name, $value);
}
}
170 changes: 170 additions & 0 deletions tests/unit/Debugger/BreakpointValidationTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
<?php
/**
* Copyright 2018 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

namespace Google\Cloud\Tests\Unit\Debugger;

use Google\Cloud\Debugger\StatusMessage;
use Google\Cloud\Debugger\Breakpoint;
use Google\Cloud\Debugger\SourceLocation;
use PHPUnit\Framework\TestCase;

/**
* @group debugger
*/
class BreakpointValidationTest extends TestCase
{
public function setUp()
{
if (!extension_loaded('stackdriver_debugger')) {
$this->markTestSkipped('Breakpoint validation requires stackdriver_debugger extension');
}
}

/**
* @dataProvider conditionsToTest
*/
public function testValidatesCondition($condition, $expectedValid)
{
$breakpoint = new Breakpoint([
'location' => [
'path' => __FILE__,
'line' => __LINE__
],
'condition' => $condition
]);
$this->assertEquals($expectedValid, $breakpoint->validate());
if (!$expectedValid) {
$status = $breakpoint->status()->jsonSerialize();
$this->assertTrue($status['isError']);
$this->assertEquals(StatusMessage::REFERENCE_BREAKPOINT_CONDITION, $status['refersTo']);
}
}

/**
* @dataProvider conditionsToTest
*/
public function testValidatesExpression($expression, $expectedValid)
{
$breakpoint = new Breakpoint([
'location' => [
'path' => __FILE__,
'line' => __LINE__
],
'expressions' => [$expression]
]);
$this->assertEquals($expectedValid, $breakpoint->validate());
if (!$expectedValid) {
$status = $breakpoint->status()->jsonSerialize();
$this->assertTrue($status['isError']);
$this->assertEquals(StatusMessage::REFERENCE_BREAKPOINT_EXPRESSION, $status['refersTo']);
}
}

public function conditionsToTest()
{
return [
['3 + 3', true],
['3 + 3 == 6', true],
['foo(3)', false],
['abs(-3)', false],
['$foo->bar', true],
['$foo->bar()', false]
];
}

public function testValidatesMissingFile()
{
$breakpoint = new Breakpoint([
'location' => [
'path' => __DIR__ . '/missing_file.php',
'line' => __LINE__
]
]);
$this->assertFalse($breakpoint->validate());
$status = $breakpoint->status()->jsonSerialize();
$this->assertTrue($status['isError']);
$this->assertEquals(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
$status['refersTo']
);
}

public function testValidatesFileType()
{
$breakpoint = new Breakpoint([
'location' => [
'path' => __DIR__ . '/../../../composer.json',
'line' => __LINE__
]
]);
$this->assertFalse($breakpoint->validate());
$status = $breakpoint->status()->jsonSerialize();
$this->assertTrue($status['isError']);
$this->assertEquals(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
$status['refersTo']
);
}

public function testValidatesEmptyLine()
{
$breakpoint = new Breakpoint([
'location' => [
'path' => __FILE__,
'line' => __LINE__ - 6
]
]);
$this->assertFalse($breakpoint->validate());
$status = $breakpoint->status()->jsonSerialize();
$this->assertTrue($status['isError']);
$this->assertEquals(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
$status['refersTo']
);
}

public function testValidatesCommentLine()
{
$breakpoint = new Breakpoint([
'location' => [
'path' => __FILE__,
'line' => 2
]
]);
$this->assertFalse($breakpoint->validate());
$status = $breakpoint->status()->jsonSerialize();
$this->assertTrue($status['isError']);
$this->assertEquals(
StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION,
$status['refersTo']
);
}

public function testValidatesNonCommentMultiplication()
{
$value = 6
* 3;

$breakpoint = new Breakpoint([
'location' => [
'path' => __FILE__,
'line' => __LINE__ - 5
]
]);
$this->assertTrue($breakpoint->validate());
}
}

0 comments on commit 81b20d0

Please sign in to comment.