♻️ Address review findings on link-preview branch

Keep subpath in the link-preview human redirect, fail fast on absolute join-uri segments, clarify the valueless-param behavior, and detect Applebot, Googlebot, Bing and DuckDuckGo crawlers.

AI-assisted-by: muse-spark-1.3-contributor
This commit is contained in:
Andrey Antukh 2026-09-09 19:06:11 +00:00
parent 4fc177ae43
commit e24169d7f0
7 changed files with 19 additions and 8 deletions

View File

@ -17,6 +17,6 @@
<meta name="twitter:image" content="{{image}}" /> <meta name="twitter:image" content="{{image}}" />
</head> </head>
<body> <body>
<script>location.replace("/" + location.search + location.hash);</script> <script>location.replace((location.pathname.replace(/link-preview\/?$/, "") || "/") + location.search + location.hash);</script>
</body> </body>
</html> </html>

View File

@ -383,6 +383,8 @@
a trailing slash; segments must not start with `/` (a leading slash a trailing slash; segments must not start with `/` (a leading slash
would resolve against the host root and drop the subpath)." would resolve against the host root and drop the subpath)."
[base & segments] [base & segments]
(assert (not (some #(str/starts-with? % "/") segments))
"URI segments must be relative (no leading slash)")
(str (apply u/join (u/ensure-path-slash base) segments))) (str (apply u/join (u/ensure-path-slash base) segments)))
(defn get-public-uri (defn get-public-uri

View File

@ -33,3 +33,8 @@
(cf/join-uri "https://nitrate.example.com" "api/teams/123"))) (cf/join-uri "https://nitrate.example.com" "api/teams/123")))
(t/is (= "https://nitrate.example.com/api/teams/123" (t/is (= "https://nitrate.example.com/api/teams/123"
(cf/join-uri "https://nitrate.example.com/" "api/teams/123"))))) (cf/join-uri "https://nitrate.example.com/" "api/teams/123")))))
(t/deftest join-uri-rejects-leading-slash
(t/testing "a leading slash would drop the subpath, so it fails fast"
(t/is (thrown? AssertionError
(cf/join-uri "https://example.com/penpot" "/assets/by-id/123")))))

View File

@ -53,7 +53,7 @@ http {
# are served with dynamic Open Graph metadata from the backend. # are served with dynamic Open Graph metadata from the backend.
map $http_user_agent $penpot_link_preview_agent { map $http_user_agent $penpot_link_preview_agent {
default 0; default 0;
~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky) 1; ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky|applebot|googlebot|bingbot|bingpreview|duckduckbot) 1;
} }
proxy_cache_path /tmp/cache/ levels=2:2 keys_zone=penpot:20m; proxy_cache_path /tmp/cache/ levels=2:2 keys_zone=penpot:20m;

View File

@ -61,7 +61,7 @@ http {
# are served with dynamic Open Graph metadata from the backend. # are served with dynamic Open Graph metadata from the backend.
map $http_user_agent $penpot_link_preview_agent { map $http_user_agent $penpot_link_preview_agent {
default 0; default 0;
~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky) 1; ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky|applebot|googlebot|bingbot|bingpreview|duckduckbot) 1;
} }
proxy_cache_path /tmp/cache/ levels=2:2 keys_zone=penpot:20m; proxy_cache_path /tmp/cache/ levels=2:2 keys_zone=penpot:20m;

View File

@ -86,7 +86,7 @@ A `map` block classifies the request by `User-Agent`:
```nginx ```nginx
map $http_user_agent $penpot_link_preview_agent { map $http_user_agent $penpot_link_preview_agent {
default 0; default 0;
~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky) 1; ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky|applebot|googlebot|bingbot|bingpreview|duckduckbot) 1;
} }
``` ```
@ -163,12 +163,14 @@ equivalent `twitter:*` card tags and `<meta name="robots" content="noindex">`.
The body contains a single script: The body contains a single script:
```html ```html
<script>location.replace("/" + location.search + location.hash);</script> <script>location.replace((location.pathname.replace(/link-preview\/?$/, "") || "/") + location.search + location.hash);</script>
``` ```
so that if a *human* somehow lands on `/link-preview` (e.g. some clients let users so that if a *human* somehow lands on `/link-preview` (e.g. some clients let users
click through to the fetched URL), the browser bounces back to the SPA root click through to the fetched URL), the browser bounces back to the SPA root
keeping the query string and the fragment, and the app loads normally. Crawlers do not execute keeping the query string and the fragment, and the app loads normally. The
redirect strips only the trailing `link-preview` segment so subpath
deployments keep their prefix. Crawlers do not execute
JavaScript, so they just read the meta tags. JavaScript, so they just read the meta tags.
### Making file thumbnails publicly accessible ### Making file thumbnails publicly accessible

View File

@ -103,8 +103,10 @@
;; the effect always runs after the update. The sharing-context ids ;; the effect always runs after the update. The sharing-context ids
;; are synced into the pre-fragment query (the fragment never reaches ;; are synced into the pre-fragment query (the fragment never reaches
;; the server, so shared links need them there); every other param is ;; the server, so shared links need them there); every other param is
;; left untouched. The backend applies its own file > project > team ;; left untouched, except valueless ones (`?flag`), which the query
;; priority, so no filtering is needed here. ;; codec cannot round-trip and are dropped. The backend applies its
;; own file > project > team priority, so no filtering is needed
;; here.
(let [params (:query-params (:route state)) (let [params (:query-params (:route state))
uri (u/uri (.-href globals/location)) uri (u/uri (.-href globals/location))
search (reduce (fn [m k] search (reduce (fn [m k]