mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(places): a website without a scheme no longer blocks saving a place
A place from the TREK index can carry its website the way Overture has it, such as fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët, and saving it to a collection or a trip failed with "website must be an http or https URL" over a field the dialog does not show (#2483). OpenStreetMap, Amap and booking mails hand over websites the same way. Every place a website comes in from outside now goes through one shared helper, normalizePlaceWebsite: the index client and its nearby search, the Overpass and OSM detail mappers, Google search and details, Amap, plugin search and the booking import. It completes a bare or protocol-relative host to https, keeps http and https as they are, and drops every other scheme and anything that is no address. Only the scheme is added, so an accented path is stored as written. placeWebsiteSchema completes a bare host the same way before its check, so an older client, a cached offline result, an MCP agent or a plugin is no longer refused, while javascript:, data: and mailto: still are. The REST controller, the plugin RPC and the enrichment now store the parsed value instead of the raw one, and create_place_accommodation takes the website through the same schema as create_place. safeHttpUrl still links only a value already stored in its final form, so nothing changes on screen. Tests: SHARED-WEBSITE-001 to 012, SHARED-PLACE-030 to 034, SHARED-COLLFILE-2483-01, COLLECTIONS-E2E-089 and 090, PLACES-E2E-2483-01 and 02, PLACES-CTRL-2483-01 (the louvre.fr case adjusted), MCP-PLACES-2483-01 to 03, MCP-ACCOM-2483-01, MCP-COLL-2483-01, PLACES-RPC-2483-01, MAPS-2483-01 to 05, MAPS-POIS-015, MAPS-GERS-010, AMAP-012b, TREK-PLACES-2483-01, PLUGIN-SEARCH-2483-01, BOOKING-IMPORT-2483-01, ENRICH-052e, ENRICH-102 extended, ENRICH-120 adjusted, FE-UTIL-SAFEURL-007.
This commit is contained in:
Maurice
@@ -27,6 +27,14 @@ describe('safeHttpUrl', () => {
|
||||
expect(safeHttpUrl('//evil.example')).toBeNull()
|
||||
})
|
||||
|
||||
// #2483: the write contract now completes a bare host to https. What is read
|
||||
// back here is the stored text, which is linked only when it is already final.
|
||||
it('FE-UTIL-SAFEURL-007: a value the contract would complete is still not linked as stored', () => {
|
||||
expect(safeHttpUrl('fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët')).toBeNull()
|
||||
expect(safeHttpUrl(' https://louvre.fr')).toBeNull()
|
||||
expect(safeHttpUrl('https://louvre.fr ')).toBe('https://louvre.fr ')
|
||||
})
|
||||
|
||||
it('FE-UTIL-SAFEURL-006: treats absent and empty as nothing to open', () => {
|
||||
expect(safeHttpUrl(null)).toBeNull()
|
||||
expect(safeHttpUrl(undefined)).toBeNull()
|
||||
|
||||
@@ -14,7 +14,12 @@ import { placeWebsiteSchema } from '@trek/shared'
|
||||
* have not updated yet — the same split as safeHexColor.
|
||||
*/
|
||||
export function safeHttpUrl(value: string | null | undefined): string | null {
|
||||
return typeof value === 'string' && placeWebsiteSchema.safeParse(value).success ? value : null
|
||||
if (typeof value !== 'string') return null
|
||||
const parsed = placeWebsiteSchema.safeParse(value)
|
||||
// The contract completes a bare host to https on the way in (#2483), but the
|
||||
// value handed to window.open here is the one stored, and "louvre.fr" would
|
||||
// resolve against this app. So only a value already in its final form passes.
|
||||
return parsed.success && parsed.data === value ? value : null
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
demoDenied, ok,
|
||||
} from '../../nest-mcp';
|
||||
import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
|
||||
import { placeWebsiteSchema } from '@trek/shared';
|
||||
import { z } from 'zod';
|
||||
import { AuthService } from '../auth/auth.service';
|
||||
import { PlacesService } from '../places/places.service';
|
||||
@@ -92,7 +93,8 @@ export class AccommodationsMcp {
|
||||
google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'),
|
||||
osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345")'),
|
||||
place_notes: z.string().max(2000).optional().describe('Notes for the place'),
|
||||
website: z.string().max(500).optional(),
|
||||
// The place contract create_place uses: a bare host gains https (#2483).
|
||||
website: placeWebsiteSchema.optional(),
|
||||
phone: z.string().max(50).optional(),
|
||||
start_day_id: z.number().int().positive().describe('Check-in day ID'),
|
||||
end_day_id: z.number().int().positive().describe('Check-out day ID'),
|
||||
|
||||
@@ -12,7 +12,7 @@ import type { User } from '../../types';
|
||||
import { KitineraryExtractorService } from './kitinerary-extractor.service';
|
||||
import { LlmParseService } from '../llm-parse/llm-parse.service';
|
||||
import { mapReservations } from './kitinerary-mapper';
|
||||
import { typeToCostCategory } from '@trek/shared';
|
||||
import { normalizePlaceWebsite, typeToCostCategory } from '@trek/shared';
|
||||
import type { BookingImportPreviewItem, BookingImportPreviewResponse, BookingImportConfirmResponse, BookingImportMode, BookingImportFileReport, Reservation } from '@trek/shared';
|
||||
import type { ParsedBookingItem, KiReservation } from './kitinerary.types';
|
||||
|
||||
@@ -257,7 +257,9 @@ export class BookingImportService {
|
||||
lat,
|
||||
lng,
|
||||
address: _venue.address,
|
||||
website: _venue.website,
|
||||
// A booking mail gives the venue's site however its sender wrote it;
|
||||
// it lands as https or not at all (#2483).
|
||||
website: normalizePlaceWebsite(_venue.website) ?? undefined,
|
||||
phone: _venue.phone,
|
||||
});
|
||||
placeId = (place as any).id;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { normalizePlaceWebsite } from '@trek/shared';
|
||||
import { readEnv, getAppUrl } from '../../app-config';
|
||||
import { stripHtmlTags } from '../common/stripHtmlTags';
|
||||
import { haversineMetres } from '../common/geo';
|
||||
@@ -661,7 +662,7 @@ export function buildOsmDetails(tags: Record<string, string>, osmType: string, o
|
||||
}
|
||||
}
|
||||
return {
|
||||
website: tags['contact:website'] || tags.website || null,
|
||||
website: normalizePlaceWebsite(tags['contact:website']) ?? normalizePlaceWebsite(tags.website),
|
||||
phone: tags['contact:phone'] || tags.phone || null,
|
||||
opening_hours,
|
||||
open_now,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { isOutsideChina } from '@trek/shared';
|
||||
import { isOutsideChina, normalizePlaceWebsite } from '@trek/shared';
|
||||
import type {
|
||||
MapsSearchResult,
|
||||
MapsAutocompleteResult,
|
||||
@@ -972,7 +972,7 @@ export class MapsService {
|
||||
category: labelFor(p.category ?? null, p.categoryPath ?? null) ?? wanted[0],
|
||||
poi_type: p.category ?? wanted[0],
|
||||
address: p.address?.freeform ?? null,
|
||||
website: p.contact?.website ?? null,
|
||||
website: normalizePlaceWebsite(p.contact?.website),
|
||||
phone: p.contact?.phone ?? null,
|
||||
opening_hours: p.hours?.osm ?? null,
|
||||
// The index carries the chain and its Wikidata item, which is what
|
||||
@@ -1493,7 +1493,7 @@ export class MapsService {
|
||||
category: categoryOfFilter.get(matched) ?? categories[0],
|
||||
poi_type: matched,
|
||||
address: addr,
|
||||
website: tags.website || tags['contact:website'] || null,
|
||||
website: normalizePlaceWebsite(tags.website) ?? normalizePlaceWebsite(tags['contact:website']),
|
||||
phone: tags.phone || tags['contact:phone'] || null,
|
||||
opening_hours: tags.opening_hours || null,
|
||||
cuisine: tags.cuisine || null,
|
||||
@@ -2208,7 +2208,7 @@ export class MapsService {
|
||||
lat: p.location?.latitude ?? null,
|
||||
lng: p.location?.longitude ?? null,
|
||||
rating: p.rating || null,
|
||||
website: p.websiteUri || null,
|
||||
website: normalizePlaceWebsite(p.websiteUri),
|
||||
phone: p.nationalPhoneNumber || null,
|
||||
types: p.types || [],
|
||||
source: 'google',
|
||||
@@ -2570,7 +2570,7 @@ export class MapsService {
|
||||
lng: data.location?.longitude ?? null,
|
||||
rating: data.rating || null,
|
||||
rating_count: data.userRatingCount || null,
|
||||
website: data.websiteUri || null,
|
||||
website: normalizePlaceWebsite(data.websiteUri),
|
||||
phone: data.nationalPhoneNumber || null,
|
||||
opening_hours: data.regularOpeningHours?.weekdayDescriptions || null,
|
||||
open_now: data.regularOpeningHours?.openNow ?? null,
|
||||
@@ -2715,7 +2715,7 @@ export class MapsService {
|
||||
lng: data.location?.longitude ?? null,
|
||||
rating: data.rating || null,
|
||||
rating_count: data.userRatingCount || null,
|
||||
website: data.websiteUri || null,
|
||||
website: normalizePlaceWebsite(data.websiteUri),
|
||||
phone: data.nationalPhoneNumber || null,
|
||||
opening_hours: data.regularOpeningHours?.weekdayDescriptions || null,
|
||||
open_now: data.regularOpeningHours?.openNow ?? null,
|
||||
|
||||
@@ -29,7 +29,7 @@
|
||||
* the same way a place found on OpenStreetMap does.
|
||||
*/
|
||||
import { createHash } from 'node:crypto';
|
||||
import { fromAmapLocation, gcj02ToWgs84, toAmapLocation } from '@trek/shared';
|
||||
import { fromAmapLocation, gcj02ToWgs84, normalizePlaceWebsite, toAmapLocation } from '@trek/shared';
|
||||
import { readEnv } from '../../../app-config';
|
||||
import { safeFetchFollow } from '../../../utils/ssrfGuard';
|
||||
import { discardBody, exceedsDeclaredLength, readCappedJson } from '../../../utils/cappedFetch';
|
||||
@@ -403,7 +403,8 @@ export class AmapPlacesProvider implements PlacesProvider {
|
||||
lng: coords?.lng ?? null,
|
||||
rating: amapNumber(business.rating ?? bizExt.rating),
|
||||
rating_count: null,
|
||||
website: amapText(poi.website) || null,
|
||||
// Free text, often a bare host: it gains https here like any other source's.
|
||||
website: normalizePlaceWebsite(amapText(poi.website)),
|
||||
phone: amapText(business.tel ?? poi.tel) || null,
|
||||
// Amap's type is a slash-separated taxonomy ("餐饮服务;中餐厅;川菜"); split so
|
||||
// it reads like the string array every other provider returns.
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
* to configure and no quota to exhaust, so the failure modes are the network
|
||||
* and the service being down, both of which fall back to what TREK did before.
|
||||
*/
|
||||
import { normalizePlaceWebsite } from '@trek/shared';
|
||||
import { readEnv, getAppUrl } from '../../app-config';
|
||||
|
||||
/** The public instance. An operator may point at their own copy instead. */
|
||||
@@ -425,7 +426,8 @@ export function toPlaceRecord(p: TrekPlace): Record<string, unknown> {
|
||||
lng: Number.isFinite(p.lng) ? p.lng : null,
|
||||
// Ratings exist nowhere in open data, and saying so with null is honest.
|
||||
rating: null,
|
||||
website: p.contact?.website ?? null,
|
||||
// The index keeps websites as Overture has them, often without a scheme.
|
||||
website: normalizePlaceWebsite(p.contact?.website),
|
||||
phone: p.contact?.phone ?? null,
|
||||
email: p.contact?.email ?? null,
|
||||
category: p.category ?? null,
|
||||
|
||||
@@ -192,9 +192,10 @@ function yesNo(value: unknown): 'yes' | 'no' | string | null {
|
||||
* dropped rather than linked.
|
||||
*/
|
||||
function googleMapsLink(value: unknown): string | null {
|
||||
if (typeof value !== 'string' || !placeWebsiteSchema.safeParse(value).success) return null;
|
||||
const parsed = placeWebsiteSchema.safeParse(value);
|
||||
if (!parsed.success) return null;
|
||||
try {
|
||||
return isGoogleMapsHost(new URL(value).hostname) ? value : null;
|
||||
return isGoogleMapsHost(new URL(parsed.data).hostname) ? parsed.data : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
@@ -233,8 +234,8 @@ export function collectFacts(details: Record<string, unknown> | null): PlaceFact
|
||||
// `menu_url` is a community-editable OSM tag that becomes an href on the
|
||||
// client, so it goes through the same allow-list as a place's website —
|
||||
// anything but http(s) is dropped rather than rendered as a link.
|
||||
const menu = typeof details.menu_url === 'string' ? details.menu_url.trim() : '';
|
||||
if (placeWebsiteSchema.safeParse(menu).success) push('menu', null, menu);
|
||||
const menu = placeWebsiteSchema.safeParse(typeof details.menu_url === 'string' ? details.menu_url.trim() : '');
|
||||
if (menu.success) push('menu', null, menu.data);
|
||||
|
||||
if (yesNo(details.outdoor_seating) === 'yes') push('outdoorSeating', null);
|
||||
if (yesNo(details.takeaway) === 'yes') push('takeaway', null);
|
||||
@@ -635,11 +636,11 @@ export class PlaceEnrichmentService {
|
||||
// becomes an href on the client, and the value comes from whatever the
|
||||
// configured index answered with. Anything but http(s) loses the link and
|
||||
// keeps the text, rather than being rendered as one.
|
||||
const rawUrl = typeof got?.sourceUrl === 'string' ? got.sourceUrl : null;
|
||||
const url = placeWebsiteSchema.safeParse(got?.sourceUrl);
|
||||
return {
|
||||
text,
|
||||
source: 'website',
|
||||
sourceUrl: rawUrl && placeWebsiteSchema.safeParse(rawUrl).success ? rawUrl : null,
|
||||
sourceUrl: url.success ? url.data : null,
|
||||
// Not a licensed corpus: a quoted summary from the operator's own page,
|
||||
// credited and linked back. Saying "CC-something" here would be a claim
|
||||
// about terms nobody granted.
|
||||
|
||||
@@ -79,9 +79,13 @@ function validateUrlFields(body: Record<string, unknown>): void {
|
||||
const website = body.website;
|
||||
// '' is how the UI clears the field; treat it like an absent value.
|
||||
if (website !== undefined && website !== null && website !== '') {
|
||||
if (typeof website !== 'string' || !placeWebsiteSchema.safeParse(website).success) {
|
||||
const parsed = placeWebsiteSchema.safeParse(website);
|
||||
if (!parsed.success) {
|
||||
throw new HttpException({ error: 'website must be an http or https URL' }, 400);
|
||||
}
|
||||
// The parsed form is the one stored: a bare host from a search result
|
||||
// arrives here without its https (#2483).
|
||||
body.website = parsed.data;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -30,8 +30,11 @@ function capUrls(input: Record<string, unknown>): void {
|
||||
throw new BadParams('invalid place: image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL');
|
||||
}
|
||||
const website = input.website;
|
||||
if (website !== undefined && website !== null && website !== '' && !placeWebsiteSchema.safeParse(website).success) {
|
||||
throw new BadParams('invalid place: website must be an http or https URL');
|
||||
if (website !== undefined && website !== null && website !== '') {
|
||||
const parsed = placeWebsiteSchema.safeParse(website);
|
||||
if (!parsed.success) throw new BadParams('invalid place: website must be an http or https URL');
|
||||
// Stored in its parsed form, like the REST route: a bare host gains https.
|
||||
input.website = parsed.data;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { normalizePlaceWebsite } from '@trek/shared';
|
||||
import { stripEmoji } from '../text-sanitize';
|
||||
|
||||
/**
|
||||
@@ -45,20 +46,6 @@ const capOrNull = (v: unknown, n: number): string | null => {
|
||||
return s === '' ? null : s;
|
||||
};
|
||||
|
||||
/**
|
||||
* A url that may become an `<a href>`. Anything but http/https is dropped — a
|
||||
* `javascript:` or `data:` url rendered as a link is click-XSS into the search list.
|
||||
*/
|
||||
function safeUrl(raw: unknown): string | null {
|
||||
if (typeof raw !== 'string' || raw === '') return null;
|
||||
try {
|
||||
const u = new URL(raw);
|
||||
return u.protocol === 'http:' || u.protocol === 'https:' ? raw.slice(0, 2048) : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A rating a plugin index carries, on the five-point scale every such index uses.
|
||||
*
|
||||
@@ -94,7 +81,10 @@ export function normalizeSearchHits(pluginId: string, raw: unknown): SearchHit[]
|
||||
lat,
|
||||
lng,
|
||||
rating: safeRating(h.rating),
|
||||
website: safeUrl(h.website),
|
||||
// Through the helper every other source uses (#2483): a `javascript:` or
|
||||
// `data:` url rendered as a link is click-XSS into the search list, and a
|
||||
// bare host gains https here as it does from the core search.
|
||||
website: normalizePlaceWebsite(h.website),
|
||||
phone: capOrNull(h.phone, 60),
|
||||
category: capOrNull(h.category, 60),
|
||||
description: capOrNull(h.description, 500),
|
||||
|
||||
@@ -139,6 +139,37 @@ describe('Collections e2e (real auth guard + real service + temp SQLite)', () =>
|
||||
expect(placed.reservation_status).toBe('none'); // itinerary defaults
|
||||
});
|
||||
|
||||
// #2483: a place from the TREK index can carry its website without a scheme,
|
||||
// and the save dialog has no website field to correct it in.
|
||||
it('COLLECTIONS-E2E-089: saving a place whose website has no scheme stores it as https', async () => {
|
||||
const col = (await request(server).post('/api/addons/collections').set('Cookie', sessionCookie(ownerId)).send({ name: 'Bretagne' })).body;
|
||||
const saved = await request(server).post('/api/addons/collections/places')
|
||||
.set('Cookie', sessionCookie(ownerId)).send({
|
||||
collection_id: col.id,
|
||||
name: 'Chapelle Sainte-Barbe',
|
||||
lat: 48.0286,
|
||||
lng: -3.4862,
|
||||
osm_id: 'gers:ceba0e62-172b-4343-bb3b-78b915a18383',
|
||||
website: 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët',
|
||||
});
|
||||
expect(saved.status).toBe(200);
|
||||
expect(saved.body.place.website).toBe('https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët');
|
||||
expect(db.prepare('SELECT website FROM collection_places WHERE id = ?').get(saved.body.place.id)).toEqual({
|
||||
website: 'https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët',
|
||||
});
|
||||
});
|
||||
|
||||
it('COLLECTIONS-E2E-090: a script link on save is still a 400 and stores nothing', async () => {
|
||||
const col = (await request(server).post('/api/addons/collections').set('Cookie', sessionCookie(ownerId)).send({ name: 'Script links' })).body;
|
||||
for (const website of ['javascript:alert(1)', 'mailto:mairie@example.fr', 'Chapelle']) {
|
||||
const res = await request(server).post('/api/addons/collections/places')
|
||||
.set('Cookie', sessionCookie(ownerId)).send({ collection_id: col.id, name: 'Chapelle', website });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/^website: /);
|
||||
}
|
||||
expect(db.prepare('SELECT COUNT(*) AS n FROM collection_places WHERE collection_id = ?').get(col.id)).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
// Regression for #1437: editing a place (PATCH without a status field) must NOT
|
||||
// reset a 'want'/'visited' place back to 'idea'.
|
||||
it('COLLECTIONS-E2E-012: PATCH without status leaves the saved status unchanged', async () => {
|
||||
|
||||
@@ -235,6 +235,36 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
expect(bad.body).toEqual({ error: 'route_color must be a hex colour like #4f46e5' });
|
||||
});
|
||||
|
||||
// #2483: a place from the TREK index can carry its website without a scheme.
|
||||
it('PLACES-E2E-2483-01: create and update take a website without a scheme and store it as https', async () => {
|
||||
const created = await request(server).post('/api/trips/5/places').set('Cookie', sessionCookie(1))
|
||||
.send({ name: 'Chapelle Sainte-Barbe', website: 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët' });
|
||||
expect(created.status).toBe(201);
|
||||
expect(created.body.place.website).toBe('https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët');
|
||||
|
||||
const id = created.body.place.id;
|
||||
const updated = await request(server).put(`/api/trips/5/places/${id}`).set('Cookie', sessionCookie(1))
|
||||
.send({ website: '//www.example.fr/patrimoine' });
|
||||
expect(updated.status).toBe(200);
|
||||
expect(db.prepare('SELECT website FROM places WHERE id = ?').get(id)).toEqual({ website: 'https://www.example.fr/patrimoine' });
|
||||
|
||||
// An explicit scheme is stored exactly as sent, and '' still clears the field.
|
||||
const kept = await request(server).put(`/api/trips/5/places/${id}`).set('Cookie', sessionCookie(1))
|
||||
.send({ website: 'http://Example.fr/Pfad?q=1' });
|
||||
expect(kept.body.place.website).toBe('http://Example.fr/Pfad?q=1');
|
||||
const cleared = await request(server).put(`/api/trips/5/places/${id}`).set('Cookie', sessionCookie(1)).send({ website: '' });
|
||||
expect(cleared.status).toBe(200);
|
||||
});
|
||||
|
||||
it('PLACES-E2E-2483-02: a script link, another scheme or a bare word is still a 400 with the same message', async () => {
|
||||
for (const website of ['javascript:alert(1)', 'mailto:mairie@example.fr', 'Chapelle', 42]) {
|
||||
const res = await request(server).post('/api/trips/5/places').set('Cookie', sessionCookie(1)).send({ name: 'Chapelle', website });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toEqual({ error: 'website must be an http or https URL' });
|
||||
}
|
||||
expect(db.prepare('SELECT COUNT(*) AS n FROM places WHERE trip_id = 5').get()).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
it('409 on a stale If-Match token (#1135)', async () => {
|
||||
db.prepare("INSERT INTO places (id, trip_id, name, updated_at) VALUES (9, 5, 'Walk', '2026-01-01 00:00:00')").run();
|
||||
const res = await request(server)
|
||||
|
||||
@@ -338,6 +338,27 @@ describe('Tool: save_place_to_collection', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// #2483: the tool spreads the REST save contract, so a bare host from
|
||||
// search_place is saved with https here too.
|
||||
describe('Tool: save_place_to_collection and a website without a scheme (#2483)', () => {
|
||||
it('MCP-COLL-2483-01: stores https for a bare host and still refuses a script link', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const col = seedCollection(user.id, 'Bretagne');
|
||||
const site = 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët';
|
||||
await withHarness(user.id, async (h) => {
|
||||
const saved = parseToolResult(await h.client.callTool({
|
||||
name: 'save_place_to_collection',
|
||||
arguments: { collection_id: col, name: 'Chapelle Sainte-Barbe', website: site },
|
||||
})) as { place: { website: string } };
|
||||
expect(saved.place.website).toBe(`https://${site}`);
|
||||
|
||||
const refused = await h.client.callTool({ name: 'save_place_to_collection', arguments: { collection_id: col, name: 'Hostile', website: 'javascript:alert(1)' } });
|
||||
expect(refused.isError).toBe(true);
|
||||
});
|
||||
expect(testDb.prepare('SELECT website FROM collection_places WHERE collection_id = ?').all(col)).toEqual([{ website: `https://${site}` }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Tool: save_trip_places_to_collection', () => {
|
||||
it('copies trip places, skipping duplicates', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
|
||||
@@ -335,6 +335,28 @@ describe('Tool: delete_accommodation', () => {
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Tool: create_place_accommodation', () => {
|
||||
// #2483: the place it creates takes its website through the same contract as
|
||||
// create_place, so a bare host from search_place lands as https.
|
||||
it('MCP-ACCOM-2483-01: a website without a scheme is stored with https, a script link is refused', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const data = parseToolResult(await h.client.callTool({
|
||||
name: 'create_place_accommodation',
|
||||
arguments: { tripId: trip.id, name: 'Hôtel du Faouët', start_day_id: day.id, end_day_id: day.id, website: 'www.hotel-faouet.example' },
|
||||
})) as { place: { id: number } };
|
||||
expect(testDb.prepare('SELECT website FROM places WHERE id = ?').get(data.place.id)).toEqual({ website: 'https://www.hotel-faouet.example' });
|
||||
|
||||
const refused = await h.client.callTool({
|
||||
name: 'create_place_accommodation',
|
||||
arguments: { tripId: trip.id, name: 'Hostile', start_day_id: day.id, end_day_id: day.id, website: 'javascript:alert(1)' },
|
||||
});
|
||||
expect(refused.isError).toBe(true);
|
||||
});
|
||||
expect(testDb.prepare("SELECT COUNT(*) AS n FROM places WHERE trip_id = ? AND name = 'Hostile'").get(trip.id)).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
it('creates the place and the accommodation atomically and broadcasts both', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
@@ -1271,3 +1271,62 @@ describe('journey hooks on the MCP delete paths', () => {
|
||||
expect(skeletonFor(journey.id, foreign.id)).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// A website without a scheme (#2483): the tools parse it the way REST does
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('place tools and a website without a scheme (#2483)', () => {
|
||||
const SITE = 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët';
|
||||
const websiteOf = (placeId: number) =>
|
||||
(testDb.prepare('SELECT website FROM places WHERE id = ?').get(placeId) as { website: string | null }).website;
|
||||
|
||||
it('MCP-PLACES-2483-01: create_place, create_and_assign_place, update_place and bulk_update_places store https', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
|
||||
await withHarness(user.id, async (h) => {
|
||||
const created = parseToolResult(await h.client.callTool({ name: 'create_place', arguments: { tripId: trip.id, name: 'Chapelle', website: SITE } })) as { place: { id: number } };
|
||||
expect(websiteOf(created.place.id)).toBe(`https://${SITE}`);
|
||||
|
||||
const assigned = parseToolResult(await h.client.callTool({
|
||||
name: 'create_and_assign_place',
|
||||
arguments: { tripId: trip.id, dayId: day.id, name: 'Halles', website: 'www.example.fr/halles' },
|
||||
})) as { place: { id: number } };
|
||||
expect(websiteOf(assigned.place.id)).toBe('https://www.example.fr/halles');
|
||||
|
||||
await h.client.callTool({ name: 'update_place', arguments: { tripId: trip.id, placeId: created.place.id, website: '//www.example.fr' } });
|
||||
expect(websiteOf(created.place.id)).toBe('https://www.example.fr');
|
||||
|
||||
await h.client.callTool({ name: 'bulk_update_places', arguments: { tripId: trip.id, placeIds: [created.place.id, assigned.place.id], website: 'example.fr:8080/x' } });
|
||||
expect([websiteOf(created.place.id), websiteOf(assigned.place.id)]).toEqual(['https://example.fr:8080/x', 'https://example.fr:8080/x']);
|
||||
});
|
||||
});
|
||||
|
||||
it('MCP-PLACES-2483-02: a script link or another scheme is still refused and stores nothing', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
await withHarness(user.id, async (h) => {
|
||||
for (const website of ['javascript:alert(1)', 'mailto:mairie@example.fr', 'Chapelle']) {
|
||||
const result = await h.client.callTool({ name: 'create_place', arguments: { tripId: trip.id, name: 'Hostile', website } });
|
||||
expect(result.isError, website).toBe(true);
|
||||
expect((result.content as { text: string }[])[0].text).toMatch(/Invalid arguments/);
|
||||
}
|
||||
});
|
||||
|
||||
expect(testDb.prepare('SELECT COUNT(*) AS n FROM places WHERE trip_id = ?').get(trip.id)).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
it('MCP-PLACES-2483-03: tools/list still describes the field as a plain capped string', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const { tools } = await h.client.listTools();
|
||||
for (const name of ['create_place', 'create_and_assign_place', 'update_place', 'bulk_update_places']) {
|
||||
const tool = tools.find((t) => t.name === name);
|
||||
expect((tool?.inputSchema.properties as Record<string, unknown>).website, name).toEqual({ type: 'string', maxLength: 500 });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -187,3 +187,32 @@ describe('BookingImportService.preview endpoint geocoding (#1969)', () => {
|
||||
expect(maps.geocodeQuery).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// #2483: a venue's website from a booking mail is one more outside source, and it
|
||||
// used to reach the place row exactly as the mail or the review form had it.
|
||||
describe('BookingImportService.confirm venue website (#2483)', () => {
|
||||
it('BOOKING-IMPORT-2483-01: a bare host is saved with https, a script link or nothing not at all', async () => {
|
||||
const reservations = { create: vi.fn(() => ({ reservation: { id: 1 }, accommodationCreated: false })) };
|
||||
const places = { create: vi.fn((_tripId: string, _input: { website?: string }) => ({ id: 7 })) };
|
||||
const svc = new BookingImportService(
|
||||
{} as never, {} as never, new DatabaseService(dbConn), reservations as never, permissionsStub as never,
|
||||
undefined as never, { isAddonEnabled: () => false } as never, { broadcast: vi.fn() } as never,
|
||||
{ geocodeQuery: vi.fn() } as never, places as never,
|
||||
);
|
||||
const item = (website?: string) => ({
|
||||
type: 'restaurant',
|
||||
title: 'Dîner',
|
||||
_venue: { name: 'Crêperie', lat: 48.03, lng: -3.49, website },
|
||||
source: { fileName: 'booking.eml', index: 0 },
|
||||
});
|
||||
|
||||
const res = await svc.confirm('5', [item('www.creperie.example/carte'), item('javascript:alert(1)'), item()], undefined);
|
||||
|
||||
expect(res.created).toHaveLength(3);
|
||||
expect(places.create.mock.calls.map(([, input]) => input.website)).toEqual([
|
||||
'https://www.creperie.example/carte',
|
||||
undefined,
|
||||
undefined,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -230,6 +230,23 @@ describe('AmapPlacesProvider.searchText', () => {
|
||||
expect(place.website).toBeNull();
|
||||
});
|
||||
|
||||
// #2483: a site in Amap is free text, and often a bare host.
|
||||
it('AMAP-012b: a website without a scheme gains https, one that is no website becomes null', async () => {
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue(
|
||||
ok({
|
||||
pois: [
|
||||
{ id: 'B3', name: '故宫博物院', website: 'www.dpm.org.cn', location: TIANANMEN_LOCATION },
|
||||
{ id: 'B4', name: '某小店', website: 'javascript:alert(1)', location: TIANANMEN_LOCATION },
|
||||
],
|
||||
}),
|
||||
),
|
||||
);
|
||||
const places = await provider().searchText('故宫');
|
||||
expect(places.map((p) => p.website)).toEqual(['https://www.dpm.org.cn', null]);
|
||||
});
|
||||
|
||||
it('AMAP-013: uses place/around with a bias, because text search cannot be biased', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue(ok({ pois: [] })));
|
||||
await provider().searchText('咖啡', 'zh', { lat: 31.2304, lng: 121.4737, radius: 3000 });
|
||||
|
||||
@@ -120,6 +120,19 @@ describe('MapsService.getPlaceDetails for a gers: id', () => {
|
||||
expect(out.place?.opening_hours).toBeTruthy();
|
||||
});
|
||||
|
||||
// #2483: the place from the issue, whose index record carries its website
|
||||
// without a scheme. Both halves of the merge hand it over completed.
|
||||
it('MAPS-GERS-010: the merged website has its scheme, from the index or else from OSM', async () => {
|
||||
const site = 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët';
|
||||
mockById.mockResolvedValue({ ...PLACE, gers: 'ceba0e62-172b-4343-bb3b-78b915a18383', contact: { ...PLACE.contact, website: site } });
|
||||
expect((await make(null).getPlaceDetails(1, 'gers:ceba0e62-172b-4343-bb3b-78b915a18383')).place?.website).toBe(`https://${site}`);
|
||||
expect((await make({ website: 'www.example.fr' }).getPlaceDetails(1, 'gers:ceba0e62-172b-4343-bb3b-78b915a18383')).place?.website).toBe(`https://${site}`);
|
||||
|
||||
mockById.mockResolvedValue({ ...PLACE, contact: { ...PLACE.contact, website: 'javascript:alert(1)' } });
|
||||
expect((await make({ website: 'www.example.fr' }).getPlaceDetails(1, 'gers:abc-123')).place?.website).toBe('https://www.example.fr');
|
||||
expect((await make(null).getPlaceDetails(1, 'gers:abc-123')).place?.website).toBeNull();
|
||||
});
|
||||
|
||||
it('MAPS-GERS-009: with the index switched off, opening a saved place asks nobody', async () => {
|
||||
// TREK_PLACES_ENABLED=false is the operator's word that nothing leaves for
|
||||
// the index. Search, autocomplete and the area download honoured it; the
|
||||
|
||||
@@ -236,6 +236,20 @@ describe('MapsService.pois answered from the index', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// #2483: the index keeps websites the way the operator typed them.
|
||||
it('MAPS-POIS-015: a website without a scheme gains https, one that is no website becomes null', async () => {
|
||||
mockNearby.mockResolvedValue([
|
||||
{ ...FULL, contact: { ...FULL.contact, website: 'cafecentral.wien/de' } },
|
||||
{ ...FULL, gers: 'a-2', contact: { ...FULL.contact, website: 'javascript:alert(1)' } },
|
||||
]);
|
||||
const svc = make();
|
||||
stubOverpass(svc);
|
||||
|
||||
const out = await svc.pois('cafe', BOX);
|
||||
|
||||
expect(out.pois.map((p) => p.website)).toEqual(['https://cafecentral.wien/de', null]);
|
||||
});
|
||||
|
||||
it('MAPS-POIS-004: asks the index for the categories behind the pill, around the viewport centre', async () => {
|
||||
mockNearby.mockResolvedValue([FULL]);
|
||||
const svc = make();
|
||||
|
||||
@@ -3516,3 +3516,75 @@ describe('brandLogo', () => {
|
||||
expect(await service().brandLogo('Q3')).toBeNull();
|
||||
});
|
||||
})
|
||||
|
||||
// ── Websites without a scheme (#2483) ─────────────────────────────────────────
|
||||
//
|
||||
// Every source hands its website through normalizePlaceWebsite, so a place that
|
||||
// is picked from any of them saves without a 400 over a field nobody sees.
|
||||
|
||||
describe('websites from the map sources (#2483)', () => {
|
||||
it('MAPS-2483-01: OSM details complete a bare host and skip a contact:website that is no website', () => {
|
||||
expect(buildOsmDetails({ website: 'www.example.fr/patrimoine' }, 'way', '1').website).toBe(
|
||||
'https://www.example.fr/patrimoine',
|
||||
);
|
||||
expect(buildOsmDetails({ 'contact:website': 'mailto:mairie@example.fr', website: 'example.fr' }, 'node', '1').website).toBe(
|
||||
'https://example.fr',
|
||||
);
|
||||
expect(buildOsmDetails({ website: 'javascript:alert(1)' }, 'node', '1').website).toBeNull();
|
||||
});
|
||||
|
||||
it('MAPS-2483-02: an Overpass POI gets https on a bare host and falls back past a script link', async () => {
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
elements: [
|
||||
{ type: 'node', id: 1, lat: 48.03, lon: -3.49, tags: { name: 'Chapelle', tourism: 'attraction', website: 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët' } },
|
||||
{ type: 'node', id: 2, lat: 48.04, lon: -3.49, tags: { name: 'Mairie', tourism: 'attraction', website: 'javascript:alert(1)', 'contact:website': '//www.example.fr' } },
|
||||
{ type: 'node', id: 3, lat: 48.05, lon: -3.49, tags: { name: 'Halles', tourism: 'attraction', website: 'Halles' } },
|
||||
],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const { pois } = await svc.searchOverpassPois('sights', { south: 48.0, west: -3.6, north: 48.1, east: -3.4 }, 'fr-FR');
|
||||
expect(pois.map((p) => p.website)).toEqual([
|
||||
'https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët',
|
||||
'https://www.example.fr',
|
||||
null,
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-2483-03: a Google search result goes through the same helper', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'ENCRYPTED' }).mockReturnValueOnce(null);
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
places: [{ id: 'gid-web', displayName: { text: 'Chapelle' }, location: { latitude: 48, longitude: -3 }, websiteUri: 'www.example.fr' }],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const search = await svc.searchPlaces(1, 'Chapelle');
|
||||
expect((search.places[0] as { website: unknown }).website).toBe('https://www.example.fr');
|
||||
});
|
||||
|
||||
const googleDetails = (websiteUri: string) =>
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => ({ id: 'ChIJWeb', websiteUri }) }));
|
||||
|
||||
it('MAPS-2483-04: so does the website of a Google details lookup', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
googleDetails('example.fr/visite');
|
||||
const place = (await svc.getPlaceDetails(1, 'ChIJWeb-lean')).place as { website: unknown };
|
||||
expect(place.website).toBe('https://example.fr/visite');
|
||||
});
|
||||
|
||||
it('MAPS-2483-05: and of the expanded one, where a value that is no page becomes null', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
mockDbGet.mockReturnValueOnce(undefined);
|
||||
googleDetails('data:text/html,x');
|
||||
const place = (await svc.getPlaceDetailsExpanded(1, 'ChIJWeb-expanded')).place as { website: unknown };
|
||||
expect(place.website).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -198,7 +198,7 @@ describe('description source order', () => {
|
||||
// The record and its link come with the request, and the answer is cached
|
||||
// for every user of the instance under the label "Google". A link that is
|
||||
// not a Google Maps address by shape is dropped; the text stays.
|
||||
for (const bad of ['https://phish.example/maps', 'https://google.evil.example/x', 'javascript:alert(1)', 'maps.google.com/x']) {
|
||||
for (const bad of ['https://phish.example/maps', 'https://google.evil.example/x', 'javascript:alert(1)', 'phish.example/maps']) {
|
||||
const maps = mapsStub({
|
||||
getMapsKey: vi.fn(() => 'key'),
|
||||
fetchEditorialSummary: vi.fn(async () => 'Casual chain for wood-fired pizza.'),
|
||||
@@ -221,6 +221,15 @@ describe('description source order', () => {
|
||||
});
|
||||
expect(out.description, ok).toMatchObject({ source: 'google', sourceUrl: ok });
|
||||
}
|
||||
|
||||
// #2483: the website contract completes a bare host, and the link is the
|
||||
// completed one. The host check still decides, on the host it names.
|
||||
const maps = mapsStub({
|
||||
getMapsKey: vi.fn(() => 'key'),
|
||||
fetchEditorialSummary: vi.fn(async () => 'Casual chain for wood-fired pizza.'),
|
||||
});
|
||||
const bare = await make(maps).enrich(1, { ...GOOGLE_REQ, details: { source: 'google', google_maps_url: 'maps.google.com/x' } });
|
||||
expect(bare.description).toMatchObject({ source: 'google', sourceUrl: 'https://maps.google.com/x' });
|
||||
});
|
||||
|
||||
it('ENRICH-075: keeps the OpenStreetMap description ahead of everything', async () => {
|
||||
@@ -450,6 +459,14 @@ describe("the description on the place's own website", () => {
|
||||
});
|
||||
const ok = await make(mapsStub()).enrich(1, GERS_REQ);
|
||||
expect(ok.description).toMatchObject({ sourceUrl: 'https://losteria.net/rostock' });
|
||||
|
||||
// #2483: the index keeps a page the way it found it, sometimes without a
|
||||
// scheme. The link that is stored is the completed one, never the bare text.
|
||||
mockTrekPlacesById.mockResolvedValue({
|
||||
description: { text: 'Pizza in Rostock.', sourceUrl: 'losteria.net/rostock' },
|
||||
});
|
||||
const bare = await make(mapsStub()).enrich(1, GERS_REQ);
|
||||
expect(bare.description).toMatchObject({ sourceUrl: 'https://losteria.net/rostock' });
|
||||
});
|
||||
|
||||
it('ENRICH-117: a description the request made up never reaches the shared cache', async () => {
|
||||
|
||||
@@ -757,6 +757,14 @@ describe('collectFacts', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
// #2483: an OSM menu tag is written the way people type a website. The link
|
||||
// the client gets is the completed one, never the bare text.
|
||||
it('ENRICH-052e: completes a menu_url without a scheme to https', () => {
|
||||
expect(collectFacts(osm({ menu_url: 'www.example.org/speisekarte' }))).toEqual([
|
||||
{ kind: 'menu', value: null, url: 'https://www.example.org/speisekarte' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('ENRICH-053: takes no chips at all from a Google place', () => {
|
||||
expect(collectFacts(null)).toEqual([]);
|
||||
// cuisine/outdoor_seating are OSM tagging and never appear on a Google blob;
|
||||
|
||||
@@ -410,7 +410,9 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou
|
||||
|
||||
it('400s a website that window.open would not treat as a page', async () => {
|
||||
const canEdit = vi.fn().mockReturnValue(false);
|
||||
for (const website of ['javascript:fetch("/api/trips")', 'data:text/html,x', 'louvre.fr', 42]) {
|
||||
// 'louvre' rather than 'louvre.fr': a bare host is completed since #2483,
|
||||
// a single word is still no address.
|
||||
for (const website of ['javascript:fetch("/api/trips")', 'data:text/html,x', 'mailto:info@louvre.fr', 'louvre', 42]) {
|
||||
expect(await thrownAsync(() => ctl({ canEdit }).update(user, '5', '9', { website }))).toEqual(siteErr);
|
||||
}
|
||||
expect(canEdit).not.toHaveBeenCalled();
|
||||
@@ -420,8 +422,23 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou
|
||||
const update = vi.fn().mockReturnValue({ id: 9 });
|
||||
for (const website of ['https://louvre.fr', 'http://pension.at', '', null]) {
|
||||
expect(await ctl({ update } as Partial<PlacesService>).update(user, '5', '9', { website })).toEqual({ place: { id: 9 } });
|
||||
expect(update).toHaveBeenLastCalledWith('5', '9', { website }, undefined);
|
||||
}
|
||||
});
|
||||
|
||||
// #2483: the value that reaches the service is the parsed one, on both write
|
||||
// routes. The MCP tools parse the same schema (tools-places.test.ts), so an
|
||||
// agent and the web app store the same string for the same input.
|
||||
it('PLACES-CTRL-2483-01: hands the service a bare host with https, on create and on update', async () => {
|
||||
const site = 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët';
|
||||
const create = vi.fn().mockReturnValue({ id: 9 });
|
||||
ctl({ create } as Partial<PlacesService>).create(user, '5', { name: 'Chapelle', website: site });
|
||||
expect(create).toHaveBeenCalledWith('5', { name: 'Chapelle', website: `https://${site}` });
|
||||
|
||||
const update = vi.fn().mockReturnValue({ id: 9 });
|
||||
await ctl({ update } as Partial<PlacesService>).update(user, '5', '9', { website: '//www.example.fr' });
|
||||
expect(update).toHaveBeenCalledWith('5', '9', { website: 'https://www.example.fr' }, undefined);
|
||||
});
|
||||
});
|
||||
|
||||
it('PUT /:id forwards the base-version token and 409s on a conflict (#1135)', async () => {
|
||||
|
||||
@@ -602,6 +602,16 @@ describe('toPlaceRecord', () => {
|
||||
expect(r.lat).toBeNull();
|
||||
});
|
||||
|
||||
// #2483: Overture keeps a website the way the operator typed it.
|
||||
it('TREK-PLACES-2483-01: a website without a scheme gains https, one that is no website becomes null', () => {
|
||||
const withSite = (website: string | null) => toPlaceRecord({ ...PLACE, contact: { ...PLACE.contact, website } }).website;
|
||||
expect(withSite('fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët')).toBe(
|
||||
'https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët',
|
||||
);
|
||||
expect(withSite('javascript:alert(1)')).toBeNull();
|
||||
expect(withSite(null)).toBeNull();
|
||||
});
|
||||
|
||||
it('builds an address from whatever parts exist', () => {
|
||||
const r = toPlaceRecord({
|
||||
...PLACE,
|
||||
|
||||
@@ -176,4 +176,22 @@ describe('PlacesRpc', () => {
|
||||
expect(res.error.message).toBe('no place 7 on trip 1');
|
||||
expect(f.realtime.broadcast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #2483: the same parse the REST controller and the MCP tools apply, so a
|
||||
// plugin that hands over a bare host stores the https form, not the text.
|
||||
it('PLACES-RPC-2483-01 a website without a scheme is stored with https, a script link is BAD_PARAMS', async () => {
|
||||
const f = build();
|
||||
const host = f.host('db:write:places');
|
||||
const site = 'fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët';
|
||||
expect((await host.dispatch(req('places.create', { tripId: 1, input: { name: 'Chapelle', website: site } }), 42)).ok).toBe(true);
|
||||
expect(f.places.create).toHaveBeenCalledWith('1', { name: 'Chapelle', website: `https://${site}` });
|
||||
|
||||
expect((await host.dispatch(req('places.update', { tripId: 1, placeId: 7, input: { website: '//www.example.fr' } }), 42)).ok).toBe(true);
|
||||
expect(f.places.update).toHaveBeenCalledWith('1', '7', { website: 'https://www.example.fr' });
|
||||
|
||||
const refused = (await host.dispatch(req('places.create', { tripId: 1, input: { name: 'X', website: 'javascript:alert(1)' } }), 42)) as RpcError;
|
||||
expect(refused.error.code).toBe('BAD_PARAMS');
|
||||
expect(refused.error.message).toBe('invalid place: website must be an http or https URL');
|
||||
expect(f.places.create).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -152,6 +152,19 @@ describe('PluginSearchController', () => {
|
||||
expect(places[2].rating).toBeNull();
|
||||
});
|
||||
|
||||
// #2483: a plugin index is one more source of websites typed without a scheme.
|
||||
it('PLUGIN-SEARCH-2483-01: a website without a scheme gains https like one from the core search', async () => {
|
||||
const { c } = controller({
|
||||
searchPlaces: vi.fn(async () => [
|
||||
hit({ id: 'a', website: 'www.ristorante.example/menu' }),
|
||||
hit({ id: 'b', website: '//ristorante.example' }),
|
||||
hit({ id: 'c', website: 'mailto:info@ristorante.example' }),
|
||||
]) as unknown as PluginHooks['searchPlaces'],
|
||||
});
|
||||
const { places } = await c.search('milan', undefined, undefined, undefined, undefined, req(5));
|
||||
expect(places.map((p) => p.website)).toEqual(['https://www.ristorante.example/menu', 'https://ristorante.example', null]);
|
||||
});
|
||||
|
||||
it('caps one provider at 20 hits', async () => {
|
||||
const { c } = controller({
|
||||
searchPlaces: vi.fn(async () =>
|
||||
|
||||
@@ -42,3 +42,17 @@ describe('collectionGpxReadRequestSchema (#2301)', () => {
|
||||
expect(accepts({})).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('collectionFilePlaceSchema website (#2483)', () => {
|
||||
// The importer stores the parsed place, so the completed form is the one that
|
||||
// lands; a link that is no page is still dropped rather than costing the place.
|
||||
it('SHARED-COLLFILE-2483-01: a bare host arrives with https, a script link as null', () => {
|
||||
const place = (website: unknown) => collectionFilePlaceSchema.parse({ name: 'Chapelle', website });
|
||||
expect(place('fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët').website).toBe(
|
||||
'https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët',
|
||||
);
|
||||
expect(place('https://example.fr').website).toBe('https://example.fr');
|
||||
expect(place('javascript:alert(1)').website).toBeNull();
|
||||
expect(place('Chapelle').website).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -42,6 +42,7 @@ export * from './day/stay-bookends';
|
||||
export * from './assignment/assignment.schema';
|
||||
export * from './place/place.schema';
|
||||
export * from './place/place-match';
|
||||
export * from './place/place-website';
|
||||
export * from './roadtrip/roadtrip.schema';
|
||||
export * from './place/track-colors';
|
||||
export * from './collection/collection.schema';
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
import { PLACE_WEBSITE_MAX_LENGTH, normalizePlaceWebsite } from './place-website';
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
describe('normalizePlaceWebsite (#2483)', () => {
|
||||
it('SHARED-WEBSITE-001: the value from the issue gains https and keeps its accented path as written', () => {
|
||||
expect(normalizePlaceWebsite('fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët')).toBe(
|
||||
'https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët',
|
||||
);
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-002: http and https stay exactly as they came, case included', () => {
|
||||
expect(normalizePlaceWebsite('https://louvre.fr/en/visit')).toBe('https://louvre.fr/en/visit');
|
||||
expect(normalizePlaceWebsite('http://pension-alpenblick.at')).toBe('http://pension-alpenblick.at');
|
||||
expect(normalizePlaceWebsite('HTTPS://Example.COM/Pfad?q=1#top')).toBe('HTTPS://Example.COM/Pfad?q=1#top');
|
||||
expect(normalizePlaceWebsite('https://user@example.com/')).toBe('https://user@example.com/');
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-003: surrounding whitespace is trimmed, empty and blank are nothing', () => {
|
||||
expect(normalizePlaceWebsite(' www.example.com \n')).toBe('https://www.example.com');
|
||||
expect(normalizePlaceWebsite(' https://example.com ')).toBe('https://example.com');
|
||||
expect(normalizePlaceWebsite('')).toBeNull();
|
||||
expect(normalizePlaceWebsite(' ')).toBeNull();
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-004: a host with a port is a host, not a scheme', () => {
|
||||
expect(normalizePlaceWebsite('example.com:8080/x')).toBe('https://example.com:8080/x');
|
||||
expect(normalizePlaceWebsite('www.example.com:443')).toBe('https://www.example.com:443');
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-005: a protocol-relative address gains https:', () => {
|
||||
expect(normalizePlaceWebsite('//www.example.fr/patrimoine')).toBe('https://www.example.fr/patrimoine');
|
||||
expect(normalizePlaceWebsite('//')).toBeNull();
|
||||
expect(normalizePlaceWebsite('//Chapelle')).toBeNull();
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-006: every other scheme is dropped, a script one in any spelling included', () => {
|
||||
for (const value of [
|
||||
'javascript:alert(1)',
|
||||
'JavaScript:alert(1)',
|
||||
' javascript:alert(1)',
|
||||
'java\tscript:alert(1)',
|
||||
'javascript:80',
|
||||
'data:text/html,<script>alert(1)</script>',
|
||||
'vbscript:msgbox(1)',
|
||||
'mailto:mairie@example.fr',
|
||||
'tel:+33297230000',
|
||||
'ftp://example.com/file',
|
||||
'file:///etc/passwd',
|
||||
'http:example.com',
|
||||
]) {
|
||||
expect(normalizePlaceWebsite(value), value).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-007: text that is no address is dropped rather than linked', () => {
|
||||
for (const value of [
|
||||
'Chapelle',
|
||||
'localhost',
|
||||
'localhost:3000',
|
||||
'3.5',
|
||||
'call the hotel',
|
||||
'exa mple.com',
|
||||
'mairie@example.fr',
|
||||
'-example.com',
|
||||
'example-.com',
|
||||
'example.com.',
|
||||
'https://',
|
||||
'https://localhost:3000',
|
||||
'https://exa mple.com',
|
||||
]) {
|
||||
expect(normalizePlaceWebsite(value), value).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-008: an IPv4 address is a host, a number that only looks like one is not', () => {
|
||||
expect(normalizePlaceWebsite('192.168.1.10/admin')).toBe('https://192.168.1.10/admin');
|
||||
expect(normalizePlaceWebsite('http://203.0.113.7')).toBe('http://203.0.113.7');
|
||||
expect(normalizePlaceWebsite('256.1.1.1')).toBeNull();
|
||||
expect(normalizePlaceWebsite('1.2.3')).toBeNull();
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-009: an internationalised host keeps its letters', () => {
|
||||
expect(normalizePlaceWebsite('münchen.de/rathaus')).toBe('https://münchen.de/rathaus');
|
||||
expect(normalizePlaceWebsite('www.北京.cn')).toBe('https://www.北京.cn');
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-010: a query or fragment right after the host still counts as a host', () => {
|
||||
expect(normalizePlaceWebsite('example.com?lang=de')).toBe('https://example.com?lang=de');
|
||||
expect(normalizePlaceWebsite('example.com#kontakt')).toBe('https://example.com#kontakt');
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-011: a value the contract could not store is dropped rather than cut', () => {
|
||||
const path = 'a'.repeat(PLACE_WEBSITE_MAX_LENGTH);
|
||||
expect(normalizePlaceWebsite(`example.com/${path}`)).toBeNull();
|
||||
const fits = `example.com/${'a'.repeat(PLACE_WEBSITE_MAX_LENGTH - 'https://example.com/'.length)}`;
|
||||
expect(normalizePlaceWebsite(fits)).toHaveLength(PLACE_WEBSITE_MAX_LENGTH);
|
||||
});
|
||||
|
||||
it('SHARED-WEBSITE-012: anything but a string is nothing', () => {
|
||||
expect(normalizePlaceWebsite(null)).toBeNull();
|
||||
expect(normalizePlaceWebsite(undefined)).toBeNull();
|
||||
expect(normalizePlaceWebsite(42)).toBeNull();
|
||||
expect(normalizePlaceWebsite({ href: 'https://example.com' })).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,84 @@
|
||||
import { z } from 'zod';
|
||||
|
||||
/**
|
||||
* A place's homepage as a source hands it over, made into an address a browser
|
||||
* opens as a page, or nothing.
|
||||
*
|
||||
* The place index, OpenStreetMap and Amap carry websites the way people type
|
||||
* them into a form: `fr.wikipedia.org/wiki/…`, `www.hotel.cn`, `//example.com`.
|
||||
* The write contract wants http(s), so a place picked from search used to be
|
||||
* refused on save over a field the dialog does not even show (#2483). Every
|
||||
* path that brings a website in from outside goes through here, and a value
|
||||
* either arrives with its scheme or does not arrive at all.
|
||||
*
|
||||
* Only the scheme is added. The rest of the text stays as it came, so an
|
||||
* accented path is not percent-encoded on the way through.
|
||||
*/
|
||||
|
||||
/** The cap the write contract puts on the field; a longer value could not be saved. */
|
||||
export const PLACE_WEBSITE_MAX_LENGTH = 500;
|
||||
|
||||
const HTTP_URL = /^https?:\/\//i;
|
||||
// A host and an optional port. The port is what tells `example.com:8080/x`
|
||||
// apart from a scheme such as `mailto:` or `javascript:`, which never match.
|
||||
const HOST_AND_PORT = /^([^:/?#@\s]+)(?::\d{1,5})?$/;
|
||||
const LABEL = /^[\p{L}\p{N}\p{M}_-]+$/u;
|
||||
const OCTET = /^\d{1,3}$/;
|
||||
const LETTER = /\p{L}/u;
|
||||
const openable = z.url({ protocol: /^https?$/ });
|
||||
|
||||
/** Everything up to the path, the query or the fragment. */
|
||||
function authorityOf(rest: string): string {
|
||||
const end = rest.search(/[/?#]/);
|
||||
return end === -1 ? rest : rest.slice(0, end);
|
||||
}
|
||||
|
||||
/**
|
||||
* A name with at least one dot and a top-level label that has a letter in it,
|
||||
* or an IPv4 address. "Chapelle", "localhost" and "3.5" are not websites.
|
||||
*/
|
||||
function isHostName(host: string): boolean {
|
||||
const labels = host.split('.');
|
||||
if (labels.length < 2) return false;
|
||||
if (labels.every((label) => OCTET.test(label))) {
|
||||
return labels.length === 4 && labels.every((label) => Number(label) <= 255);
|
||||
}
|
||||
const valid = labels.every((label) => LABEL.test(label) && !label.startsWith('-') && !label.endsWith('-'));
|
||||
return valid && LETTER.test(labels[labels.length - 1] ?? '');
|
||||
}
|
||||
|
||||
function hasHost(authority: string): boolean {
|
||||
const host = HOST_AND_PORT.exec(authority)?.[1];
|
||||
return host !== undefined && isHostName(host);
|
||||
}
|
||||
|
||||
/**
|
||||
* The website a place gets from `value`, or null.
|
||||
*
|
||||
* Trimmed; http and https stay as they are; a bare host or a protocol-relative
|
||||
* `//host` gains https. Any other scheme (`javascript:`, `data:`, `mailto:`,
|
||||
* `ftp:`), free text, a host without a dot and anything a browser cannot
|
||||
* parse come back as null, as does a value longer than the contract allows.
|
||||
*/
|
||||
export function normalizePlaceWebsite(value: unknown): string | null {
|
||||
if (typeof value !== 'string') return null;
|
||||
const text = value.trim();
|
||||
if (!text) return null;
|
||||
|
||||
let url: string;
|
||||
if (HTTP_URL.test(text)) {
|
||||
const authority = authorityOf(text.replace(HTTP_URL, ''));
|
||||
// Credentials in front of the host are not part of the name being checked.
|
||||
if (!hasHost(authority.slice(authority.lastIndexOf('@') + 1))) return null;
|
||||
url = text;
|
||||
} else if (text.startsWith('//')) {
|
||||
if (!hasHost(authorityOf(text.slice(2)))) return null;
|
||||
url = `https:${text}`;
|
||||
} else {
|
||||
if (!hasHost(authorityOf(text))) return null;
|
||||
url = `https://${text}`;
|
||||
}
|
||||
|
||||
if (url.length > PLACE_WEBSITE_MAX_LENGTH) return null;
|
||||
return openable.safeParse(url).success ? url : null;
|
||||
}
|
||||
@@ -4,9 +4,11 @@ import {
|
||||
placeImportListRequestSchema,
|
||||
placeSchema,
|
||||
placeUpdateRequestSchema,
|
||||
placeWebsiteSchema,
|
||||
} from './place.schema';
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { z } from 'zod';
|
||||
|
||||
describe('placeSchema route_color (#776)', () => {
|
||||
const place = { id: 1, trip_id: 1, name: 'Walk' };
|
||||
@@ -77,3 +79,52 @@ describe('stop_type on the write routes', () => {
|
||||
expect(placeCreateRequestSchema.safeParse(body).success).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('placeWebsiteSchema (#2483)', () => {
|
||||
it('SHARED-PLACE-030: a bare host from a search result parses to its https form', () => {
|
||||
const parsed = placeWebsiteSchema.safeParse('fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët');
|
||||
expect(parsed.success && parsed.data).toBe('https://fr.wikipedia.org/wiki/Chapelle_Sainte-Barbe_du_Faouët');
|
||||
const relative = placeWebsiteSchema.safeParse('//www.example.fr');
|
||||
expect(relative.success && relative.data).toBe('https://www.example.fr');
|
||||
});
|
||||
|
||||
it('SHARED-PLACE-031: a value that names http(s) comes back exactly as sent', () => {
|
||||
for (const value of [
|
||||
'https://louvre.fr/en/visit',
|
||||
'http://pension-alpenblick.at',
|
||||
'https://x.example ',
|
||||
'http://localhost:3000',
|
||||
]) {
|
||||
const parsed = placeWebsiteSchema.safeParse(value);
|
||||
expect(parsed.success && parsed.data, value).toBe(value);
|
||||
}
|
||||
});
|
||||
|
||||
it('SHARED-PLACE-032: every other scheme and free text still fail with the same message', () => {
|
||||
for (const value of [
|
||||
'javascript:alert(1)',
|
||||
'data:text/html,x',
|
||||
'mailto:a@example.fr',
|
||||
'Chapelle',
|
||||
'',
|
||||
'localhost',
|
||||
]) {
|
||||
const parsed = placeWebsiteSchema.safeParse(value);
|
||||
expect(parsed.success, value).toBe(false);
|
||||
expect(parsed.error?.issues.map((i) => i.message)).toContain('must be an http or https URL');
|
||||
}
|
||||
});
|
||||
|
||||
it('SHARED-PLACE-033: the length cap applies to what is stored', () => {
|
||||
expect(placeWebsiteSchema.safeParse(`https://example.com/${'a'.repeat(480)}`).success).toBe(true);
|
||||
expect(placeWebsiteSchema.safeParse(`https://example.com/${'a'.repeat(481)}`).success).toBe(false);
|
||||
// Measured after the scheme is added, which is the value that is stored.
|
||||
expect(placeWebsiteSchema.safeParse(`example.com/${'a'.repeat(480)}`).success).toBe(true);
|
||||
expect(placeWebsiteSchema.safeParse(`example.com/${'a'.repeat(481)}`).success).toBe(false);
|
||||
});
|
||||
|
||||
it('SHARED-PLACE-034: the tool schema an MCP client reads is still a plain capped string', () => {
|
||||
const shape = z.toJSONSchema(z.object({ website: placeWebsiteSchema.optional() }), { io: 'input' });
|
||||
expect(shape.properties?.website).toEqual({ type: 'string', maxLength: 500 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { tagSchema } from '../tag/tag.schema';
|
||||
import { PLACE_WEBSITE_MAX_LENGTH, normalizePlaceWebsite } from './place-website';
|
||||
|
||||
import { z } from 'zod';
|
||||
|
||||
@@ -41,14 +42,23 @@ export const placeImageUrlSchema = z
|
||||
{ message: 'must be an uploaded path, a photo-proxy path, an inline image or an https URL' },
|
||||
);
|
||||
|
||||
const HTTP_URL = /^https?:\/\//i;
|
||||
|
||||
/**
|
||||
* A place's homepage. It reaches window.open() on the client, where a
|
||||
* javascript: value would run in this origin rather than opening a page.
|
||||
*
|
||||
* A bare host is completed to https before the check instead of refused
|
||||
* (#2483): search results carry websites that way, and an older client, a
|
||||
* cached offline result or an MCP agent must not be stopped by one. A value
|
||||
* that already names http(s) is kept exactly as sent, and every other scheme
|
||||
* still fails. Read the parsed value, not the input: that is the one to store.
|
||||
*/
|
||||
export const placeWebsiteSchema = z
|
||||
.string()
|
||||
.max(500)
|
||||
.refine((v) => /^https?:\/\//i.test(v), { message: 'must be an http or https URL' });
|
||||
.overwrite((v) => (HTTP_URL.test(v) ? v : (normalizePlaceWebsite(v) ?? v)))
|
||||
.max(PLACE_WEBSITE_MAX_LENGTH)
|
||||
.refine((v) => HTTP_URL.test(v), { message: 'must be an http or https URL' });
|
||||
|
||||
/**
|
||||
* Embedded category as returned on a place — a trimmed projection of the
|
||||
|
||||
Reference in New Issue
Block a user