Misc perf and validation fixes

config/packages/framework.yaml

@@ -14,6 +14,11 @@ framework:
         name: 'DAVIS_SESSION'
         storage_factory_id: session.storage.factory.native
 
+    # The CSS and JS only ever change with a release, so the version is the release. It also makes
+    # `asset()` prepend the base path, which a hard-coded `/css/…` does not on a sub-directory install.
+    assets:
+        version: !php/const App\Version::VERSION
+
     property_info:
         with_constructor_extractor: false
 

docker/configurations/Caddyfile

@@ -37,4 +37,9 @@
                 Permissions-Policy "camera=(), microphone=(), geolocation=()"
         }
 
+        # Every URL Davis emits for these carries a `?<version>` that changes with the release, so a
+        # stale copy can never be served: a new release is a new URL
+        @static path *.css *.js *.png *.jpg *.jpeg *.gif *.svg *.ico *.woff *.woff2
+        header @static Cache-Control "public, max-age=31536000, immutable"
+
 }

docker/configurations/nginx.conf

@@ -28,6 +28,21 @@ server {
         return 404;
     }
 
+    # Every URL Davis emits for these carries a `?<version>` that changes with the release, so a
+    # stale copy can never be served: a new release is a new URL
+    location ~* \.(css|js|png|jpe?g|gif|svg|ico|woff2?)$ {
+        add_header Cache-Control "public, max-age=31536000, immutable" always;
+
+        # An `add_header` here replaces the whole inherited set, so the server-level headers above
+        # have to be repeated or static files would be served without them
+        add_header X-Content-Type-Options nosniff always;
+        add_header X-Frame-Options DENY always;
+        add_header Referrer-Policy strict-origin-when-cross-origin always;
+        add_header Permissions-Policy "camera=(), microphone=(), geolocation=()" always;
+
+        try_files $uri =404;
+    }
+
     location / {
         try_files $uri $uri/ /index.php$is_args$args;
     }

migrations/Version20260930120000.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Version20250409193948 scaled every timestamp to BIGINT for the Year 2038 problem except this one.
+ */
+final class Version20260930120000 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Scale cards.lastmodified to big int for the Year 2038 problem';
+    }
+
+    public function up(Schema $schema): void
+    {
+        // SQLite stores every INTEGER as a 64-bit value, so it was never affected
+        if ($this->connection->getDatabasePlatform() instanceof SqlitePlatform) {
+            return;
+        }
+
+        if ($this->connection->getDatabasePlatform() instanceof AbstractMySQLPlatform) {
+            $this->addSql('ALTER TABLE cards CHANGE lastmodified lastmodified BIGINT DEFAULT NULL');
+        }
+
+        if ($this->connection->getDatabasePlatform() instanceof PostgreSQLPlatform) {
+            $this->addSql('ALTER TABLE cards ALTER COLUMN lastmodified TYPE BIGINT');
+        }
+    }
+
+    public function down(Schema $schema): void
+    {
+        // Narrowing back to INT would truncate post-2038 timestamps, like Version20250409193948
+    }
+}

public/.htaccess

@@ -5,6 +5,14 @@
 # to each configured DirectoryIndex file (e.g. index.php, index.html, index.pl).
 DirectoryIndex index.php
 
+# Every URL Davis emits for a static file carries a `?<version>` that changes with the release, so
+# a stale copy can never be served: a new release is a new URL.
+<IfModule mod_headers.c>
+    <FilesMatch "\.(css|js|png|jpe?g|gif|svg|ico|woff2?)$">
+        Header set Cache-Control "public, max-age=31536000, immutable"
+    </FilesMatch>
+</IfModule>
+
 # By default, Apache does not evaluate symbolic links if you did not enable this
 # feature in your server configuration. Uncomment the following line if you
 # install assets as symlinks or if you experience problems related to symlinks

src/Controller/Admin/UserController.php

@@ -202,14 +202,18 @@ public function userDelete(ManagerRegistry $doctrine, Request $request, #[MapEnt
     public function userDelegates(ManagerRegistry $doctrine, #[MapEntity(id: 'userId')] User $user, int $userId): Response
     {
         $principalUri = $user->getPrincipalUri();
+        $readProxyUri = $principalUri.Principal::READ_PROXY_SUFFIX;
+        $writeProxyUri = $principalUri.Principal::WRITE_PROXY_SUFFIX;
 
-        $principal = $doctrine->getRepository(Principal::class)->findOneByUri($principalUri);
+        // Delegates are not linked to the principal itself but to its proxies, so the three
+        // principals and the delegees the template reads off the proxies come back together
+        $principals = $doctrine->getRepository(Principal::class)->findWithDelegeesByUris([$principalUri, $readProxyUri, $writeProxyUri]);
 
-        $allPrincipalsExcept = $doctrine->getRepository(Principal::class)->findAllExceptPrincipal($principalUri);
+        $principal = $principals[$principalUri] ?? null;
+        $principalProxyRead = $principals[$readProxyUri] ?? null;
+        $principalProxyWrite = $principals[$writeProxyUri] ?? null;
 
-        // Get delegates. They are not linked to the principal in itself, but to its proxies
-        $principalProxyRead = $doctrine->getRepository(Principal::class)->findOneByUri($principal->getUri().Principal::READ_PROXY_SUFFIX);
-        $principalProxyWrite = $doctrine->getRepository(Principal::class)->findOneByUri($principal->getUri().Principal::WRITE_PROXY_SUFFIX);
+        $allPrincipalsExcept = $doctrine->getRepository(Principal::class)->findAllExceptPrincipal($principalUri);
 
         return $this->render('users/delegates.html.twig', [
             'principal' => $principal,

src/Entity/AddressBook.php

@@ -27,7 +27,8 @@ class AddressBook
     private $displayName;
 
     #[ORM\Column(type: 'string', length: 255)]
-    #[Assert\Regex("/[0-9a-z\-]+/")]
+    #[Assert\NotBlank]
+    #[Assert\Regex("/^[0-9a-zA-Z_\-]+$/")]
     #[Assert\Length(max: 255)]
     private $uri;
 

src/Entity/CalendarInstance.php

@@ -41,7 +41,8 @@ public static function getOwnerAccesses(): array
     private $displayName;
 
     #[ORM\Column(type: 'string', length: 255, nullable: true)]
-    #[Assert\Regex("/[0-9a-z\-]+/")]
+    #[Assert\NotBlank]
+    #[Assert\Regex("/^[0-9a-zA-Z_\-]+$/")]
     #[Assert\Length(max: 255)]
     private $uri;
 
@@ -52,7 +53,7 @@ public static function getOwnerAccesses(): array
     private $calendarOrder;
 
     #[ORM\Column(name: 'calendarcolor', type: 'string', length: 10, nullable: true)]
-    #[Assert\Regex("/\#[0-9A-F]{6}/")]
+    #[Assert\Regex('/^#([0-9A-Fa-f]{3,4}|[0-9A-Fa-f]{6}|[0-9A-Fa-f]{8})$/')]
     #[Assert\Length(max: 10)]
     private $calendarColor;
 

src/Entity/Card.php

@@ -26,7 +26,7 @@ class Card
     #[ORM\Column(type: 'string', length: 255, nullable: true)]
     private $uri;
 
-    #[ORM\Column(name: 'lastmodified', type: 'integer', nullable: true)]
+    #[ORM\Column(name: 'lastmodified', type: 'bigint', nullable: true)]
     private $lastModified;
 
     #[ORM\Column(type: 'string', length: 32, nullable: true)]

src/Repository/PrincipalRepository.php

@@ -33,6 +33,32 @@ public function findAllExceptPrincipal(string $principalUri)
             ->getResult();
     }
 
+    /**
+     * The delegates page needs a principal and both of its proxies, and reads the `delegees` of each
+     * proxy, which is six lookups done one at a time.
+     *
+     * @param string[] $uris
+     *
+     * @return array<string, Principal> keyed by uri, missing uris simply absent
+     */
+    public function findWithDelegeesByUris(array $uris): array
+    {
+        $principals = $this->createQueryBuilder('p')
+            ->leftJoin('p.delegees', 'd')
+            ->addSelect('d')
+            ->andWhere('p.uri IN (:uris)')
+            ->setParameter('uris', $uris)
+            ->getQuery()
+            ->getResult();
+
+        $byUri = [];
+        foreach ($principals as $principal) {
+            $byUri[$principal->getUri()] = $principal;
+        }
+
+        return $byUri;
+    }
+
     /**
      * @return array<array{Principal, userId: int}>
      */

templates/_partials/navigation.html.twig

@@ -1,7 +1,7 @@
 <nav class="navbar fixed-top navbar-expand-lg bg-body-tertiary">
   <div class="container">
     <a class="navbar-brand" href="{{ path('dashboard') }}">
-      <img src="/images/logo.png" width="30" height="30" alt=""> Davis
+      <img src="{{ asset('images/logo.png') }}" width="30" height="30" alt=""> Davis
     </a>
     <button class="navbar-toggler" type="button" data-bs-toggle="collapse" data-bs-target="#navbarNav" aria-controls="navbarNav" aria-expanded="false" aria-label="{{ "toggle.navigation"|trans }}">
       <span class="navbar-toggler-icon"></span>

templates/base.html.twig

@@ -4,17 +4,17 @@
         <meta charset="UTF-8">
         <meta name="csrf-token" content="{{ csrf_token('admin_action') }}">
         <title>{% block title %}Davis{% endblock %}</title>
-        <script type="text/javascript" src="/js/color.mode.toggler.js"></script>
-        <link rel="stylesheet" href="/css/bootstrap.min.css" />
-        <link rel="stylesheet" type="text/css" href="/css/style.css">
+        <script type="text/javascript" src="{{ asset('js/color.mode.toggler.js') }}"></script>
+        <link rel="stylesheet" href="{{ asset('css/bootstrap.min.css') }}" />
+        <link rel="stylesheet" type="text/css" href="{{ asset('css/style.css') }}">
     </head>
     <body>
         {% include '_partials/navigation.html.twig' %}
         {% include '_partials/flashes.html.twig' %}
         <div class="container">
             {% block body %}{% endblock %}
         </div>
-        <script type="text/javascript" src="/js/bootstrap.bundle.min.js"></script>
-        <script type="text/javascript" src="/js/app.js"></script>
+        <script type="text/javascript" src="{{ asset('js/bootstrap.bundle.min.js') }}"></script>
+        <script type="text/javascript" src="{{ asset('js/app.js') }}"></script>
     </body>
 </html>

templates/index.html.twig

@@ -4,11 +4,11 @@
         <meta charset="UTF-8">
         <meta name="viewport" content="width=device-width, initial-scale=1">
         <title>{% block title %}Davis{% endblock %}</title>
-        <link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon.png">
-        <link rel="icon" type="image/png" sizes="32x32" href="/favicon-32x32.png">
-        <link rel="icon" type="image/png" sizes="16x16" href="/favicon-16x16.png">
-        <link rel="manifest" href="/site.webmanifest">
-        <link rel="stylesheet" href="/css/bootstrap.min.css" />
+        <link rel="apple-touch-icon" sizes="180x180" href="{{ asset('apple-touch-icon.png') }}">
+        <link rel="icon" type="image/png" sizes="32x32" href="{{ asset('favicon-32x32.png') }}">
+        <link rel="icon" type="image/png" sizes="16x16" href="{{ asset('favicon-16x16.png') }}">
+        <link rel="manifest" href="{{ asset('site.webmanifest') }}">
+        <link rel="stylesheet" href="{{ asset('css/bootstrap.min.css') }}" />
         <style type="text/css">
           .hero {
             height: 100vh;
@@ -22,11 +22,11 @@
             width: 240px;
           }
         </style>
-        <script type="text/javascript" src="/js/color.mode.toggler.js"></script>
+        <script type="text/javascript" src="{{ asset('js/color.mode.toggler.js') }}"></script>
     </head>
     <body>
       <div class="hero">
-        <img class="mb-2" src="/images/logo.png" width="60px">
+        <img class="mb-2" src="{{ asset('images/logo.png') }}" width="60px">
         <h3 class="mb-4">{{ "davis"|trans }}</h3>
         <ul class="list-group">
           <li class="caldav list-group-item d-flex justify-content-between align-items-center">

tests/Functional/Controllers/AddressBookControllerTest.php

@@ -125,6 +125,34 @@ public function testAddressBookNewRefusesOverLongValues(): void
         $this->assertNull($em->getRepository(AddressBook::class)->findOneBy(['uri' => str_repeat('a', 256)]));
     }
 
+    /**
+     * The uri becomes a path segment under `/dav/addressbooks/<user>/`, so it has to be a single
+     * segment and nothing else.
+     */
+    public function testAddressBookNewRefusesAUriThatIsNotOneSegment(): void
+    {
+        $client = static::createClient();
+        $client->loginUser(new AdminUser('admin', 'test'));
+
+        $userId = $this->getUserId($client, 'test_user');
+        $em = static::getContainer()->get('doctrine.orm.entity_manager');
+
+        foreach (['../../evil', 'a/b', 'has space', 'dots.and.more', ''] as $uri) {
+            $crawler = $client->request('GET', '/addressbooks/'.$userId.'/new');
+            $form = $crawler->selectButton('address_book_save')->form();
+
+            $client->submit($form, [
+                'address_book[uri]' => $uri,
+                'address_book[displayName]' => 'Traversal',
+                'address_book[description]' => 'nope',
+            ]);
+
+            $this->assertResponseIsSuccessful('"'.$uri.'" should come back as a form error');
+            $this->assertSelectorExists('.invalid-feedback, .form-error-message');
+            $this->assertNull($em->getRepository(AddressBook::class)->findOneBy(['uri' => $uri]));
+        }
+    }
+
     /**
      * The field is mapped, so `handleRequest()` writes it onto the entity and the flush persists
      * it, both ways round.

tests/Functional/Controllers/CalendarControllerTest.php

@@ -319,6 +319,76 @@ public function testCalendarNewRefusesOverLongValues(): void
         $this->assertNull($em->getRepository(CalendarInstance::class)->findOneBy(['uri' => str_repeat('a', 256)]));
     }
 
+    /**
+     * The uri becomes a path segment under `/dav/calendars/<user>/`, so it has to be a single
+     * segment and nothing else.
+     */
+    public function testCalendarNewRefusesAUriThatIsNotOneSegment(): void
+    {
+        $client = $this->loggedInClient();
+
+        $userId = $this->getUserId($client, 'test_user');
+        $em = static::getContainer()->get('doctrine.orm.entity_manager');
+
+        foreach (['../../evil', 'a/b', 'has space', 'dots.and.more', ''] as $uri) {
+            $crawler = $client->request('GET', '/calendars/'.$userId.'/new');
+            $form = $crawler->selectButton('calendar_instance_save')->form();
+
+            $client->submit($form, [
+                'calendar_instance[uri]' => $uri,
+                'calendar_instance[displayName]' => 'Traversal',
+                'calendar_instance[description]' => 'nope',
+                'calendar_instance[calendarColor]' => '#001122',
+            ]);
+
+            $this->assertResponseIsSuccessful('"'.$uri.'" should come back as a form error');
+            $this->assertSelectorExists('.invalid-feedback, .form-error-message');
+            $this->assertNull($em->getRepository(CalendarInstance::class)->findOneBy(['uri' => $uri]));
+        }
+    }
+
+    /**
+     * The form has to accept exactly what the `colour()` macro renders, and refuse the rest: a
+     * value it accepts but the macro blanks out is a colour the admin cannot set.
+     */
+    public function testTheColourFieldAgreesWithWhatThePageRenders(): void
+    {
+        $client = $this->loggedInClient();
+
+        $userId = $this->getUserId($client, 'test_user');
+        $em = static::getContainer()->get('doctrine.orm.entity_manager');
+
+        $submit = function (string $uri, string $colour) use ($client, $userId): void {
+            $crawler = $client->request('GET', '/calendars/'.$userId.'/new');
+            $form = $crawler->selectButton('calendar_instance_save')->form();
+
+            $client->submit($form, [
+                'calendar_instance[uri]' => $uri,
+                'calendar_instance[displayName]' => 'Colour '.$uri,
+                'calendar_instance[description]' => '',
+                'calendar_instance[calendarColor]' => $colour,
+            ]);
+        };
+
+        // Lowercase and the 8-digit form the placeholder advertises were both refused before
+        foreach (['#abc', '#aabbcc', '#00112233'] as $i => $colour) {
+            $uri = 'colour_ok_'.$i;
+            $submit($uri, $colour);
+
+            $this->assertResponseRedirects('/calendars/'.$userId, null, '"'.$colour.'" should be accepted');
+            $this->assertSame($colour, $em->getRepository(CalendarInstance::class)->findOneBy(['uri' => $uri])->getCalendarColor());
+        }
+
+        foreach (['red;x:y', 'red', '#gggggg', '#12345'] as $i => $colour) {
+            $uri = 'colour_bad_'.$i;
+            $submit($uri, $colour);
+
+            $this->assertResponseIsSuccessful('"'.$colour.'" should come back as a form error');
+            $this->assertSelectorExists('.invalid-feedback, .form-error-message');
+            $this->assertNull($em->getRepository(CalendarInstance::class)->findOneBy(['uri' => $uri]));
+        }
+    }
+
     /**
      * A calendar colour is whatever the owner's client sent over CalDAV, and it lands inside a
      * `style` attribute. HTML escaping does not stop it from closing the declaration and adding