fix(kml-import): address PR #488 review issues

- Strip BOM (U+FEFF) from 14 translation files injected by editor
- Guard KMZ unpack against zip-bomb: check entry.uncompressedSize against
  50 MB cap (KMZ_DECOMPRESSED_SIZE_LIMIT) before calling .buffer();
  limit is an exported constant so tests can override it
- Fix non-BMP HTML entity decoding: replace String.fromCharCode with
  String.fromCodePoint + 0x10FFFF bounds check so emoji like 😀
  round-trip correctly
- Switch KML namespace stripping from regex to fast-xml-parser's
  removeNSPrefix option; XMLValidator accepts namespaced XML natively,
  making the pre-strip step unnecessary
- Remove dead skippedCount overwrite after transaction; per-loop
  increment already tracks it alongside per-item error messages
- Type multer req.file as Express.Multer.File on both /import/gpx
  and /import/map routes instead of (req as any).file
- Add unit tests: emoji entity decoding (decimal + hex), KMZ zip-bomb
  rejection, KMZ-with-no-KML rejection
This commit is contained in:
jubnl
2026-04-15 05:16:47 +02:00
parent a1a7795945
commit 801ffbfb7b
19 changed files with 103 additions and 41 deletions
+1 -1
View File
@@ -1,4 +1,4 @@
import en from './en' import en from './en'
const ar: Record<string, string | { name: string; category: string }[]> = { const ar: Record<string, string | { name: string; category: string }[]> = {
...en, ...en,
+1 -1
View File
@@ -1,4 +1,4 @@
const br: Record<string, string | { name: string; category: string }[]> = { const br: Record<string, string | { name: string; category: string }[]> = {
// Common // Common
'common.save': 'Salvar', 'common.save': 'Salvar',
'common.showMore': 'Mostrar mais', 'common.showMore': 'Mostrar mais',
+1 -1
View File
@@ -1,4 +1,4 @@
const cs: Record<string, string | { name: string; category: string }[]> = { const cs: Record<string, string | { name: string; category: string }[]> = {
// Společné (Common) // Společné (Common)
'common.save': 'Uložit', 'common.save': 'Uložit',
'common.showMore': 'Zobrazit více', 'common.showMore': 'Zobrazit více',
+1 -1
View File
@@ -1,4 +1,4 @@
const de: Record<string, string | { name: string; category: string }[]> = { const de: Record<string, string | { name: string; category: string }[]> = {
// Allgemein // Allgemein
'common.save': 'Speichern', 'common.save': 'Speichern',
'common.showMore': 'Mehr anzeigen', 'common.showMore': 'Mehr anzeigen',
+1 -1
View File
@@ -1,4 +1,4 @@
const en: Record<string, string | { name: string; category: string }[]> = { const en: Record<string, string | { name: string; category: string }[]> = {
// Common // Common
'common.save': 'Save', 'common.save': 'Save',
'common.showMore': 'Show more', 'common.showMore': 'Show more',
+1 -1
View File
@@ -1,4 +1,4 @@
const es: Record<string, string> = { const es: Record<string, string> = {
// Common // Common
'common.save': 'Guardar', 'common.save': 'Guardar',
'common.showMore': 'Ver más', 'common.showMore': 'Ver más',
+1 -1
View File
@@ -1,4 +1,4 @@
const fr: Record<string, string> = { const fr: Record<string, string> = {
// Common // Common
'common.save': 'Enregistrer', 'common.save': 'Enregistrer',
'common.showMore': 'Voir plus', 'common.showMore': 'Voir plus',
+1 -1
View File
@@ -1,4 +1,4 @@
const hu: Record<string, string | { name: string; category: string }[]> = { const hu: Record<string, string | { name: string; category: string }[]> = {
// Általános // Általános
'common.save': 'Mentés', 'common.save': 'Mentés',
'common.showMore': 'Továbbiak', 'common.showMore': 'Továbbiak',
+1 -1
View File
@@ -1,4 +1,4 @@
const it: Record<string, string | { name: string; category: string }[]> = { const it: Record<string, string | { name: string; category: string }[]> = {
// Common // Common
'common.save': 'Salva', 'common.save': 'Salva',
'common.showMore': 'Mostra di più', 'common.showMore': 'Mostra di più',
+1 -1
View File
@@ -1,4 +1,4 @@
const nl: Record<string, string> = { const nl: Record<string, string> = {
// Common // Common
'common.save': 'Opslaan', 'common.save': 'Opslaan',
'common.showMore': 'Meer tonen', 'common.showMore': 'Meer tonen',
+1 -1
View File
@@ -1,4 +1,4 @@
const pl: Record<string, string | { name: string; category: string }[]> = { const pl: Record<string, string | { name: string; category: string }[]> = {
// Common // Common
'common.save': 'Zapisz', 'common.save': 'Zapisz',
'common.showMore': 'Pokaż więcej', 'common.showMore': 'Pokaż więcej',
+1 -1
View File
@@ -1,4 +1,4 @@
const ru: Record<string, string> = { const ru: Record<string, string> = {
// Common // Common
'common.save': 'Сохранить', 'common.save': 'Сохранить',
'common.showMore': 'Показать больше', 'common.showMore': 'Показать больше',
+1 -1
View File
@@ -1,4 +1,4 @@
const zh: Record<string, string> = { const zh: Record<string, string> = {
// Common // Common
'common.save': '保存', 'common.save': '保存',
'common.showMore': '显示更多', 'common.showMore': '显示更多',
+1 -1
View File
@@ -1,4 +1,4 @@
const zhTw: Record<string, string> = { const zhTw: Record<string, string> = {
// Common // Common
'common.save': '儲存', 'common.save': '儲存',
'common.showMore': '顯示更多', 'common.showMore': '顯示更多',
+2 -2
View File
@@ -63,7 +63,7 @@ router.post('/import/gpx', authenticate, requireTripAccess, uploadMulter.single(
return res.status(403).json({ error: 'No permission' }); return res.status(403).json({ error: 'No permission' });
const { tripId } = req.params; const { tripId } = req.params;
const file = (req as any).file; const file = req.file as Express.Multer.File | undefined;
if (!file) return res.status(400).json({ error: 'No file uploaded' }); if (!file) return res.status(400).json({ error: 'No file uploaded' });
const created = importGpx(tripId, file.buffer); const created = importGpx(tripId, file.buffer);
@@ -84,7 +84,7 @@ router.post('/import/map', authenticate, requireTripAccess, uploadMulter.single(
} }
const { tripId } = req.params; const { tripId } = req.params;
const file = (req as any).file; const file = req.file as Express.Multer.File | undefined;
if (!file) return res.status(400).json({ error: 'No file uploaded' }); if (!file) return res.status(400).json({ error: 'No file uploaded' });
try { try {
+2 -10
View File
@@ -50,22 +50,14 @@ function decodeHtmlEntities(value: string): string {
return withNamedEntities return withNamedEntities
.replace(/&#(\d+);/g, (_, dec) => { .replace(/&#(\d+);/g, (_, dec) => {
const code = Number(dec); const code = Number(dec);
return Number.isFinite(code) ? String.fromCharCode(code) : _; return Number.isFinite(code) && code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : _;
}) })
.replace(/&#x([0-9a-fA-F]+);/g, (_, hex) => { .replace(/&#x([0-9a-fA-F]+);/g, (_, hex) => {
const code = Number.parseInt(hex, 16); const code = Number.parseInt(hex, 16);
return Number.isFinite(code) ? String.fromCharCode(code) : _; return Number.isFinite(code) && code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : _;
}); });
} }
export function stripXmlNamespaces(xml: string): string {
// KML exports vary heavily; stripping namespace declarations/prefixes makes parsing resilient.
return xml
.replace(/\sxmlns(:\w+)?="[^"]*"/g, '')
.replace(/\sxmlns(:\w+)?='[^']*'/g, '')
.replace(/<(\/?)\w+:/g, '<$1');
}
export function decodeUtf8WithWarning(fileBuffer: Buffer): { text: string; warning: string | null } { export function decodeUtf8WithWarning(fileBuffer: Buffer): { text: string; warning: string | null } {
try { try {
return { text: UTF8_DECODER_FATAL.decode(fileBuffer), warning: null }; return { text: UTF8_DECODER_FATAL.decode(fileBuffer), warning: null };
+14 -6
View File
@@ -11,7 +11,6 @@ import {
extractKmlPlacemarkNodes, extractKmlPlacemarkNodes,
parsePlacemarkNode, parsePlacemarkNode,
resolveCategoryIdForFolder, resolveCategoryIdForFolder,
stripXmlNamespaces,
type KmlImportSummary, type KmlImportSummary,
} from './kmlImport'; } from './kmlImport';
@@ -254,9 +253,12 @@ const gpxParser = new XMLParser({
const kmlParser = new XMLParser({ const kmlParser = new XMLParser({
ignoreAttributes: false, ignoreAttributes: false,
attributeNamePrefix: '@_', attributeNamePrefix: '@_',
removeNSPrefix: true,
isArray: (name) => ['Placemark', 'Folder', 'Document'].includes(name), isArray: (name) => ['Placemark', 'Folder', 'Document'].includes(name),
}); });
export const KMZ_DECOMPRESSED_SIZE_LIMIT = 50 * 1024 * 1024; // 50 MB
export function importGpx(tripId: string, fileBuffer: Buffer) { export function importGpx(tripId: string, fileBuffer: Buffer) {
const parsed = gpxParser.parse(fileBuffer.toString('utf-8')); const parsed = gpxParser.parse(fileBuffer.toString('utf-8'));
const gpx = parsed?.gpx; const gpx = parsed?.gpx;
@@ -327,14 +329,13 @@ export function importGpx(tripId: string, fileBuffer: Buffer) {
export function importKmlPlaces(tripId: string, fileBuffer: Buffer): PlaceImportResult { export function importKmlPlaces(tripId: string, fileBuffer: Buffer): PlaceImportResult {
const decoded = decodeUtf8WithWarning(fileBuffer); const decoded = decodeUtf8WithWarning(fileBuffer);
const xmlWithoutNamespaces = stripXmlNamespaces(decoded.text);
const validationResult = XMLValidator.validate(xmlWithoutNamespaces); const validationResult = XMLValidator.validate(decoded.text);
if (validationResult !== true) { if (validationResult !== true) {
throw new Error('Malformed KML: invalid XML structure'); throw new Error('Malformed KML: invalid XML structure');
} }
const parsed = kmlParser.parse(xmlWithoutNamespaces); const parsed = kmlParser.parse(decoded.text);
const kmlRoot = parsed?.kml ?? parsed; const kmlRoot = parsed?.kml ?? parsed;
if (!kmlRoot || typeof kmlRoot !== 'object') { if (!kmlRoot || typeof kmlRoot !== 'object') {
@@ -391,7 +392,6 @@ export function importKmlPlaces(tripId: string, fileBuffer: Buffer): PlaceImport
}); });
insertAll(); insertAll();
summary.skippedCount = summary.totalPlacemarks - summary.createdCount;
if (summary.totalPlacemarks === 0) { if (summary.totalPlacemarks === 0) {
summary.errors.push('No Placemarks found in KML file.'); summary.errors.push('No Placemarks found in KML file.');
@@ -400,7 +400,10 @@ export function importKmlPlaces(tripId: string, fileBuffer: Buffer): PlaceImport
return { places: created, count: created.length, summary }; return { places: created, count: created.length, summary };
} }
export async function unpackKmzToKml(kmzBuffer: Buffer): Promise<Buffer> { export async function unpackKmzToKml(
kmzBuffer: Buffer,
decompressedSizeLimit = KMZ_DECOMPRESSED_SIZE_LIMIT,
): Promise<Buffer> {
let zip; let zip;
try { try {
zip = await unzipper.Open.buffer(kmzBuffer); zip = await unzipper.Open.buffer(kmzBuffer);
@@ -414,6 +417,11 @@ export async function unpackKmzToKml(kmzBuffer: Buffer): Promise<Buffer> {
} }
const preferredEntry = kmlEntries.find((entry) => entry.path.toLowerCase().endsWith('doc.kml')) || kmlEntries[0]; const preferredEntry = kmlEntries.find((entry) => entry.path.toLowerCase().endsWith('doc.kml')) || kmlEntries[0];
if (preferredEntry.uncompressedSize > decompressedSizeLimit) {
throw new Error('KMZ archive exceeds the maximum allowed decompressed size.');
}
return preferredEntry.buffer(); return preferredEntry.buffer();
} }
@@ -7,18 +7,9 @@ import {
parsePlacemarkNode, parsePlacemarkNode,
resolveCategoryIdForFolder, resolveCategoryIdForFolder,
sanitizeKmlDescription, sanitizeKmlDescription,
stripXmlNamespaces,
} from '../../../src/services/kmlImport'; } from '../../../src/services/kmlImport';
describe('kmlImportUtils', () => { describe('kmlImportUtils', () => {
it('strips KML namespaces and prefixes', () => {
const xml = '<kml xmlns="http://www.opengis.net/kml/2.2"><kml:Document><kml:Placemark /></kml:Document></kml>';
const stripped = stripXmlNamespaces(xml);
expect(stripped).not.toContain('xmlns');
expect(stripped).toContain('<Document>');
expect(stripped).toContain('<Placemark');
});
it('sanitizes HTML descriptions with br to newline', () => { it('sanitizes HTML descriptions with br to newline', () => {
const input = 'Line 1<br>Line <b>2</b> &amp; more'; const input = 'Line 1<br>Line <b>2</b> &amp; more';
const output = sanitizeKmlDescription(input); const output = sanitizeKmlDescription(input);
@@ -64,6 +55,16 @@ describe('kmlImportUtils', () => {
expect(resolveCategoryIdForFolder('parks', lookup)).toBe(4); expect(resolveCategoryIdForFolder('parks', lookup)).toBe(4);
}); });
it('decodes non-BMP decimal HTML entities (emoji)', () => {
// &#128512; = U+1F600 = 😀 — requires String.fromCodePoint, not fromCharCode
expect(sanitizeKmlDescription('&#128512;')).toBe('😀');
});
it('decodes non-BMP hex HTML entities (emoji)', () => {
// &#x1F600; = U+1F600 = 😀
expect(sanitizeKmlDescription('&#x1F600;')).toBe('😀');
});
it('returns warning for non-UTF8 payload', () => { it('returns warning for non-UTF8 payload', () => {
const buffer = Buffer.concat([ const buffer = Buffer.concat([
Buffer.from('<?xml version="1.0"?><kml><Document><Placemark><name>Caf'), Buffer.from('<?xml version="1.0"?><kml><Document><Placemark><name>Caf'),
@@ -0,0 +1,61 @@
import { describe, it, expect, vi } from 'vitest';
import path from 'path';
import fs from 'fs';
vi.mock('../../../src/db/database', () => ({
db: { prepare: vi.fn() },
getPlaceWithTags: vi.fn(),
}));
vi.mock('../../../src/config', () => ({
JWT_SECRET: 'test',
ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
updateJwtSecret: () => {},
}));
import { unpackKmzToKml, KMZ_DECOMPRESSED_SIZE_LIMIT } from '../../../src/services/placeService';
const KMZ_FIXTURE = path.join(__dirname, '../../fixtures/test.kmz');
describe('unpackKmzToKml', () => {
it('extracts the KML entry from a valid KMZ', async () => {
const kmzBuffer = fs.readFileSync(KMZ_FIXTURE);
const kmlBuffer = await unpackKmzToKml(kmzBuffer);
expect(kmlBuffer.length).toBeGreaterThan(0);
expect(kmlBuffer.toString('utf-8')).toContain('<kml');
});
it('rejects a KMZ whose KML entry exceeds the decompressed size limit', async () => {
const kmzBuffer = fs.readFileSync(KMZ_FIXTURE);
// test.kmz contains a KML with uncompressedSize 634 — set limit to 1 byte
await expect(unpackKmzToKml(kmzBuffer, 1)).rejects.toThrow('exceeds the maximum allowed decompressed size');
});
it('rejects a KMZ that contains no KML file', async () => {
// Craft a minimal ZIP containing only a non-KML entry using raw ZIP bytes
// We use the test GPX fixture (a real file) re-zipped via Node's zlib/archiver
// Simplest: a KMZ whose only file has a .txt extension
const Archiver = await import('archiver');
const archiver = Archiver.default;
const { PassThrough } = await import('stream');
const chunks: Buffer[] = [];
const output = new PassThrough();
output.on('data', (chunk) => chunks.push(chunk));
const archive = archiver('zip', { zlib: { level: 1 } });
archive.pipe(output);
archive.append(Buffer.from('not a kml'), { name: 'data.txt' });
await archive.finalize();
const zipBuffer = Buffer.concat(chunks);
await expect(unpackKmzToKml(zipBuffer)).rejects.toThrow('does not contain a KML file');
});
it('rejects a buffer that is not a valid ZIP archive', async () => {
await expect(unpackKmzToKml(Buffer.from('this is not a zip'))).rejects.toThrow('Invalid KMZ archive');
});
it('exports KMZ_DECOMPRESSED_SIZE_LIMIT as 50 MB', () => {
expect(KMZ_DECOMPRESSED_SIZE_LIMIT).toBe(50 * 1024 * 1024);
});
});