Merge pull request #476 from acelaya-forks/feature/fix-log-fields-error

Feature/fix log fields error

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]
+## 1.18.1 - 2019-08-24
 
 #### Added
 
@@ -15,7 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * [#450](https://github.com/shlinkio/shlink/issues/450) Added PHP 7.4 to the build matrix, as an allowed-to-fail env.
 * [#441](https://github.com/shlinkio/shlink/issues/441) and [#443](https://github.com/shlinkio/shlink/issues/443) Split some logic into independent modules.
 * [#451](https://github.com/shlinkio/shlink/issues/451) Updated to infection 0.13.
-* [#467](https://github.com/shlinkio/shlink/issues/467) Moved docker image config to this repo.
+* [#467](https://github.com/shlinkio/shlink/issues/467) Moved docker image config to main Shlink repo.
 
 #### Deprecated
 
@@ -27,7 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 #### Fixed
 
-* *Nothing*
+* [#449](https://github.com/shlinkio/shlink/issues/449) Fixed error when trying to save too big referrers on PostgreSQL.
 
 
 ## 1.18.0 - 2019-08-08

composer.json

@@ -15,7 +15,7 @@
         "php": "^7.2",
         "ext-json": "*",
         "ext-pdo": "*",
-        "acelaya/ze-content-based-error-handler": "^2.2",
+        "acelaya/ze-content-based-error-handler": "^3.0",
         "akrabat/ip-address-middleware": "^1.0",
         "cakephp/chronos": "^1.2",
         "cocur/slugify": "^3.0",

data/migrations/Version20190824075137.php

@@ -0,0 +1,36 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Schema\SchemaException;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20190824075137 extends AbstractMigration
+{
+    /**
+     * @throws SchemaException
+     */
+    public function up(Schema $schema): void
+    {
+        $this->getRefererColumn($schema)->setLength(1024);
+    }
+
+    /**
+     * @throws SchemaException
+     */
+    public function down(Schema $schema): void
+    {
+        $this->getRefererColumn($schema)->setLength(256);
+    }
+
+    /**
+     * @throws SchemaException
+     */
+    private function getRefererColumn(Schema $schema): Column
+    {
+        return $schema->getTable('visits')->getColumn('referer');
+    }
+}

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php

@@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Core;
 
 use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
+use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine
 use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
 
 /** @var $metadata ClassMetadata */

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php

@@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Core;
 
 use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
+use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine
 
 /** @var $metadata ClassMetadata */
 $builder = new ClassMetadataBuilder($metadata);

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php

@@ -5,7 +5,9 @@ namespace Shlinkio\Shlink\Core;
 
 use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
+use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine
 use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
+use Shlinkio\Shlink\Core\Model\Visitor;
 
 /** @var $metadata ClassMetadata */
 $builder = new ClassMetadataBuilder($metadata);
@@ -22,7 +24,7 @@ $builder->createField('id', Type::BIGINT)
 
 $builder->createField('referer', Type::STRING)
         ->nullable()
-        ->length(256)
+        ->length(Visitor::REFERER_MAX_LENGTH)
         ->build();
 
 $builder->createField('date', ChronosDateTimeType::CHRONOS_DATETIME)
@@ -31,13 +33,13 @@ $builder->createField('date', ChronosDateTimeType::CHRONOS_DATETIME)
 
 $builder->createField('remoteAddr', Type::STRING)
         ->columnName('remote_addr')
-        ->length(256)
+        ->length(Visitor::REMOTE_ADDRESS_MAX_LENGTH)
         ->nullable()
         ->build();
 
 $builder->createField('userAgent', Type::STRING)
         ->columnName('user_agent')
-        ->length(512)
+        ->length(Visitor::USER_AGENT_MAX_LENGTH)
         ->nullable()
         ->build();
 

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php

@@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Core;
 
 use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
+use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine
 
 /** @var $metadata ClassMetadata */
 $builder = new ClassMetadataBuilder($metadata);

module/Core/src/Model/Visitor.php

@@ -6,8 +6,14 @@ namespace Shlinkio\Shlink\Core\Model;
 use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory;
 
+use function substr;
+
 final class Visitor
 {
+    public const USER_AGENT_MAX_LENGTH = 512;
+    public const REFERER_MAX_LENGTH = 1024;
+    public const REMOTE_ADDRESS_MAX_LENGTH = 256;
+
     /** @var string */
     private $userAgent;
     /** @var string */
@@ -17,9 +23,14 @@ final class Visitor
 
     public function __construct(string $userAgent, string $referer, ?string $remoteAddress)
     {
-        $this->userAgent = $userAgent;
-        $this->referer = $referer;
-        $this->remoteAddress = $remoteAddress;
+        $this->userAgent = $this->cropToLength($userAgent, self::USER_AGENT_MAX_LENGTH);
+        $this->referer = $this->cropToLength($referer, self::REFERER_MAX_LENGTH);
+        $this->remoteAddress = $this->cropToLength($remoteAddress, self::REMOTE_ADDRESS_MAX_LENGTH);
+    }
+
+    private function cropToLength(?string $value, int $length): ?string
+    {
+        return $value === null ? null : substr($value, 0, $length);
     }
 
     public static function fromRequest(ServerRequestInterface $request): self

module/Core/test/Model/VisitorTest.php

@@ -0,0 +1,62 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Model;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
+use Shlinkio\Shlink\Core\Model\Visitor;
+
+use function str_repeat;
+use function substr;
+
+class VisitorTest extends TestCase
+{
+    use StringUtilsTrait;
+
+    /**
+     * @test
+     * @dataProvider provideParams
+     */
+    public function providedFieldsValuesAreCropped(array $params, array $expected): void
+    {
+        $visitor = new Visitor(...$params);
+        ['userAgent' => $userAgent, 'referer' => $referer, 'remoteAddress' => $remoteAddress] = $expected;
+
+        $this->assertEquals($userAgent, $visitor->getUserAgent());
+        $this->assertEquals($referer, $visitor->getReferer());
+        $this->assertEquals($remoteAddress, $visitor->getRemoteAddress());
+    }
+
+    public function provideParams(): iterable
+    {
+        yield 'all values are bigger' => [
+            [str_repeat('a', 1000), str_repeat('b', 2000), str_repeat('c', 500)],
+            [
+                'userAgent' => str_repeat('a', Visitor::USER_AGENT_MAX_LENGTH),
+                'referer' => str_repeat('b', Visitor::REFERER_MAX_LENGTH),
+                'remoteAddress' => str_repeat('c', Visitor::REMOTE_ADDRESS_MAX_LENGTH),
+            ],
+        ];
+        yield 'some values are smaller' => [
+            [str_repeat('a', 10), str_repeat('b', 2000), null],
+            [
+                'userAgent' => str_repeat('a', 10),
+                'referer' => str_repeat('b', Visitor::REFERER_MAX_LENGTH),
+                'remoteAddress' => null,
+            ],
+        ];
+        yield 'random strings' => [
+            [
+                $userAgent = $this->generateRandomString(2000),
+                $referer = $this->generateRandomString(50),
+                null,
+            ],
+            [
+                'userAgent' => substr($userAgent, 0, Visitor::USER_AGENT_MAX_LENGTH),
+                'referer' => $referer,
+                'remoteAddress' => null,
+            ],
+        ];
+    }
+}