fix: collision names must survive the sanitiser the content view applies

unique_filename produced 'manual (2).pdf'; HouseplanContentView sanitises the
name in the REQUEST too, turning ' (2)' into '_2_', so the file was written and
then 404'd. The same pattern was already in files/migrate, so a rebind that hit
a name collision has been producing dead links. Both use the shared helper now,
with '-2', which round-trips sanitize_filename — asserted.

custom_components/houseplan/plans.py

@@ -35,7 +35,11 @@ def unique_filename(directory: Path, name: str) -> str:
         stem, suffix = safe, ""
     i = 2
     while True:
-        candidate = f"{stem} ({i}){'.' + suffix if suffix else ''}"
+        # "-2", not " (2)": the content view sanitizes the name in the REQUEST
+        # too, and a space or a bracket there turns into "_", so a file called
+        # "manual (2).pdf" was written and then never served. Only characters
+        # that survive sanitize_filename may be used to build a name.
+        candidate = f"{stem}-{i}{'.' + suffix if suffix else ''}"
         if not (directory / candidate).exists():
             return candidate
         i += 1

custom_components/houseplan/websocket_api.py

@@ -20,7 +20,7 @@ from .const import (
     CONTENT_URL, FILES_DIR, MAX_SIGN_PATHS, PLANS_DIR, PLANS_URL,
 )
 from .auth import may_write
-from .plans import collect_attachments, collect_plans
+from .plans import collect_attachments, collect_plans, unique_filename
 from .store import HouseplanData, get_data, get_entry
 from .validation import (
     CONFIG_SCHEMA, LAYOUT_SCHEMA, MAX_CONFIG_BYTES, MAX_PLAN_BYTES,
@@ -193,15 +193,10 @@ async def ws_files_migrate(hass: HomeAssistant, connection, msg: dict[str, Any])
         for f in sorted(src.iterdir()):
             if not f.is_file():
                 continue
-            target = dst / f.name
+            # a different file may already own this name — do NOT silently point
-            if target.exists():
+            # the url at it; the shared helper picks a free one, using only
-                # a different file already owns this name — do NOT silently
+            # characters the content view will accept back in a request
-                # point the url at it; give the copy a unique name instead
+            target = dst / unique_filename(dst, f.name)
-                stem, suffix = f.stem, f.suffix
-                i = 2
-                while (dst / f"{stem} ({i}){suffix}").exists():
-                    i += 1
-                target = dst / f"{stem} ({i}){suffix}"
             shutil.copy2(str(f), str(target))
             mapping[f.name] = target.name
         return mapping

docs/CHANGELOG.md

@@ -25,7 +25,11 @@
   shared folder, so two of them attaching `manual.pdf` ended up pointing at the
   same physical file. Uploads now take a free name and never overwrite, a new
   icon gets its own staging folder whose files move to the real icon when the
-  save is accepted, and an upload nobody saved is collected an hour later.
+  save is accepted, and an upload nobody saved is collected an hour later. The
+  name a collision falls back to changed from `manual (2).pdf` to `manual-2.pdf`
+  — the old one was sanitised on the way back in, so a renamed attachment was
+  written and then never served (found by the new test, and it applied to
+  rebind collisions before this release too).
 - **Two quick edits can no longer lose the second one (HP-1454-03).** The
   debounce spaced out the starts of a save, not the saves themselves. If one
   took longer than half a second — a busy instance, a slow link — the next edit

docs/CHANGELOG.ru.md

@@ -33,7 +33,11 @@
   один физический файл. Теперь загрузка занимает свободное имя и никогда не
   перезаписывает, новая иконка получает собственную промежуточную папку, файлы
   из которой переезжают к настоящей иконке при принятом сохранении, а загрузка,
-  которую никто не сохранил, убирается через час.
+  которую никто не сохранил, убирается через час. Имя, на которое уходит
+  коллизия, сменилось с `manual (2).pdf` на `manual-2.pdf`: старое санитайзилось
+  на обратном пути, поэтому переименованное вложение записывалось и больше не
+  отдавалось (нашёл новый тест; до этого релиза так же ломались коллизии при
+  перепривязке).
 - **Две быстрые правки больше не теряют вторую (HP-1454-03).** Debounce
   разносил старты сохранения, а не сами сохранения. Если одно длилось дольше
   полусекунды — занятый сервер, медленная связь, — следующая правка уходила с

test/logic.test.mjs

@@ -798,8 +798,10 @@ test('migratePdfUrls: only confirmed copies are rewritten (review CR-3)', () =>
     { name: 'b.pdf', url: '/houseplan_files/files/v_old1/b.pdf?v=2' },
   ];
   // сервер скопировал только a.pdf, причём переименовал из-за коллизии
-  const out = migratePdfUrls(pdfs, 'v_old1', 'dev99', { 'a.pdf': 'a (2).pdf' });
+  // collision names use only characters the content view accepts back in a
-  assert.equal(out[0].url, '/houseplan_files/files/dev99/a%20(2).pdf?v=1');
+  // request: ' (2)' was sanitised to '_2_' server-side and 404'd (v1.46.0)
+  const out = migratePdfUrls(pdfs, 'v_old1', 'dev99', { 'a.pdf': 'a-2.pdf' });
+  assert.equal(out[0].url, '/houseplan_files/files/dev99/a-2.pdf?v=1');
   assert.equal(out[1].url, pdfs[1].url, 'нескопированный файл ссылается на старую папку');
   // пустой маппинг = ничего не переносим
   assert.deepEqual(migratePdfUrls(pdfs, 'v_old1', 'dev99', {}).map((p) => p.url),

tests_backend/test_ha_websocket.py

@@ -690,7 +690,7 @@ async def test_upload_never_overwrites_an_existing_attachment(
     names = sorted(
         n for n in await hass.async_add_executor_job(os.listdir, folder) if n.startswith("manual")
     )
-    assert names == ["manual (2).pdf", "manual.pdf"]
+    assert names == ["manual-2.pdf", "manual.pdf"]
 
     got = await http.get(first.replace(CONTENT_URL, CONTENT_URL))
     assert await got.read() == b"ONE", "the first file is untouched"

tests_backend/test_validation.py

@@ -392,14 +392,20 @@ def test_unique_filename_never_returns_a_taken_name(tmp_path):
     d.mkdir()
     assert unique_filename(d, "manual.pdf") == "manual.pdf"
     (d / "manual.pdf").write_bytes(b"x")
-    assert unique_filename(d, "manual.pdf") == "manual (2).pdf"
+    assert unique_filename(d, "manual.pdf") == "manual-2.pdf"
-    (d / "manual (2).pdf").write_bytes(b"x")
+    (d / "manual-2.pdf").write_bytes(b"x")
-    assert unique_filename(d, "manual.pdf") == "manual (3).pdf"
+    assert unique_filename(d, "manual.pdf") == "manual-3.pdf"
     # no extension, and a name that needs sanitising
     (d / "readme").write_bytes(b"x")
-    assert unique_filename(d, "readme") == "readme (2)"
+    assert unique_filename(d, "readme") == "readme-2"
     assert unique_filename(d, "../../etc/passwd") == "passwd"
 
+    # A generated name has to survive the sanitiser the CONTENT VIEW applies to
+    # the request, or the file is written and then never served: " (2)" became
+    # "_2_" and every collision-renamed attachment 404'd.
+    for candidate in ("manual-2.pdf", "manual-3.pdf", "readme-2"):
+        assert v.sanitize_filename(candidate) == candidate
+
 
 def _acfg(*pairs):
     return {"markers": [{"id": f"m{i}", "pdfs": [{"url": f"/api/houseplan/content/files/{p}"}]}