From 6643283c6f3073419285fc2f79451171c8e5aa2a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 13:46:04 +0000 Subject: [PATCH] =?UTF-8?q?Push-Zeitmail:=20Review-Fixes=20(Rate-Limit=20b?= =?UTF-8?q?egrenzt,=20kein=20NaN-Datum,=20St=C3=B6rung=20=E2=89=A0=20gel?= =?UTF-8?q?=C3=B6scht)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- server/routes/pushTime.js | 56 +++++++++++++++++++++++++------- server/services/pushTimeEntry.js | 31 +++++++++++++----- test/pushTimeEntry.test.js | 15 +++++++++ test/pushTimeRoute.test.js | 23 +++++++++++++ 4 files changed, 104 insertions(+), 21 deletions(-) diff --git a/server/routes/pushTime.js b/server/routes/pushTime.js index 45f7d27..070c74f 100644 --- a/server/routes/pushTime.js +++ b/server/routes/pushTime.js @@ -13,21 +13,43 @@ import { applyPushTime, formatMinutes, needsTimeEntry, parseMinutes, MAX_MINUTES const router = Router() -// Einfaches Rate-Limit je IP (30 Anfragen / 15 Minuten) +// Einfaches Rate-Limit je IP (30 Anfragen / 15 Minuten). Der Speicher bleibt begrenzt: abgelaufene +// Eintraege fliegen hoechstens einmal pro Minute raus, und bei mehr als RATE_MAX_KEYS IPs wird der +// aelteste Teil verworfen (Map behaelt die Einfuege-Reihenfolge). const RATE_LIMIT = 30 const RATE_WINDOW_MS = 15 * 60 * 1000 +const RATE_MAX_KEYS = 10000 const hits = new Map() +let lastPrune = 0 export function resetPushTimeRateLimitForTests() { hits.clear() + lastPrune = 0 } -function rateLimited(ip) { - const now = Date.now() - for (const [key, entry] of hits) if (entry.resetAt <= now) hits.delete(key) - const entry = hits.get(ip) || { count: 0, resetAt: now + RATE_WINDOW_MS } +export function pushTimeRateLimitSizeForTests() { + return hits.size +} + +export function isRateLimited(ip, now = Date.now()) { + if (now - lastPrune >= 60 * 1000) { + lastPrune = now + for (const [key, entry] of hits) if (entry.resetAt <= now) hits.delete(key) + } + let entry = hits.get(ip) + if (!entry || entry.resetAt <= now) { + hits.delete(ip) + if (hits.size >= RATE_MAX_KEYS) { + let drop = hits.size - RATE_MAX_KEYS + 1 + for (const key of hits.keys()) { + if (drop-- <= 0) break + hits.delete(key) + } + } + entry = { count: 0, resetAt: now + RATE_WINDOW_MS } + hits.set(ip, entry) + } entry.count += 1 - hits.set(ip, entry) return entry.count > RATE_LIMIT } @@ -86,6 +108,12 @@ function errorPage(res, status, title, text) { page(res, status, title, `

${e(title)}

${e(text)}

Zum Ticketsystem →`) } +/** Stoerung (z. B. Appwrite nicht erreichbar): eigene HTML-Seite statt JSON des globalen Handlers */ +function failurePage(res, err) { + console.error('[pushTime]', err?.message || err) + errorPage(res, 503, 'Gerade nicht erreichbar', 'Bitte versuch es gleich noch einmal. Die Zeit wurde nicht gespeichert.') +} + /** Repo und Commit-Titel aus den Arbeitsblatt-Details (Format von gitWorksheet.js) */ function pushInfo(details) { const lines = String(details || '').split('\n') @@ -143,7 +171,7 @@ function alreadyPage(res, ws, ticket) { /** Token pruefen und Arbeitsblatt laden; schickt bei Fehlern selbst die Seite und liefert null. */ async function resolve(req, res) { - if (rateLimited(req.ip || 'unbekannt')) { + if (isRateLimited(req.ip || 'unbekannt')) { errorPage(res, 429, 'Zu viele Anfragen', 'Bitte versuch es in ein paar Minuten noch einmal.') return null } @@ -159,7 +187,8 @@ async function resolve(req, res) { let ws try { ws = await getDocument(config.collections.worksheets, check.worksheetId) - } catch { + } catch (err) { + if (err?.status !== 404) throw err errorPage(res, 404, 'Nicht mehr vorhanden', 'Dieses Arbeitsblatt gibt es nicht mehr.') return null } @@ -170,18 +199,18 @@ async function resolve(req, res) { return { ws, ticket: await loadTicket(ws) } } -router.get('/:token', async (req, res, next) => { +router.get('/:token', async (req, res) => { try { const found = await resolve(req, res) if (!found) return if (!needsTimeEntry(found.ws)) return alreadyPage(res, found.ws, found.ticket) formPage(res, 200, { ...found, token: req.params.token, minutes: parseMinutes(req.query.min) ?? '' }) } catch (err) { - next(err) + failurePage(res, err) } }) -router.post('/:token', async (req, res, next) => { +router.post('/:token', async (req, res) => { try { const found = await resolve(req, res) if (!found) return @@ -199,6 +228,9 @@ router.post('/:token', async (req, res, next) => { const result = await applyPushTime(found.ws.$id, minutes) if (result.status === 'already') return alreadyPage(res, result.worksheet, found.ticket) + if (result.status === 'invalid') { + return errorPage(res, 422, 'Nicht möglich', 'Bei diesem Push fehlt die Uhrzeit. Trag die Zeit bitte direkt im Ticketsystem ein.') + } if (result.status !== 'saved') { return errorPage(res, 404, 'Nicht mehr vorhanden', 'Dieses Arbeitsblatt gibt es nicht mehr.') } @@ -210,7 +242,7 @@ router.post('/:token', async (req, res, next) => { `

Danke!

Eingetragen: ${e(formatMinutes(minutes))} auf #${e(woid)}

${openTicket(woid)}` ) } catch (err) { - next(err) + failurePage(res, err) } }) diff --git a/server/services/pushTimeEntry.js b/server/services/pushTimeEntry.js index cf40bae..05f9a0b 100644 --- a/server/services/pushTimeEntry.js +++ b/server/services/pushTimeEntry.js @@ -12,12 +12,20 @@ export const MAX_MINUTES = 720 const pad = (n) => String(n).padStart(2, '0') -/** Wand-Uhrzeit rueckwaerts rechnen ('tt.mm.jjjj', 'hhmm'); reine Kalenderrechnung, ohne Zeitzone. */ +/** + * Wand-Uhrzeit rueckwaerts rechnen ('tt.mm.jjjj', 'hhmm'); reine Kalenderrechnung, ohne Zeitzone + * (wie "Zeit nachtragen" in WOMS). Ungueltiges Datum/Uhrzeit -> null. + */ export function startFromEnd(endDate, endTime, minutes) { - const [day, month, year] = String(endDate).split('.').map(Number) - const hours = Number(String(endTime).slice(0, 2)) - const mins = Number(String(endTime).slice(2, 4)) - const start = new Date(Date.UTC(year, month - 1, day, hours, mins) - minutes * 60 * 1000) + const date = String(endDate || '').match(/^(\d{2})\.(\d{2})\.(\d{4})$/) + const time = String(endTime || '').match(/^(\d{2})(\d{2})$/) + if (!date || !time) return null + const [day, month, year] = [Number(date[1]), Number(date[2]), Number(date[3])] + const [hours, mins] = [Number(time[1]), Number(time[2])] + const end = new Date(Date.UTC(year, month - 1, day, hours, mins)) + // 31.02. o. ae. rollt in Date.UTC weiter - dann ist das Datum ungueltig + if (end.getUTCDate() !== day || end.getUTCMonth() !== month - 1 || hours > 23 || mins > 59) return null + const start = new Date(end.getTime() - minutes * 60 * 1000) return { startDate: `${pad(start.getUTCDate())}.${pad(start.getUTCMonth() + 1)}.${start.getUTCFullYear()}`, startTime: `${pad(start.getUTCHours())}${pad(start.getUTCMinutes())}`, @@ -50,20 +58,25 @@ export function formatMinutes(min) { } /** - * @returns {Promise<{ status: 'saved' | 'already' | 'missing' | 'not_git', worksheet?: object }>} + * @returns {Promise<{ status: 'saved' | 'already' | 'missing' | 'not_git' | 'invalid', worksheet?: object }>} + * invalid: Push-Datum/-Uhrzeit im Arbeitsblatt unbrauchbar (nichts gespeichert). + * Andere Appwrite-Fehler als 404 werden weitergeworfen. */ export async function applyPushTime(worksheetId, minutes) { return withLocks([`ws-time:${worksheetId}`], async () => { let ws try { ws = await getDocument(config.collections.worksheets, worksheetId) - } catch { - return { status: 'missing' } + } catch (err) { + if (err?.status === 404) return { status: 'missing' } + throw err } if (ws.serviceType !== 'GIT') return { status: 'not_git', worksheet: ws } if (!needsTimeEntry(ws)) return { status: 'already', worksheet: ws } - const { startDate, startTime } = startFromEnd(ws.endDate, ws.endTime, minutes) + const start = startFromEnd(ws.endDate, ws.endTime, minutes) + if (!start) return { status: 'invalid', worksheet: ws } + const { startDate, startTime } = start const worksheet = await updateDocument(config.collections.worksheets, worksheetId, { startDate, startTime, diff --git a/test/pushTimeEntry.test.js b/test/pushTimeEntry.test.js index a3324ba..9a4fd94 100644 --- a/test/pushTimeEntry.test.js +++ b/test/pushTimeEntry.test.js @@ -17,6 +17,7 @@ before(async () => { openGit('ws1'), openGit('ws-race'), { $id: 'remote1', serviceType: 'Remote', startTime: '', totalTime: 0 }, + { ...openGit('broken'), endDate: '', endTime: 'xx' }, ], }, }) @@ -72,3 +73,17 @@ test('gleichzeitig: nur einer speichert', async () => { const results = await Promise.all([applyPushTime('ws-race', 30), applyPushTime('ws-race', 90)]) assert.deepEqual(results.map((r) => r.status).sort(), ['already', 'saved']) }) + +test('kaputtes Push-Datum: nichts speichern (Review-Fix)', async () => { + assert.equal(startFromEnd('', '', 30), null) + assert.equal(startFromEnd('30.09.2026', '16:46', 30), null) + assert.equal(startFromEnd('31.02.2026', '1646', 30), null) + assert.equal((await applyPushTime('broken', 30)).status, 'invalid') + const ws = fake.get('worksheets', 'broken') + assert.deepEqual([ws.startTime, ws.totalTime, ws.isComment], ['', 0, true]) +}) + +test('Appwrite-Störung ist nicht „missing“ (Review-Fix)', async () => { + fake.failNext({ method: 'GET', pathIncludes: '/documents/ws-race', status: 500 }) + await assert.rejects(applyPushTime('ws-race', 30)) +}) diff --git a/test/pushTimeRoute.test.js b/test/pushTimeRoute.test.js index e1bd3d3..035c8ea 100644 --- a/test/pushTimeRoute.test.js +++ b/test/pushTimeRoute.test.js @@ -8,6 +8,8 @@ let server let baseUrl let createPushTimeToken let resetPushTimeRateLimitForTests +let isRateLimited +let rateLimitSize const gitSheet = (id, extra = {}) => ({ $id: id, wsid: '100300', woid: '100118', workorderId: 't1', serviceType: 'GIT', @@ -38,6 +40,8 @@ before(async () => { }) const routes = await import('../server/routes/pushTime.js') resetPushTimeRateLimitForTests = routes.resetPushTimeRateLimitForTests + isRateLimited = routes.isRateLimited + rateLimitSize = routes.pushTimeRateLimitSizeForTests ;({ createPushTimeToken } = await import('../server/services/pushTimeToken.js')) const app = express() @@ -159,3 +163,22 @@ test('Rate-Limit: 31. Anfrage → 429', async () => { assert.equal(r.status, 429) assert.match(r.html, /Zu viele Anfragen/) }) + +test('Rate-Limit-Speicher bleibt begrenzt (Review-Fix)', () => { + const now = Date.now() + for (let i = 0; i < 12000; i += 1) isRateLimited(`10.0.${Math.floor(i / 250)}.${i % 250}`, now) + assert.ok(rateLimitSize() <= 10000, `Größe ${rateLimitSize()}`) + // nach Ablauf des Fensters wird aufgeräumt + isRateLimited('10.9.9.9', now + 16 * 60 * 1000) + assert.equal(rateLimitSize(), 1) +}) + +test('Appwrite-Störung: neutrale HTML-Seite statt „Nicht mehr vorhanden“ (Review-Fix)', async () => { + const t = token('open1') + fake.failNext({ method: 'GET', pathIncludes: '/documents/open1', status: 500 }) + const r = await get(`/zeit/${t}`) + assert.equal(r.status, 503) + assert.match(r.headers.get('content-type'), /text\/html/) + assert.match(r.html, /Bitte versuch es gleich noch einmal/) + assert.doesNotMatch(r.html, /Nicht mehr vorhanden|fakeAppwrite/) +})