From 25315db3476a6cece64db617fcd9fb10ba7a83a4 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 23 Aug 2024 15:24:20 +0200 Subject: [PATCH] exclude more queries from logging, also lowercase first (#2542) * exclude more queries from logging, also lowercase first * misc * skip legacys --- .../Stats/FindRuleStatsController.php | 30 ------------- src/Livewire/RectorFilterComponent.php | 43 ++++++++++++------- ...hLogger.php => RectorFuleSearchLogger.php} | 34 +++++++++++---- 3 files changed, 54 insertions(+), 53 deletions(-) rename src/Logging/{SearchLogger.php => RectorFuleSearchLogger.php} (54%) diff --git a/src/Controller/Stats/FindRuleStatsController.php b/src/Controller/Stats/FindRuleStatsController.php index 14ad0b6c..5413bd88 100644 --- a/src/Controller/Stats/FindRuleStatsController.php +++ b/src/Controller/Stats/FindRuleStatsController.php @@ -27,14 +27,9 @@ public function __invoke(): View $queriesToCount = $this->filterQueriesToCount($searchRecords); $rulesToCount = $this->filterRulesToCount($searchRecords); - // remove values with space as legacy $sets = $this->getArrayFlattenKey($searchRecords, 'set'); - $sets = array_filter($sets, fn (string $set): bool => ! str_contains($set, ' ')); $setsToCount = Arrays::groupToCount($sets, 4); - - // remove ones with "\" as legacy - $nodeTypes = array_filter($nodeTypes, fn (string $nodeType): bool => ! str_contains($nodeType, '\\')); $nodeTypesToCount = Arrays::groupToCount($nodeTypes, 5); // day by day stats @@ -114,31 +109,6 @@ private function filterQueriesToCount(array $searchRecords): array // remove super short queries return array_filter($queriesToCount, function (string $query): bool { - // skip SQL injections - if (str_contains($query, ' and ')) { - return false; - } - - if (str_contains($query, ' when ')) { - return false; - } - - if (str_contains($query, ' or ')) { - return false; - } - - if (str_contains($query, 'select ')) { - return false; - } - - if (str_contains($query, 'waitfor delay')) { - return false; - } - - if (str_contains($query, ' order by ')) { - return false; - } - // skip rector rules if (str_ends_with($query, 'rector')) { return false; diff --git a/src/Livewire/RectorFilterComponent.php b/src/Livewire/RectorFilterComponent.php index cf0834ea..170e28dc 100644 --- a/src/Livewire/RectorFilterComponent.php +++ b/src/Livewire/RectorFilterComponent.php @@ -7,8 +7,9 @@ use App\Enum\ComponentEvent; use App\Enum\FindRuleQuery; use App\FileSystem\RectorFinder; -use App\Logging\SearchLogger; +use App\Logging\RectorFuleSearchLogger; use App\RuleFilter\RuleFilter; +use App\RuleFilter\ValueObject\RuleMetadata; use App\Sets\RectorSetsTreeProvider; use Illuminate\View\View; use Livewire\Attributes\Url; @@ -27,27 +28,15 @@ final class RectorFilterComponent extends Component public function render(): View { - /** @var RectorFinder $rectorFinder */ - $rectorFinder = app(RectorFinder::class); - $ruleMetadatas = $rectorFinder->findCore(); - // wip // $communityRectors = $rectorFinder->findCommunity(); // to trigger event in component javascript $this->dispatch(ComponentEvent::RULES_FILTERED); - /** @var RuleFilter $ruleFilter */ - $ruleFilter = app(RuleFilter::class); - $filteredRules = $ruleFilter->filter($ruleMetadatas, $this->query, $this->nodeType, $this->rectorSet); - - /** @var SearchLogger $searchLogger */ - $searchLogger = app(SearchLogger::class); + $filteredRules = $this->getFilteredRuleMetadatas(); - // log only meaningful query, not a start of typing, to keep data clean - if ($this->query === null || (strlen($this->query) > 3)) { - $searchLogger->log($this->query, $this->nodeType, $this->rectorSet); - } + $this->logRuleSearch(); /** @var RectorSetsTreeProvider $rectorSetsTreeProvider */ $rectorSetsTreeProvider = app(RectorSetsTreeProvider::class); @@ -72,4 +61,28 @@ private function isFilterActive(): bool return $this->rectorSet !== null && $this->rectorSet !== ''; } + + /** + * @return RuleMetadata[] + */ + private function getFilteredRuleMetadatas(): array + { + /** @var RectorFinder $rectorFinder */ + $rectorFinder = app(RectorFinder::class); + $ruleMetadatas = $rectorFinder->findCore(); + + /** @var RuleFilter $ruleFilter */ + $ruleFilter = app(RuleFilter::class); + + return $ruleFilter->filter($ruleMetadatas, $this->query, $this->nodeType, $this->rectorSet); + } + + private function logRuleSearch(): void + { + /** @var RectorFuleSearchLogger $rectorFuleSearchLogger */ + $rectorFuleSearchLogger = app(RectorFuleSearchLogger::class); + + // log only meaningful query, not a start of typing, to keep data clean + $rectorFuleSearchLogger->log($this->query, $this->nodeType, $this->rectorSet); + } } diff --git a/src/Logging/SearchLogger.php b/src/Logging/RectorFuleSearchLogger.php similarity index 54% rename from src/Logging/SearchLogger.php rename to src/Logging/RectorFuleSearchLogger.php index 43c0d14a..6d1a6189 100644 --- a/src/Logging/SearchLogger.php +++ b/src/Logging/RectorFuleSearchLogger.php @@ -6,13 +6,13 @@ use Nette\Utils\Json; -final class SearchLogger +final class RectorFuleSearchLogger { /** * Skip typical SQL injection attacks, as no value * @var string[] */ - private const EXCLUDED_QUERIES = ['ORDER BY', 'SELECT', ' AND ', ' OR ']; + private const EXCLUDED_QUERIES = ['order by', 'select', ' and ', ' or ', ' limit ', 'when', 'waitfor delay']; /** * Simple search logger, to see what is needed by the community @@ -24,13 +24,14 @@ public function log(?string $query, ?string $nodeType, ?string $set): void return; } + // avoid logging short search + if ($query !== null && strlen($query) <= 5) { + return; + } + // skip typical SQL injections attacks - if ($query) { - foreach (self::EXCLUDED_QUERIES as $excludedQuery) { - if (str_contains($query, $excludedQuery)) { - return; - } - } + if ($this->isSQLInjection($query)) { + return; } $searchJson = Json::encode([ @@ -42,4 +43,21 @@ public function log(?string $query, ?string $nodeType, ?string $set): void file_put_contents(__DIR__ . '/../../storage/logs/search.json', $searchJson, FILE_APPEND); } + + private function isSQLInjection(?string $query): bool + { + if (! is_string($query)) { + return false; + } + + $lowerQuery = strtolower($query); + + foreach (self::EXCLUDED_QUERIES as $excludedQuery) { + if (str_contains($lowerQuery, $excludedQuery)) { + return true; + } + } + + return false; + } }