From 3facc323b66c261fed5721ede8d134227d8a3202 Mon Sep 17 00:00:00 2001 From: mwiegand Date: Sun, 17 May 2026 16:24:32 +0200 Subject: [PATCH] refactor(files): extract _load_file_for_editing helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 11/12 of docs/superpowers/plans/2026-05-17-files-overlay-rewrite.md. overlay_file_content and overlay_file_edit_page used to repeat the same prelude: load the files-overlay, resolve sub_path safely, confirm it's a regular file, attempt a UTF-8 read with the is_editable sniff + UnicodeDecodeError-on-tail fallback. The bodies have diverged in two places (the binary branch differs — content endpoint returns 415, edit page renders the binary template) but the prelude is identical. Extract _load_file_for_editing(overlay_id, sub_path, user). Returns either a Flask Response (400/404/403/500) or a 5-tuple (overlay, target, content_or_None, is_binary, byte_count). Both routes call it and translate the tuple into their output shape: * overlay_file_content: 415 when is_binary, else jsonify text * overlay_file_edit_page: render binary template when is_binary, else render text template The previous _render_binary_editor helper folds back into the route since it was only called from there and benefits from inlining now that the prelude is shared. is_binary in the tuple is True both when is_editable() said no up front (8-KiB sniff) AND when the tail-of-file read raised UnicodeDecodeError — collapsing what was a duplicated try/except branch in each route. No behavior change. pytest still 580 passed, 1 skipped, 3 deselected. Step 12 (next) audits /files/content callers and likely deletes the endpoint since Phase B removed its only caller (legacy openEditorForFile via files-overlay.js); the shared helper would then collapse to single-caller status but remains useful for grouping the prelude into a named concept. Co-Authored-By: Claude Opus 4.7 (1M context) --- l4d2web/l4d2web/routes/files_routes.py | 124 +++++++++++++------------ 1 file changed, 63 insertions(+), 61 deletions(-) diff --git a/l4d2web/l4d2web/routes/files_routes.py b/l4d2web/l4d2web/routes/files_routes.py index 07dffd2..44a3c2c 100644 --- a/l4d2web/l4d2web/routes/files_routes.py +++ b/l4d2web/l4d2web/routes/files_routes.py @@ -201,6 +201,48 @@ def server_files_fragment(server_id: int): ) +def _load_file_for_editing(overlay_id: int, sub_path: str, user): + """Shared prelude for overlay_file_content + overlay_file_edit_page: + load the files-overlay, resolve sub_path safely, confirm it's a + regular file, attempt a UTF-8 read. + + Returns either: + * a 5-tuple (overlay, target, content, is_binary, byte_count) on + success — content is the UTF-8 text in text mode, None in + binary mode; byte_count is encoded text length or st_size + * a Flask Response on any failure: 400 (path traversal), 404 + (missing or non-file), 403 (system overlay, non-admin), 500 + (OS read error) + + is_binary is True both when is_editable(target) said no up front + (8-KiB sniff) and when the tail of the file failed UTF-8 decode + even though the sniff passed.""" + result = _load_files_overlay(overlay_id, user) + if isinstance(result, Response): + return result + overlay = result + + try: + target = safe_resolve_for_listing(overlay.path, sub_path) + except ValueError: + return Response("invalid path", status=400) + + if not target.exists() or not target.is_file(): + return Response(status=404) + + if not is_editable(target): + return overlay, target, None, True, target.stat().st_size + + try: + content = target.read_text(encoding="utf-8") + except OSError: + return Response("read failed", status=500) + except UnicodeDecodeError: + return overlay, target, None, True, target.stat().st_size + + return overlay, target, content, False, len(content.encode("utf-8")) + + @bp.get("/overlays//files/content") @require_login def overlay_file_content(overlay_id: int): @@ -209,29 +251,13 @@ def overlay_file_content(overlay_id: int): assert user is not None sub_path = request.args.get("path", "") - result = _load_files_overlay(overlay_id, user) + result = _load_file_for_editing(overlay_id, sub_path, user) if isinstance(result, Response): return result - overlay = result + _overlay, _target, content, is_binary, _byte_count = result - try: - target = safe_resolve_for_listing(overlay.path, sub_path) - except ValueError: - return Response("invalid path", status=400) - - if not target.exists() or not target.is_file(): - return Response(status=404) - if not is_editable(target): + if is_binary: return Response("not editable", status=415) - - try: - content = target.read_text(encoding="utf-8") - except OSError: - return Response("read failed", status=500) - except UnicodeDecodeError: - # is_editable sniffed only the first 8 KiB; the tail can still fail. - return Response("not editable", status=415) - return jsonify({"path": sub_path, "content": content}) @@ -240,42 +266,37 @@ def overlay_file_content(overlay_id: int): def overlay_file_edit_page(overlay_id: int): """Server-rendered editor page. Renders full-page by default or as a layoutless modal fragment when the HX-Modal header is set (see the - inject_base_layout context processor in app.py).""" + inject_base_layout context processor in app.py). Non-editable files + render the binary-replace branch instead of returning 415.""" user = current_user() assert user is not None sub_path = request.args.get("path", "") - result = _load_files_overlay(overlay_id, user) + result = _load_file_for_editing(overlay_id, sub_path, user) if isinstance(result, Response): return result - overlay = result + overlay, target, content, is_binary, byte_count = result - try: - target = safe_resolve_for_listing(overlay.path, sub_path) - except ValueError: - return Response("invalid path", status=400) - - if not target.exists() or not target.is_file(): - return Response(status=404) - - if not is_editable(target): - return _render_binary_editor(overlay, sub_path, target) - - try: - content = target.read_text(encoding="utf-8") - except OSError: - return Response("read failed", status=500) - except UnicodeDecodeError: - # is_editable's 8-KiB sniff said yes but the tail was binary. - # Fall back to the binary template path. - return _render_binary_editor(overlay, sub_path, target) + if is_binary: + mime, _enc = mimetypes.guess_type(target.name) + return render_template( + "overlay_file_editor.html", + overlay=overlay, + rel_path=sub_path, + content="", + byte_count=byte_count, + is_new=False, + is_binary=True, + at_folder="", + mime_type=mime or "application/octet-stream", + ) return render_template( "overlay_file_editor.html", overlay=overlay, rel_path=sub_path, content=content, - byte_count=len(content.encode("utf-8")), + byte_count=byte_count, is_new=False, is_binary=False, at_folder="", @@ -283,25 +304,6 @@ def overlay_file_edit_page(overlay_id: int): ) -def _render_binary_editor(overlay, sub_path, target): - """Render overlay_file_editor.html in binary-replace mode. Used by - overlay_file_edit_page when the target file isn't a UTF-8 text file. - The template hides the CM6 textarea + language dropdown and shows a - Replace zone + Download link; the save button reads 'Replace'.""" - mime, _enc = mimetypes.guess_type(target.name) - return render_template( - "overlay_file_editor.html", - overlay=overlay, - rel_path=sub_path, - content="", - byte_count=target.stat().st_size, - is_new=False, - is_binary=True, - at_folder="", - mime_type=mime or "application/octet-stream", - ) - - @bp.get("/overlays//files/new") @require_login def overlay_file_new_page(overlay_id: int):