Skip to main content

max / makenotwork

Fix the two upload defects the shape recon deferred Both found while drawing Shape 3's line and both left out of it, on the grounds that a conversion is the wrong place to change behaviour. formatEta rounded twice. `ceil(sec / 60)` for the minutes and `ceil(sec % 60)` for the remainder, so 125 seconds printed "3m 5s": the minutes had already absorbed the five and it was added again. Rounding to whole seconds once and splitting after gives "2m 5s", and it also keeps the remainder under a minute, which floor-then-ceil would not have (119.5 seconds would have read "1m 60s"). The test asserted the wrong answer with a comment checking the arithmetic of the expression rather than the meaning of the result, so it changes with the code, and a second test covers the two boundaries the old spelling got wrong. Fixed in both copies. static/upload.js is the original and s3.logic.ts is its typed port, which is how the bug survived the port. item-upload.js wrote a file name into a title attribute through a hand-rolled escape that covered only the double quote, eleven lines above a helper covering four characters and two lines below a call to the global covering five. All three were escaping the same untrusted string. The local helper is gone and both sites take escapeHtml, which the file was already calling.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-18 23:59 UTC
Signed with PGP, not checked
Commit: ee5d21446e73ffa38beaca678a09b21eef74b1c2
Parent: 3096d01
4 files changed, +34 insertions, -12 deletions
@@ -191,8 +191,8 @@
191 191 tr.dataset.idx = idx;
192 192 tr.style.borderBottom = '1px solid var(--border)';
193 193 tr.innerHTML =
194 - '<td style="padding: 0.4rem 0.5rem 0.4rem 0; font-size: 0.85rem; max-width: 200px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap;" title="' + file.name.replace(/"/g, '&quot;') + '">' + escapeHtml(file.name) + '</td>' +
195 - '<td style="padding: 0.4rem 0.5rem;"><input type="text" class="version-label-input" data-idx="' + idx + '" value="' + escapeAttr(guessLabel(file.name)) + '" placeholder="e.g., macOS (arm)" style="width: 100%; padding: 0.25rem 0.4rem; font-size: 0.85rem;"></td>' +
194 + '<td style="padding: 0.4rem 0.5rem 0.4rem 0; font-size: 0.85rem; max-width: 200px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap;" title="' + escapeHtml(file.name) + '">' + escapeHtml(file.name) + '</td>' +
195 + '<td style="padding: 0.4rem 0.5rem;"><input type="text" class="version-label-input" data-idx="' + idx + '" value="' + escapeHtml(guessLabel(file.name)) + '" placeholder="e.g., macOS (arm)" style="width: 100%; padding: 0.25rem 0.4rem; font-size: 0.85rem;"></td>' +
196 196 '<td style="padding: 0.4rem 0.5rem;"><button type="button" class="btn-secondary version-remove-file" data-idx="' + idx + '" style="padding: 0.2rem 0.5rem; font-size: 0.75rem;">Remove</button></td>';
197 197 fileRows.appendChild(tr);
198 198
@@ -202,8 +202,6 @@
202 202 });
203 203 }
204 204
205 - function escapeAttr(s) { return s.replace(/&/g, '&amp;').replace(/"/g, '&quot;').replace(/</g, '&lt;').replace(/>/g, '&gt;'); }
206 -
207 205 // Upload all button
208 206 document.getElementById('create-version-btn').addEventListener('click', function() {
209 207 var versionNumber = document.getElementById('new-version-number').value.trim();
@@ -48,9 +48,16 @@
48 48 var speedStr = speed > 1024 * 1024
49 49 ? (speed / (1024 * 1024)).toFixed(1) + ' MB/s'
50 50 : (speed / 1024).toFixed(0) + ' KB/s';
51 - var etaStr = remaining < 60
52 - ? Math.ceil(remaining) + 's'
53 - : Math.ceil(remaining / 60) + 'm ' + Math.ceil(remaining % 60) + 's';
51 + // Rounded once, before the split into minutes and
52 + // seconds. Rounding both parts read 125 seconds as
53 + // "3m 5s", the minutes having already absorbed the
54 + // remainder that was then added again. Same fix as
55 + // frontend/src/islands/uploader/s3.logic.ts, which is
56 + // the typed port of this block.
57 + var whole = Math.ceil(remaining);
58 + var etaStr = whole < 60
59 + ? whole + 's'
60 + : Math.floor(whole / 60) + 'm ' + (whole % 60) + 's';
54 61 self.speedEl.textContent = speedStr + ', ' + etaStr + ' remaining';
55 62 }
56 63 }
@@ -9,6 +9,15 @@
9 9
10 10 test('formatEta uses seconds under a minute, m+s above', () => {
11 11 assert.equal(formatEta(45), '45s');
12 - assert.equal(formatEta(125), '3m 5s'); // ceil(125/60)=3, ceil(125%60=5)=5
12 + assert.equal(formatEta(125), '2m 5s');
13 13 assert.equal(formatEta(0.2), '1s'); // ceil rounds up
14 14 });
15 +
16 + test('formatEta rounds once, so the remainder cannot reach a minute', () => {
17 + // The previous spelling read 125 as "3m 5s", rounding the minutes up and
18 + // then adding the remainder it had already absorbed. Rounding to whole
19 + // seconds first is what stops both that and a "1m 60s" from 119.5.
20 + assert.equal(formatEta(119.5), '2m 0s');
21 + assert.equal(formatEta(59.5), '1m 0s');
22 + assert.equal(formatEta(3600), '60m 0s');
23 + });
@@ -7,9 +7,17 @@
7 7 : (bytesPerSec / 1024).toFixed(0) + ' KB/s';
8 8 }
9 9
10 - /** Human-readable ETA, e.g. "45s" or "2m 5s". */
10 + /**
11 + * Human-readable ETA, e.g. "45s" or "2m 5s".
12 + *
13 + * One rounding, applied once, before the split into minutes and seconds. The
14 + * previous spelling rounded twice, `ceil(sec / 60)` for the minutes and
15 + * `ceil(sec % 60)` for the remainder, so 125 seconds read as "3m 5s": the
16 + * minutes had already absorbed the remainder and then it was added again.
17 + * Rounding first also keeps the remainder under 60, which floor-then-ceil
18 + * would not: 119.5 seconds would have printed "1m 60s".
19 + */
11 20 export function formatEta(remainingSec: number): string {
12 - return remainingSec < 60
13 - ? Math.ceil(remainingSec) + 's'
14 - : Math.ceil(remainingSec / 60) + 'm ' + Math.ceil(remainingSec % 60) + 's';
21 + const whole = Math.ceil(remainingSec);
22 + return whole < 60 ? whole + 's' : Math.floor(whole / 60) + 'm ' + (whole % 60) + 's';
15 23 }