From d2a2ce9b7976250f14fe0fc7d482e02b763fe8db Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 Dec 2020 10:44:51 +0100 Subject: [PATCH 1/6] Better DX for missing problem files and general thrown exceptions --- src/Application.php | 2 +- src/Command/PrintCommand.php | 11 ++++++++++- src/Exception/ProblemFileDoesNotExistException.php | 13 +++++++++++++ src/ExerciseRenderer.php | 8 +++++++- 4 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 src/Exception/ProblemFileDoesNotExistException.php diff --git a/src/Application.php b/src/Application.php index 247cad99..bec77728 100644 --- a/src/Application.php +++ b/src/Application.php @@ -219,7 +219,7 @@ public function run(): int ) ); return 1; - } catch (RuntimeException $e) { + } catch (\Throwable $e) { $container ->get(OutputInterface::class) ->printError( diff --git a/src/Command/PrintCommand.php b/src/Command/PrintCommand.php index 89a7a429..51150b9d 100644 --- a/src/Command/PrintCommand.php +++ b/src/Command/PrintCommand.php @@ -4,6 +4,8 @@ namespace PhpSchool\PhpWorkshop\Command; +use PhpSchool\PhpWorkshop\Exception\InvalidArgumentException; +use PhpSchool\PhpWorkshop\Exception\ProblemFileDoesNotExistException; use PhpSchool\PhpWorkshop\ExerciseRepository; use PhpSchool\PhpWorkshop\MarkdownRenderer; use PhpSchool\PhpWorkshop\Output\OutputInterface; @@ -68,7 +70,14 @@ public function __invoke(): void $currentExercise = $this->userState->getCurrentExercise(); $exercise = $this->exerciseRepository->findByName($currentExercise); - $markDown = (string) file_get_contents($exercise->getProblem()); + $problemFile = $exercise->getProblem(); + + if (!is_readable($problemFile)) { + throw ProblemFileDoesNotExistException::fromFile($problemFile); + } + + $markDown = (string) file_get_contents($problemFile); + $doc = $this->markdownRenderer->render($markDown); $doc = str_replace('{appname}', $this->appName, $doc); $this->output->write($doc); diff --git a/src/Exception/ProblemFileDoesNotExistException.php b/src/Exception/ProblemFileDoesNotExistException.php new file mode 100644 index 00000000..0aed8aa6 --- /dev/null +++ b/src/Exception/ProblemFileDoesNotExistException.php @@ -0,0 +1,13 @@ +color->__invoke(" " . $exercise->getName())->yellow()->bold() . "\n"; $output .= $this->color->__invoke(sprintf(" Exercise %d of %d\n\n", $exerciseIndex, $numExercises))->yellow(); - $content = (string) file_get_contents($exercise->getProblem()); + $problemFile = $exercise->getProblem(); + if (!is_readable($problemFile)) { + throw ProblemFileDoesNotExistException::fromFile($problemFile); + } + + $content = (string) file_get_contents($problemFile); $doc = $this->markdownRenderer->render($content); $doc = str_replace('{appname}', $this->appName, $doc); $output .= $doc; From 9713cc86d95f7f5f3801655636ff773fa4a384f0 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 Dec 2020 19:26:38 +0100 Subject: [PATCH 2/6] Add path canonicalise function --- src/Utils/StringUtils.php | 33 +++++++++++++++++++++++++++++++++ src/functions.php | 14 ++++++++++++++ test/Util/StringUtilsTest.php | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 src/Utils/StringUtils.php create mode 100644 test/Util/StringUtilsTest.php diff --git a/src/Utils/StringUtils.php b/src/Utils/StringUtils.php new file mode 100644 index 00000000..5bcedda6 --- /dev/null +++ b/src/Utils/StringUtils.php @@ -0,0 +1,33 @@ + 0) { + array_pop($path); + } else { + throw new RuntimeException('Climbing above the root is not permitted.'); + } + } + + return $filename[0] === '/' + ? '/' . implode('/', $path) + : implode('/', $path); + } +} diff --git a/src/functions.php b/src/functions.php index a9eb52a7..58a54126 100644 --- a/src/functions.php +++ b/src/functions.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use PhpSchool\PhpWorkshop\Utils\StringUtils; + if (!function_exists('mb_str_pad')) { /** @@ -31,3 +33,15 @@ function camel_case_to_kebab_case(string $string): string }, $string); } } + +if (!function_exists('canonicalise_path')) { + + /** + * @param string $path + * @return string + */ + function canonicalise_path(string $path): string + { + return StringUtils::canonicalisePath($path); + } +} diff --git a/test/Util/StringUtilsTest.php b/test/Util/StringUtilsTest.php new file mode 100644 index 00000000..6f6f33eb --- /dev/null +++ b/test/Util/StringUtilsTest.php @@ -0,0 +1,32 @@ +assertEquals('/path/to/file', StringUtils::canonicalisePath('/path/to/file')); + $this->assertEquals('/path/to/file', StringUtils::canonicalisePath('/path/././to/file')); + $this->assertEquals('/path/to/file', StringUtils::canonicalisePath('/path///to/file')); + $this->assertEquals('/path/to/file', StringUtils::canonicalisePath('/path/to/file')); + $this->assertEquals('/', StringUtils::canonicalisePath('/path/to/../../')); + $this->assertEquals('/path', StringUtils::canonicalisePath('/path/to/some/../../')); + $this->assertEquals('/some', StringUtils::canonicalisePath('/path/../some/')); + $this->assertEquals('some', StringUtils::canonicalisePath('path/../some/')); + } + + public function testExceptionIsThrownIfTryingToTraverseUpPastRoom(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Climbing above the root is not permitted.'); + + StringUtils::canonicalisePath('/path/to/../../../'); + } +} From 2761eb777088d8f95da17cd70038b307e2ae209f Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 Dec 2020 19:27:15 +0100 Subject: [PATCH 3/6] canonicalise problem file path --- src/Exception/ProblemFileDoesNotExistException.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Exception/ProblemFileDoesNotExistException.php b/src/Exception/ProblemFileDoesNotExistException.php index 0aed8aa6..e26808f3 100644 --- a/src/Exception/ProblemFileDoesNotExistException.php +++ b/src/Exception/ProblemFileDoesNotExistException.php @@ -8,6 +8,9 @@ class ProblemFileDoesNotExistException extends \RuntimeException { public static function fromFile(string $file): self { - return new self("Exercise problem file: '$file' does not exist or is not readable"); + return new self(sprintf( + 'Exercise problem file: "%s" does not exist or is not readable', + canonicalise_path($file) + )); } } From 8fce5916fafe153ed4e5c074e2c6737f4bad27cb Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 Dec 2020 19:27:44 +0100 Subject: [PATCH 4/6] Add package level runtime exception --- src/Exception/RuntimeException.php | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 src/Exception/RuntimeException.php diff --git a/src/Exception/RuntimeException.php b/src/Exception/RuntimeException.php new file mode 100644 index 00000000..137e4156 --- /dev/null +++ b/src/Exception/RuntimeException.php @@ -0,0 +1,10 @@ + Date: Mon, 7 Dec 2020 19:28:42 +0100 Subject: [PATCH 5/6] Strip basepath off exception messages --- src/Application.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Application.php b/src/Application.php index bec77728..3aec2936 100644 --- a/src/Application.php +++ b/src/Application.php @@ -220,12 +220,19 @@ public function run(): int ); return 1; } catch (\Throwable $e) { + $message = $e->getMessage(); + $basePath = canonicalise_path($container->get('basePath')); + + if (strpos($message, $basePath) !== null) { + $message = str_replace($basePath, '', $message); + } + $container ->get(OutputInterface::class) ->printError( sprintf( '%s', - $e->getMessage() + $message ) ); return 1; From 316df75bf1810054a7a6e4ea7a29d81d7bfc6fb5 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Mon, 7 Dec 2020 19:45:49 +0100 Subject: [PATCH 6/6] CS --- src/Utils/StringUtils.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Utils/StringUtils.php b/src/Utils/StringUtils.php index 5bcedda6..f20d7921 100644 --- a/src/Utils/StringUtils.php +++ b/src/Utils/StringUtils.php @@ -11,7 +11,7 @@ class StringUtils public static function canonicalisePath(string $filename): string { $path = []; - foreach(explode('/', $filename) as $part) { + foreach (explode('/', $filename) as $part) { // ignore parts that have no value if (strlen($part) === 0 || $part === '.') { continue; @@ -19,7 +19,7 @@ public static function canonicalisePath(string $filename): string if ($part !== '..') { $path[] = $part; - } else if (count($path) > 0) { + } elseif (count($path) > 0) { array_pop($path); } else { throw new RuntimeException('Climbing above the root is not permitted.');