From 9df154105a6410d4efcc85af0a813a2f0fa30c79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 18 Jan 2024 13:56:07 +0100 Subject: [PATCH 1/3] fix(link): Properly sanizite and map database values to what we expect it to be MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/ColumnTypes/TextLinkBusiness.php | 39 +++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/lib/Service/ColumnTypes/TextLinkBusiness.php b/lib/Service/ColumnTypes/TextLinkBusiness.php index c36cfabfa..33de3dc3f 100644 --- a/lib/Service/ColumnTypes/TextLinkBusiness.php +++ b/lib/Service/ColumnTypes/TextLinkBusiness.php @@ -12,18 +12,30 @@ class TextLinkBusiness extends SuperBusiness implements IColumnTypeBusiness { * @return string */ public function parseValue($value, ?Column $column = null): string { + if ($value === null) { + return ''; + } + // if is import from export in format "[description] ([link])" preg_match('/(.*) \((http.*)\)/', $value, $matches); if (!empty($matches) && $matches[0] && $matches[1]) { return json_encode(json_encode([ 'title' => $matches[1], - 'resourceUrl' => $matches[2] + 'value' => $matches[2], + 'providerId' => 'url', ])); } - // if is json + // if is json (this is the default case, other formats are backward compatibility $data = json_decode($value, true); if($data !== null) { + if (isset($data['resourceUrl'])) { + return json_encode(json_encode([ + 'title' => $data['title'] ?? $data['resourceUrl'], + 'value' => $data['resourceUrl'], + 'providerId' => 'url', + ])); + } // at least title and resUrl have to be set if(isset($data['title']) && isset($data['value'])) { return json_encode($value); @@ -40,7 +52,8 @@ public function parseValue($value, ?Column $column = null): string { } return json_encode(json_encode([ 'title' => $matches[0], - 'resourceUrl' => $matches[0] + 'value' => $matches[0], + 'providerId' => 'url', ])); } @@ -50,14 +63,28 @@ public function parseValue($value, ?Column $column = null): string { * @return bool */ public function canBeParsed($value, ?Column $column = null): bool { + if (!$value) { + return true; + } preg_match('/(.*) \((http.*)\)/', $value, $matches); if (!empty($matches) && $matches[0] && $matches[1]) { return true; } - // if is json - $data = json_decode($value); - if($data != null) { + $data = json_decode($value, true); + if($data !== null) { + if (!isset($data['resourceUrl']) && !isset($data['value'])) { + $this->logger->error('Value ' . $value . ' cannot be parsed as the column ' . $column->getId(). ' as it contains incomplete data'); + return false; + } + + // Validate url providers + $allowedProviders = explode(',', $column->getTextAllowedPattern() ?? '') ?: []; + if (isset($data['providerId']) && !in_array($data['providerId'], $allowedProviders)) { + $this->logger->error('Value ' . $value . ' cannot be parsed as the column ' . $column->getId(). ' does not allow the provider: ' . $data['providerId']); + return false; + } + return true; } From 852df2156d730381b8b89f40fc4db3823a2110fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 18 Jan 2024 14:07:29 +0100 Subject: [PATCH 2/3] fix: Avoid failing on empty strings for URL values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../ncTable/partials/rowTypePartials/TextLinkForm.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/components/ncTable/partials/rowTypePartials/TextLinkForm.vue b/src/shared/components/ncTable/partials/rowTypePartials/TextLinkForm.vue index 131201a49..d7701535b 100644 --- a/src/shared/components/ncTable/partials/rowTypePartials/TextLinkForm.vue +++ b/src/shared/components/ncTable/partials/rowTypePartials/TextLinkForm.vue @@ -71,7 +71,7 @@ export default { localValue: { get() { // if we got an old value (string not object as json) - if (!this.hasJsonStructure(this.value) && this.value !== '' && this.value !== null && this.value !== '""') { + if (this.value && !this.hasJsonStructure(this.value)) { return { title: this.value, subline: t('tables', 'URL'), @@ -80,7 +80,7 @@ export default { } } - return JSON.parse(this.value) + return this.value ? JSON.parse(this.value) : null }, set(v) { let value = null From 612c4379741252732a53dcab2b666a0de06525fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 18 Jan 2024 14:30:36 +0100 Subject: [PATCH 3/3] tests: Cover link business logic for parsing with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../ColumnTypes/TextLinkBusinessTest.php | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 tests/unit/Service/ColumnTypes/TextLinkBusinessTest.php diff --git a/tests/unit/Service/ColumnTypes/TextLinkBusinessTest.php b/tests/unit/Service/ColumnTypes/TextLinkBusinessTest.php new file mode 100644 index 000000000..e7dcc34a0 --- /dev/null +++ b/tests/unit/Service/ColumnTypes/TextLinkBusinessTest.php @@ -0,0 +1,105 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Tables\Service\ColumnTypes; + +use OCA\Tables\Db\Column; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; + +class TextLinkBusinessTest extends TestCase { + + private TextLinkBusiness $textLink; + + public function setUp(): void { + $this->textLink = new TextLinkBusiness( + $this->createMock(LoggerInterface::class) + ); + } + + public function testCanBeParsed() { + self::assertTrue($this->textLink->canBeParsed(null)); + self::assertTrue($this->textLink->canBeParsed('')); + self::assertFalse($this->textLink->canBeParsed('null')); + self::assertFalse($this->textLink->canBeParsed('invalidurl')); + + self::assertTrue($this->textLink->canBeParsed('https://nextcloud.com')); + + self::assertTrue($this->textLink->canBeParsed('test (https://nextcloud.com)')); + self::assertTrue($this->textLink->canBeParsed('https://nextcloud.com (https://nextcloud.com)')); + + $column = new Column(); + self::assertFalse($this->textLink->canBeParsed(json_encode([ + 'unknown' => 'https://nextcloud.com' + ]), $column)); + self::assertTrue($this->textLink->canBeParsed(json_encode([ + 'resourceUrl' => 'https://nextcloud.com' + ]), $column)); + self::assertTrue($this->textLink->canBeParsed(json_encode([ + 'value' => 'https://nextcloud.com' + ]), $column)); + } + + public function testParseValue() { + self::assertEquals('', $this->textLink->parseValue(null)); + self::assertEquals('', $this->textLink->parseValue('')); + self::assertEquals('', $this->textLink->parseValue('null')); + + self::assertEquals(json_encode(json_encode([ + 'title' => 'https://nextcloud.com', + 'value' => 'https://nextcloud.com', + 'providerId' => 'url', + ])), $this->textLink->parseValue('https://nextcloud.com')); + + self::assertEquals(json_encode(json_encode([ + 'title' => 'https://nextcloud.com', + 'value' => 'https://nextcloud.com', + 'providerId' => 'url', + ])), $this->textLink->parseValue('https://nextcloud.com (https://nextcloud.com)')); + self::assertEquals(json_encode(json_encode([ + 'title' => 'test', + 'value' => 'https://nextcloud.com', + 'providerId' => 'url', + ])), $this->textLink->parseValue('test (https://nextcloud.com)')); + + $column = new Column(); + self::assertEquals('', $this->textLink->parseValue(json_encode([ + 'unknown' => 'https://nextcloud.com' + ]), $column)); + self::assertEquals(json_encode(json_encode([ + 'title' => 'https://nextcloud.com', + 'value' => 'https://nextcloud.com', + 'providerId' => 'url', + ])), $this->textLink->parseValue(json_encode([ + 'resourceUrl' => 'https://nextcloud.com' + ]), $column)); + self::assertEquals(json_encode(json_encode([ + 'title' => 'Test link', + 'value' => 'https://nextcloud.com', + 'providerId' => 'url', + ])), $this->textLink->parseValue(json_encode([ + 'title' => 'Test link', + 'value' => 'https://nextcloud.com', + 'providerId' => 'url', + ]), $column)); + } +}