mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(journey): escape the tooltip text on the Leaflet maps
A string handed to bindTooltip becomes innerHTML. JourneyMap passed an entry title and a track name through untouched; the GL renderer beside it escapes both already, and JourneyPublicPage imports the Leaflet one directly. The atlas bucket-list tooltip had the same gap. Also pins the escape on MapViewGL's photo marker. Its Leaflet twin has FE-COMP-MAPVIEW-074 for this, but the GL side was only ever exercised with benign values, so escapeHtml could have been dropped there without a test noticing.
This commit is contained in:
@@ -550,4 +550,30 @@ describe('JourneyMap', () => {
|
||||
expect(colors).not.toContain('#ff0000');
|
||||
expect(colors).not.toContain('#ffffff');
|
||||
});
|
||||
|
||||
// A string handed to bindTooltip becomes innerHTML. A track name is a place
|
||||
// name off a shared trip and an entry label is a journey title, so both are
|
||||
// written by someone who is not necessarily the person reading the map — and
|
||||
// this component is what the public journey page renders.
|
||||
it('FE-COMP-JOURNEYMAP-044: escapes a track name before it becomes tooltip markup', () => {
|
||||
vi.mocked(L.polyline).mockClear();
|
||||
render(<JourneyMap checkins={[]} entries={entriesWithCoords} tracks={[track({ name: '<img src=x onerror="alert(1)">' })]} />);
|
||||
|
||||
const line = vi.mocked(L.polyline).mock.results
|
||||
.map(r => r.value as any)
|
||||
.find(v => v.bindTooltip.mock.calls.length > 0);
|
||||
const [label] = line.bindTooltip.mock.calls[0];
|
||||
expect(label).not.toContain('<img');
|
||||
expect(label).not.toContain('onerror="');
|
||||
expect(label).toBe('<img src=x onerror="alert(1)">');
|
||||
});
|
||||
|
||||
it('FE-COMP-JOURNEYMAP-045: escapes an entry title before it becomes tooltip markup', () => {
|
||||
const hostile = [{ id: 'e9', lat: 48.1, lng: 2.1, title: '<img src=x onerror="alert(1)">', mood: null, entry_date: '2025-06-04' }];
|
||||
render(<JourneyMap checkins={[]} entries={hostile} />);
|
||||
|
||||
const labels = vi.mocked(mockedMarker().bindTooltip).mock.calls.map(c => c[0]);
|
||||
expect(labels.some(l => typeof l === 'string' && l.includes('<img'))).toBe(false);
|
||||
expect(labels).toContain('<img src=x onerror="alert(1)">');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useEffect, useRef, useImperativeHandle, useCallback, type Ref } from 'react'
|
||||
import L from 'leaflet'
|
||||
import { useSettingsStore } from '../../store/settingsStore'
|
||||
import type { JourneyTrack } from '@trek/shared'
|
||||
import { escapeHtml, type JourneyTrack } from '@trek/shared'
|
||||
|
||||
export interface MapMarkerItem {
|
||||
id: string
|
||||
@@ -212,8 +212,9 @@ function JourneyMap(
|
||||
const line = L.polyline(coords, { color, weight: 3.5, opacity: 0.95, lineCap: 'round', lineJoin: 'round' })
|
||||
// Same tooltip the markers on this map use, rather than Leaflet's default box:
|
||||
// it follows the appearance tokens, so it lands right in dark mode and with
|
||||
// transparency switched off.
|
||||
if (track.name) line.bindTooltip(track.name, { sticky: true, direction: 'top', className: 'map-tooltip' })
|
||||
// transparency switched off. Escaped because a string handed to bindTooltip
|
||||
// becomes innerHTML, and a track name is a place name off a shared trip.
|
||||
if (track.name) line.bindTooltip(escapeHtml(track.name), { sticky: true, direction: 'top', className: 'map-tooltip' })
|
||||
line.addTo(map)
|
||||
coords.forEach(c => allCoords.push(c))
|
||||
}
|
||||
@@ -243,7 +244,9 @@ function JourneyMap(
|
||||
})
|
||||
|
||||
const marker = L.marker(pos, { icon }).addTo(map)
|
||||
marker.bindTooltip(item.label, {
|
||||
// Escaped for the same reason as the track tooltip above: the label is an
|
||||
// entry title, and this map is what the public journey page renders.
|
||||
marker.bindTooltip(escapeHtml(item.label), {
|
||||
direction: 'top',
|
||||
offset: [0, -MARKER_H],
|
||||
className: 'map-tooltip',
|
||||
|
||||
@@ -851,6 +851,23 @@ describe('MapViewGL', () => {
|
||||
expect(glMarkers.created[1].lngLat).toEqual([2.12, 48.12])
|
||||
})
|
||||
|
||||
// The Leaflet twin of this is FE-COMP-MAPVIEW-074. Both renderers build their
|
||||
// marker as an HTML string, so both need the escape pinned — otherwise it can
|
||||
// be dropped from one of them without a single test going red.
|
||||
it('FE-COMP-MAPVIEWGL-023b: an image_url that passes the prefix check is still escaped', async () => {
|
||||
loadOnAttach()
|
||||
render(<MapViewGL places={[buildMapPlace({
|
||||
id: 13, lat: 48.13, lng: 2.13, image_url: '/uploads/x" onerror="alert(1)" y="',
|
||||
})]} fitKey={1} />)
|
||||
await act(async () => {})
|
||||
|
||||
const marker = glMarkers.created[0].element
|
||||
expect(marker.innerHTML).not.toContain('onerror="alert(1)"')
|
||||
expect(marker.innerHTML).toContain('"')
|
||||
// The escape must not break the ordinary case: the src still resolves.
|
||||
expect(marker.querySelector('img')?.getAttribute('src')).toBe('/uploads/x" onerror="alert(1)" y="')
|
||||
})
|
||||
|
||||
it('FE-COMP-MAPVIEWGL-024: the hover card shows name, category and address and follows the cursor', async () => {
|
||||
loadOnAttach()
|
||||
const places = [buildMapPlace({
|
||||
|
||||
@@ -6,7 +6,7 @@ import apiClient, { mapsApi, pluginsApi, type PluginAtlasLayer } from '../../api
|
||||
import L from 'leaflet'
|
||||
import type { GeoJsonFeatureCollection } from '../../types'
|
||||
import { A2_TO_A3, countryStatus, findBucketDuplicate, isBucketDuplicateError, isCountryVisible, normalizeRegionName, withCountryMarkedVisited, wishlistA3Codes, countryColor, type AtlasData, type AtlasPlaceHit, type CountryDetail, type BucketItem } from './atlasModel'
|
||||
import { continentForCountry, type VisitStatus } from '@trek/shared'
|
||||
import { continentForCountry, escapeHtml, type VisitStatus } from '@trek/shared'
|
||||
import { useToast } from '../../components/shared/Toast'
|
||||
import { getApiErrorMessage } from '../../types'
|
||||
|
||||
@@ -941,7 +941,7 @@ export function useAtlas() {
|
||||
iconAnchor: [14, 14],
|
||||
})
|
||||
return L.marker([b.lat!, b.lng!], { icon }).bindTooltip(
|
||||
`<div style="font-size:12px;font-weight:600">${b.name}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${b.notes}</div>` : ''}`,
|
||||
`<div style="font-size:12px;font-weight:600">${escapeHtml(b.name)}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${escapeHtml(b.notes)}</div>` : ''}`,
|
||||
{ className: 'atlas-tooltip', direction: 'top', offset: [0, -14] }
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user