validate color (#2072)

src/Entity/ColorTrait.php

@@ -11,9 +11,9 @@ namespace App\Entity;
 
 use App\Constants;
 use App\Export\Annotation as Exporter;
+use App\Validator\Constraints as Constraints;
 use Doctrine\ORM\Mapping as ORM;
 use JMS\Serializer\Annotation as Serializer;
-use Symfony\Component\Validator\Constraints as Assert;
 
 trait ColorTrait
 {
@@ -28,7 +28,7 @@ trait ColorTrait
      * @Exporter\Expose(label="label.color")
      *
      * @ORM\Column(name="color", type="string", length=7, nullable=true)
-     * @Assert\Length(min=4, max=7, allowEmptyString=true)
+     * @Constraints\HexColor()
      */
     private $color = null;
 

src/Form/EntityFormTrait.php

@@ -21,9 +21,10 @@ trait EntityFormTrait
 {
     public function addCommonFields(FormBuilderInterface $builder, array $options): void
     {
-        $currency = $options['currency'];
         $builder
-            ->add('color', ColorPickerType::class)
+            ->add('color', ColorPickerType::class, [
+                'required' => false,
+            ])
         ;
 
         if ($options['include_budget']) {
@@ -32,7 +33,7 @@ trait EntityFormTrait
                     'empty_data' => '0.00',
                     'label' => 'label.budget',
                     'required' => false,
-                    'currency' => $currency,
+                    'currency' => $options['currency'],
                 ])
                 ->add('timeBudget', DurationType::class, [
                     'empty_data' => 0,

src/Form/Type/ColorPickerType.php

@@ -36,9 +36,10 @@ class ColorPickerType extends AbstractType implements DataTransformerInterface
         $resolver->setDefaults([
             'documentation' => [
                 'type' => 'string',
-                'description' => sprintf('The color code as hex (default: %s)', self::DEFAULT_COLOR),
+                'description' => sprintf('The hexadecimal color code (default: %s)', self::DEFAULT_COLOR),
             ],
             'label' => 'label.color',
+            'empty_data' => null,
         ]);
     }
 

src/Utils/Color.php

@@ -78,14 +78,18 @@ final class Color
 
     public function getFontContrastColor(string $color): string
     {
-        if ($color[0] !== '#') {
-            throw new \InvalidArgumentException('Invalid color code given, only #hexadecimal is supported.');
+        if (empty($color) || $color[0] !== '#') {
+            // do not throw exception on invalid colors, as they were not validated in the past
+            $color = Constants::DEFAULT_COLOR;
         }
 
         $color = substr($color, 1);
+        $length = \strlen($color);
 
-        if (\strlen($color) === 3) {
+        if ($length === 3) {
             $color = $color[0] . $color[0] . $color[1] . $color[1] . $color[2] . $color[2];
+        } elseif ($length !== 6) {
+            $color = substr(Constants::DEFAULT_COLOR, 1);
         }
 
         $r = hexdec(substr($color, 0, 2));

src/Validator/Constraints/HexColor.php

@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Validator\Constraints;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * @Annotation
+ * @Target({"PROPERTY", "METHOD", "ANNOTATION"})
+ */
+class HexColor extends Constraint
+{
+    public const HEX_COLOR_ERROR = 'xd5hffg-dsfef3-426a-83d7-2g8jkfr56d84';
+
+    protected static $errorNames = [
+        self::HEX_COLOR_ERROR => 'HEX_COLOR_ERROR',
+    ];
+
+    public $message = 'The given value is not a valid hexadecimal color.';
+}

src/Validator/Constraints/HexColorValidator.php

@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Validator\Constraints;
+
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+
+class HexColorValidator extends ConstraintValidator
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function validate($value, Constraint $constraint)
+    {
+        if (!$constraint instanceof HexColor) {
+            throw new UnexpectedTypeException($constraint, __NAMESPACE__ . '\HexColor');
+        }
+
+        $color = $value;
+
+        if ($color === null || (\is_string($color) && empty($color))) {
+            return;
+        }
+
+        if (!\is_string($color) || 1 !== preg_match('/^#([0-9a-fA-F]{6}|[0-9a-fA-F]{3})$/i', $color)) {
+            $this->context->buildViolation($constraint->message)
+                ->setParameter('{{ value }}', $this->formatValue($color))
+                ->setCode(HexColor::HEX_COLOR_ERROR)
+                ->addViolation();
+        }
+    }
+}

tests/Utils/ColorTest.php

@@ -100,12 +100,17 @@ class ColorTest extends TestCase
         $this->assertEquals('#000000', $sut->getFontContrastColor('#ffffff'));
     }
 
-    public function testGetFontContrastColorThrowsExceptionOnNonHexadecimalColor()
+    public function testGetFontContrastColorReturnsContrastForDefaultColorOnInvalidColor()
     {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Invalid color code given, only #hexadecimal is supported.');
-
         $sut = new Color();
-        $sut->getFontContrastColor('000000');
+        $this->assertEquals('#000000', $sut->getFontContrastColor(''));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('000000'));
+        $this->assertEquals('#000000', $sut->getFontContrastColor(Constants::DEFAULT_COLOR));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('#6'));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('#66'));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('#6666'));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('#cccc'));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('#ccccc'));
+        $this->assertEquals('#000000', $sut->getFontContrastColor('#ccccccc'));
     }
 }

tests/Validator/Constraints/HexColorValidatorTest.php

@@ -0,0 +1,97 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Validator\Constraints;
+
+use App\Validator\Constraints\HexColor;
+use App\Validator\Constraints\HexColorValidator;
+use Symfony\Component\Validator\Constraints\NotBlank;
+use Symfony\Component\Validator\Exception\UnexpectedTypeException;
+use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
+
+/**
+ * @covers \App\Validator\Constraints\HexColorValidator
+ */
+class HexColorValidatorTest extends ConstraintValidatorTestCase
+{
+    protected function createValidator()
+    {
+        return new HexColorValidator();
+    }
+
+    public function getValidColors()
+    {
+        yield ['#000'];
+        yield ['#aaa'];
+        yield ['#000000'];
+        yield ['#fff000'];
+        yield ['#000aaa'];
+        yield ['#fffaaa'];
+        yield ['']; // should actually be invalid, but it was allowed in the past :(
+        yield [null];
+    }
+
+    public function testConstraintIsInvalid()
+    {
+        $this->expectException(UnexpectedTypeException::class);
+
+        $this->validator->validate('#000', new NotBlank());
+    }
+
+    /**
+     * @dataProvider getValidColors
+     * @param string $color
+     */
+    public function testConstraintWithValidColor($color)
+    {
+        $constraint = new HexColor();
+        $this->validator->validate($color, $constraint);
+        $this->assertNoViolation();
+    }
+
+    public function getInvalidColors()
+    {
+        yield ['string'];
+        yield ['000'];
+        yield ['aaa'];
+        yield ['000000'];
+        yield ['fff000'];
+        yield ['000aaa'];
+        yield ['fffaaa'];
+        yield ['#f'];
+        yield ['#ff'];
+        yield ['#ffdd'];
+        yield ['#ffddd'];
+        yield ['#ffddddd'];
+        yield [new \stdClass(), 'object'];
+        yield [[], 'array'];
+    }
+
+    /**
+     * @dataProvider getInvalidColors
+     * @param mixed $color
+     */
+    public function testValidationError($color, $parameterType = null)
+    {
+        $constraint = new HexColor();
+
+        $this->validator->validate($color, $constraint);
+
+        if ($parameterType !== null) {
+            $expectedFormat = $parameterType;
+        } else {
+            $expectedFormat = \is_string($color) ? '"' . $color . '"' : $color;
+        }
+
+        $this->buildViolation('The given value is not a valid hexadecimal color.')
+            ->setParameter('{{ value }}', $expectedFormat)
+            ->setCode(HexColor::HEX_COLOR_ERROR)
+            ->assertRaised();
+    }
+}