diff --git a/client/src/api/client.ts b/client/src/api/client.ts index c6bc1d065..d3c60819c 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -971,9 +971,14 @@ export const journeyApi = { // Photo providers (Immich, Synology Photos, …) behind /api/integrations/memories. // The provider sits on the user's own hardware and the server proxies through to -// it, so these get a longer leash than the 8s default — a cold Immich answering a -// date-range search regularly needs more. -const MEMORIES_TIMEOUT = 20000 +// it, so the 8s default does not apply — and neither does any number picked +// here. The server already holds the deadline, and its worst case is longer than +// anything that would look reasonable in this file: a Synology call is 30s, and +// an expired session makes it re-authenticate and try again. A client cap below +// that turns a slow NAS into a NAS that reads as disconnected, which is what +// these calls looked like before they moved off raw fetch (which had no timeout +// either). Callers that need to give up early pass an AbortSignal. +const MEMORIES_TIMEOUT = 0 export const memoriesApi = { status: (provider: string): Promise<{ connected: boolean }> => diff --git a/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx b/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx index 4a8e64832..21f59cdb7 100644 Binary files a/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx and b/client/src/components/Collab/CollabNotesWebsiteThumbnail.tsx differ diff --git a/client/src/components/Planner/ReservationsPanel.tsx b/client/src/components/Planner/ReservationsPanel.tsx index 88ec7e864..74d5eb2a6 100644 --- a/client/src/components/Planner/ReservationsPanel.tsx +++ b/client/src/components/Planner/ReservationsPanel.tsx @@ -12,7 +12,7 @@ import { TramFront, Footprints, StickyNote, ParkingSquare, } from 'lucide-react' import { openFile } from '../../utils/fileDownload' -import { safeHttpUrl } from '../../utils/safeUrl' +import { safeExternalHref } from '../../utils/safeUrl' import { TransitTitle, TransitLegChips, TransitMetaBadges, fmtTransitDuration } from './transitDisplay' import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' @@ -117,9 +117,10 @@ function ReservationCard({ r, tripId, onEdit, onDelete, files = [], onNavigateTo })() const attachedFiles = files.filter(f => f.reservation_id === r.id || (f.linked_reservation_ids || []).includes(r.id)) const linked = r.assignment_id ? assignmentLookup[r.assignment_id] : null - // A booking link is free-form text on rows written before the contract checked it, - // so only an http(s) value becomes a real anchor — the rest stays readable text. - const bookingUrl = safeHttpUrl(r.url) + // A booking link is deliberately free-form - people paste bare hosts and long + // provider deep links - so this refuses the schemes that execute in this + // origin and leaves everything else linkable. + const bookingUrl = safeExternalHref(r.url) const [showDeleteConfirm, setShowDeleteConfirm] = useState(false) const handleDelete = async () => { diff --git a/client/src/store/slices/placesSlice.test.ts b/client/src/store/slices/placesSlice.test.ts index a3c1b0687..6ab305dea 100644 --- a/client/src/store/slices/placesSlice.test.ts +++ b/client/src/store/slices/placesSlice.test.ts @@ -28,6 +28,11 @@ describe('placesSlice', () => { const assignment = buildAssignment({ id: 100, day_id: 3, + // The override is what makes the embedded copy differ from the pool row: + // the server projects COALESCE(assignment_time, place_time), so without + // it these two could not disagree in the first place. + assignment_time: '09:00', + assignment_end_time: '10:30', place: { ...place, place_time: '09:00', end_time: '10:30' }, }); seedStore(useTripStore, { places: [place], assignments: { '3': [assignment] } }); @@ -43,7 +48,7 @@ describe('placesSlice', () => { expect(useTripStore.getState().places[0].image_url).toBe('/uploads/places/pic.jpg'); const embedded = useTripStore.getState().assignments['3'][0].place; expect(embedded.image_url).toBe('/uploads/places/pic.jpg'); - // The assignment owns its own times — the fresh place must not overwrite them. + // The assignment owns these times, so the fresh place must not overwrite them. expect(embedded.place_time).toBe('09:00'); expect(embedded.end_time).toBe('10:30'); }); diff --git a/client/src/store/slices/placesSlice.ts b/client/src/store/slices/placesSlice.ts index 8dc2c8b88..64fdf9e44 100644 --- a/client/src/store/slices/placesSlice.ts +++ b/client/src/store/slices/placesSlice.ts @@ -19,12 +19,30 @@ export interface PlacesSlice { updatePlacesMany: (tripId: number | string, placeIds: number[], patch: Partial) => Promise } -/** Swap the pool place embedded in an assignment while keeping the assignment's - * own times: the day view shows COALESCE(assignment_time, place_time), so - * overwriting them with the pool row would wipe a per-stop schedule. Exported - * because the WS applier in remoteEventHandler has to merge the same way. */ +/** + * Swap the pool place embedded in an assignment, resolving its times the way + * the server does. + * + * The embedded copy is a projection, not the pool row: assignments.service + * selects COALESCE(da.assignment_time, p.place_time). So a per-stop override + * wins and has to survive a pool edit, and where there is no override the pool + * time is what the day card shows - which means a pool edit must reach it. + * Pinning the old embedded times instead would keep a renamed place's schedule + * frozen until the next reload. + * + * Exported because the WS applier in remoteEventHandler has to merge + * identically; two spellings of this rule is how the two paths drifted apart in + * the first place. + */ export function mergeAssignmentPlace(a: Assignment, place: Place): Assignment { - return { ...a, place: { ...place, place_time: a.place.place_time, end_time: a.place.end_time } } + return { + ...a, + place: { + ...place, + place_time: a.assignment_time ?? place.place_time, + end_time: a.assignment_end_time ?? place.end_time, + }, + } } /** Replace a place in the pool and in every day-assignment that embeds it, diff --git a/client/src/sync/mutationQueue.ts b/client/src/sync/mutationQueue.ts index ab7035107..1c21ab487 100644 --- a/client/src/sync/mutationQueue.ts +++ b/client/src/sync/mutationQueue.ts @@ -144,6 +144,12 @@ export const mutationQueue = { .sortBy('createdAt') for (const mutation of pending) { + // Re-checked every pass, not just on entry: this loop writes server + // responses straight into Dexie, and after a logout the proxy points at + // the shared anonymous database. A flush that started before logout + // would otherwise seed it with the previous account's rows. + if (!isAuthed()) break + // Mark as syncing so UI can show progress. The stamp is what lets the // next flush tell an in-flight row from one a killed tab abandoned. await offlineDb.mutationQueue.update(mutation.id, { status: 'syncing', syncingSince: Date.now() }) diff --git a/client/src/sync/tripSyncManager.ts b/client/src/sync/tripSyncManager.ts index d38acd18e..64de8af8b 100644 --- a/client/src/sync/tripSyncManager.ts +++ b/client/src/sync/tripSyncManager.ts @@ -209,9 +209,11 @@ export const tripSyncManager = { } } - // Cache global user data (tags + categories) — fire-and-forget - tagsApi.list().then(d => upsertTags(d.tags)).catch(() => {}) - categoriesApi.list().then(d => upsertCategories(d.categories)).catch(() => {}) + // Cache global user data (tags + categories) — fire-and-forget, so the + // gate has to be re-read when the response lands, not when it was issued: + // a logout in between would put these rows in the anonymous database. + tagsApi.list().then(d => { if (isAuthed()) upsertTags(d.tags) }).catch(() => {}) + categoriesApi.list().then(d => { if (isAuthed()) upsertCategories(d.categories) }).catch(() => {}) // Cache file blobs + map tiles in background (don't block syncAll) const cacheTiles = getOfflinePrefs().cacheTiles diff --git a/client/src/utils/safeUrl.ts b/client/src/utils/safeUrl.ts index a89390fa1..ba87d71b6 100644 --- a/client/src/utils/safeUrl.ts +++ b/client/src/utils/safeUrl.ts @@ -16,3 +16,31 @@ import { placeWebsiteSchema } from '@trek/shared' export function safeHttpUrl(value: string | null | undefined): string | null { return typeof value === 'string' && placeWebsiteSchema.safeParse(value).success ? value : null } + +/** + * A user-pasted link, made safe for an href without narrowing what people are + * allowed to paste. + * + * Different job from safeHttpUrl above. That one guards fields whose contract + * really is http(s)-only and capped. A booking url is deliberately free-form - + * shared's reservationUrlSchema says so in as many words, because people paste + * bare hosts and very long provider deep links - so holding it to the stricter + * contract would turn a link somebody saved last year into dead text. + * + * Only the schemes that execute in this origin are refused. A value with no + * scheme gets https://, which is what an address bar does with the same input; + * without it the browser would resolve "www.hotel.com" against the current page + * and navigate inside the app. + */ +export function safeExternalHref(value: string | null | undefined): string | null { + if (typeof value !== 'string') return null + const trimmed = value.trim() + if (!trimmed) return null + // Browsers strip control characters before they resolve a scheme, so the test + // has to run against the same string they will see. + const collapsed = Array.from(trimmed).filter(c => c > ' ').join('') + if (/^https?:\/\//i.test(collapsed)) return trimmed + // Any other scheme, executing or merely unexpected, stays unlinked. + if (/^[a-z][a-z0-9+.-]*:/i.test(collapsed)) return null + return `https://${trimmed}` +} diff --git a/client/tests/unit/remoteEventHandler/places.test.ts b/client/tests/unit/remoteEventHandler/places.test.ts index 9fb244581..82b2ca238 100644 --- a/client/tests/unit/remoteEventHandler/places.test.ts +++ b/client/tests/unit/remoteEventHandler/places.test.ts @@ -51,14 +51,16 @@ describe('remoteEventHandler > places', () => { expect(assignments['10'][0].place?.name).toBe('Cascaded Update'); }); - it('FE-WSEVT-PLACE-004b: place:updated keeps the assignment its own times', () => { - // The day view renders COALESCE(assignment_time, place_time), so the copy - // embedded in an assignment carries the per-stop schedule. A broadcast about - // the pool row must not overwrite it (the local reducer merges the same way). + it('FE-WSEVT-PLACE-004b: place:updated keeps a per-stop override', () => { + // The day view renders COALESCE(assignment_time, place_time), so an + // assignment that carries its own time keeps it when the pool row changes + // (the local reducer merges the same way). const place = buildPlace({ id: 1, name: 'Original', place_time: '08:00', end_time: '09:00' }); const assignment = buildAssignment({ id: 100, day_id: 10, + assignment_time: '14:00', + assignment_end_time: '15:30', place: { ...place, place_time: '14:00', end_time: '15:30' }, }); useTripStore.setState({ places: [place], assignments: { '10': [assignment] } }); @@ -74,6 +76,31 @@ describe('remoteEventHandler > places', () => { expect(embedded.end_time).toBe('15:30'); }); + it('FE-WSEVT-PLACE-004c: without an override the pool time reaches the day card', () => { + // The other half of the COALESCE. An assignment with no assignment_time + // shows whatever the pool row says, so a collaborator moving the place from + // 08:00 to 10:00 has to land here - freezing the embedded copy would leave + // the day plan wrong until the next reload. + const place = buildPlace({ id: 1, name: 'Original', place_time: '08:00', end_time: '09:00' }); + const assignment = buildAssignment({ + id: 100, + day_id: 10, + assignment_time: null, + assignment_end_time: null, + place: { ...place }, + }); + useTripStore.setState({ places: [place], assignments: { '10': [assignment] } }); + + useTripStore.getState().handleRemoteEvent({ + type: 'place:updated', + place: buildPlace({ id: 1, name: 'Original', place_time: '10:00', end_time: '11:00' }), + }); + + const embedded = useTripStore.getState().assignments['10'][0].place!; + expect(embedded.place_time).toBe('10:00'); + expect(embedded.end_time).toBe('11:00'); + }); + it('FE-WSEVT-PLACE-005: place:deleted removes place from places array', () => { seedData(); useTripStore.getState().handleRemoteEvent({ type: 'place:deleted', placeId: 1 });