From ec17eb3fbc542ae6945491c445649daf72098387 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 May 2022 08:29:26 +0200 Subject: [PATCH 1/4] Ensured html entities are parsed when auto-resolving titles --- module/Core/src/Util/UrlValidator.php | 8 +++++++- module/Core/test/Util/UrlValidatorTest.php | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 6bd8d76f..2e2965b1 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Throwable; +use function html_entity_decode; use function preg_match; use function str_contains; use function str_starts_with; @@ -71,7 +72,7 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface $collectedBody .= $body->read(1024); } preg_match(TITLE_TAG_VALUE, $collectedBody, $matches); - return isset($matches[1]) ? trim($matches[1]) : null; + return isset($matches[1]) ? $this->normalizeTitle($matches[1]) : null; } /** @@ -101,4 +102,9 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface return null; } } + + private function normalizeTitle(string $title): string + { + return html_entity_decode(trim($title)); + } } diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php index 7e1f9a1b..8aba6598 100644 --- a/module/Core/test/Util/UrlValidatorTest.php +++ b/module/Core/test/Util/UrlValidatorTest.php @@ -128,7 +128,7 @@ class UrlValidatorTest extends TestCase $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); - self::assertEquals('Resolved title', $result); + self::assertEquals('Resolved "title"', $result); $request->shouldHaveBeenCalledOnce(); } @@ -162,7 +162,7 @@ class UrlValidatorTest extends TestCase private function respWithTitle(): Response { - $body = $this->createStreamWithContent(' Resolved title'); + $body = $this->createStreamWithContent(' Resolved "title" '); return new Response($body, 200, ['Content-Type' => 'TEXT/html; charset=utf-8']); } From f224bb98c4ef83c901f4345657c5af9c9072206f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 May 2022 08:30:46 +0200 Subject: [PATCH 2/4] Updated changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97153daf..56c66d15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [Unreleased] +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#1448](https://github.com/shlinkio/shlink/issues/1448) Fixed HTML entities not being properly parsed when auto-resolving page titles. + + ## [3.1.1] - 2022-05-09 ### Added * *Nothing* From de30c6ad790b5d722ba02f3c2a5de9a68a03989e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Jun 2022 11:20:08 +0200 Subject: [PATCH 3/4] Fixed error when filtering short URLs by ALL tags and search term --- CHANGELOG.md | 3 ++- .../src/Repository/ShortUrlRepository.php | 23 ++++++++++++------- .../Repository/ShortUrlRepositoryTest.php | 8 ++++++- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56c66d15..1cfe581b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [3.1.2] - 2022-06-04 ### Added * *Nothing* @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1448](https://github.com/shlinkio/shlink/issues/1448) Fixed HTML entities not being properly parsed when auto-resolving page titles. +* [#1458](https://github.com/shlinkio/shlink/issues/1458) Fixed 500 error when filtering short URLs by ALL tags and search term. ## [3.1.1] - 2022-05-09 diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 9852c530..d16d9993 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -102,15 +102,22 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $qb->leftJoin('s.tags', 't'); } - // Apply search conditions + // Apply general search conditions + $conditions = [ + $qb->expr()->like('s.longUrl', ':searchPattern'), + $qb->expr()->like('s.shortCode', ':searchPattern'), + $qb->expr()->like('s.title', ':searchPattern'), + $qb->expr()->like('d.authority', ':searchPattern'), + ]; + + // Apply tag conditions, only when not filtering by all provided tags + $tagsMode = $filtering->tagsMode() ?? ShortUrlsParams::TAGS_MODE_ANY; + if (empty($tags) || $tagsMode === ShortUrlsParams::TAGS_MODE_ANY) { + $conditions[] = $qb->expr()->like('t.name', ':searchPattern'); + } + $qb->leftJoin('s.domain', 'd') - ->andWhere($qb->expr()->orX( - $qb->expr()->like('s.longUrl', ':searchPattern'), - $qb->expr()->like('s.shortCode', ':searchPattern'), - $qb->expr()->like('s.title', ':searchPattern'), - $qb->expr()->like('t.name', ':searchPattern'), - $qb->expr()->like('d.authority', ':searchPattern'), - )) + ->andWhere($qb->expr()->orX(...$conditions)) ->setParameter('searchPattern', '%' . $searchTerm . '%'); } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 4ad89629..275df3f9 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -125,9 +125,15 @@ class ShortUrlRepositoryTest extends DatabaseTestCase new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), 'foo', ['bar']), ); self::assertCount(1, $result); - self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering('foo', ['bar']))); self::assertSame($foo, $result[0]); + // Assert searched text also applies to tags + $result = $this->repo->findList(new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), 'bar')); + self::assertCount(2, $result); + self::assertContains($foo, $result); + + self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering('foo', ['bar']))); + $result = $this->repo->findList(new ShortUrlsListFiltering(null, null, Ordering::emptyInstance())); self::assertCount(3, $result); From 1ab492ce5be199beaee018503e3226ea9ebfed20 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 4 Jun 2022 11:22:10 +0200 Subject: [PATCH 4/4] Added missing test case --- module/Core/test-db/Repository/ShortUrlRepositoryTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 275df3f9..c67ff34b 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -125,15 +125,15 @@ class ShortUrlRepositoryTest extends DatabaseTestCase new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), 'foo', ['bar']), ); self::assertCount(1, $result); + self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering('foo', ['bar']))); self::assertSame($foo, $result[0]); // Assert searched text also applies to tags $result = $this->repo->findList(new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), 'bar')); self::assertCount(2, $result); + self::assertEquals(2, $this->repo->countList(new ShortUrlsCountFiltering('bar'))); self::assertContains($foo, $result); - self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering('foo', ['bar']))); - $result = $this->repo->findList(new ShortUrlsListFiltering(null, null, Ordering::emptyInstance())); self::assertCount(3, $result);