From 5b22e921fd815d1bfdfad3adc516a17cb4e74a3a Mon Sep 17 00:00:00 2001 From: Joe Farrelly Date: Tue, 30 Jun 2026 23:58:09 +0100 Subject: [PATCH] Fix 500 on .lua file upload from closed-file write perform_update closed the uploaded file's stream via a context manager before handing it to the storage backend, which then failed trying to re-read it. Validate and normalise the content directly, then save it as a fresh ContentFile instead of reusing the original upload stream. --- apicore/views.py | 19 ++++++++----------- tests/test_views.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/apicore/views.py b/apicore/views.py index b48bbd4..1cca3d0 100644 --- a/apicore/views.py +++ b/apicore/views.py @@ -9,6 +9,7 @@ import environ import requests from django.core.cache import cache +from django.core.files.base import ContentFile from django.db import models from django.utils import timezone from requests.adapters import HTTPAdapter @@ -175,18 +176,14 @@ def perform_update(self, serializer): logger.warning("Rejected upload: file too large (%d bytes)", file.size) return - file.name = user_id + ".lua" - with file.open("r+") as f: - content = f.read().decode("utf-8") + content = file.read().decode("utf-8") - if "FazzToolsScraperDB" not in content[0:25]: - logger.warning("Rejected upload: invalid file header") - return + if "FazzToolsScraperDB" not in content[0:25]: + logger.warning("Rejected upload: invalid file header") + return - normalised = re.sub(r'(\r\n|\r|\n)(?=(?:[^"]*"[^"]*")*[^"]*$)', r"\n", content) - f.seek(0) - f.write(normalised.encode()) - f.truncate() + normalised = re.sub(r'(\r\n|\r|\n)(?=(?:[^"]*"[^"]*")*[^"]*$)', r"\n", content) + normalised_file = ContentFile(normalised.encode(), name=user_id + ".lua") user_obj = ProfileUser.objects.get(user_id=user_id) update_date = user_obj.user_last_update @@ -197,7 +194,7 @@ def perform_update(self, serializer): except OSError as exc: logger.warning("Could not remove old file: %s", exc) - serializer.save(user_id=user_id, user_file=file, user_last_update=update_date) + serializer.save(user_id=user_id, user_file=normalised_file, user_last_update=update_date) cache.delete(f"userfile:{user_id}") def list(self, request): diff --git a/tests/test_views.py b/tests/test_views.py index 7553615..0b21c33 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -6,9 +6,13 @@ """ import pytest +from django.core.files.uploadedfile import SimpleUploadedFile from django.test import Client +from django.test.client import BOUNDARY, MULTIPART_CONTENT, encode_multipart +from django.utils import timezone from apicore.libs.expansion_order import tier_sort_key +from apicore.models import ProfileUser # --------------------------------------------------------------------------- # Pure helper: tier_sort_key @@ -124,6 +128,34 @@ def test_header_page_for_unknown_user_returns_empty(self): assert resp.status_code == 200 assert resp.json() == [] + def test_upload_does_not_500(self): + """Regression test: uploads used to crash perform_update with 'I/O operation + on closed file' because a `with file.open("r+")` block closed the upload + stream's underlying BytesIO before it was handed to the storage backend — + Django's InMemoryUploadedFile doesn't override File.close().""" + user_id = "u1" + ProfileUser.objects.create(user_id=user_id, user_file="", user_last_update=timezone.now()) + c = self._authed_client(user_id) + + body = "FazzToolsScraperDB = {\n}\n" + upload = SimpleUploadedFile( + "FazzToolsScraper.lua", body.encode(), content_type="text/plain" + ) + + resp = c.put( + f"/api/profile/users/{user_id}/", + data=encode_multipart( + BOUNDARY, + { + "user_id": user_id, + "user_file": upload, + "user_last_update": timezone.now().isoformat(), + }, + ), + content_type=MULTIPART_CONTENT, + ) + assert resp.status_code == 200 + # --------------------------------------------------------------------------- # ProfileAltView — IsSessionUser enforcement