DEV Community

Cover image for What My FastAPI CSV Upload Service Got Wrong: Four Bugs the Test Suite Never Caught
Marce
Marce

Posted on

What My FastAPI CSV Upload Service Got Wrong: Four Bugs the Test Suite Never Caught

I was testing the upload size limit when I sent a CSV file of about 6.4 MB to my FastAPI endpoint. I expected a 413 Request Entity Too Large response telling me the file exceeded the configured limit. Instead, I got a plain 500 Internal Server Error. My first thought was that the size check wasn’t working at all, but the check was in place. The actual problem was that the code trying to return the 413 response was broken.
That was the first of four problems I found in the upload path of a small CSV-cleaning service. The service is built with FastAPI and Pandas: it accepts CSV files through POST /api/v1/process-csv, cleans the data, and returns the results as JSON. I introduced it in my previous article; this time, I focus on what I found in the upload process. The four problems were a status constant that didn’t exist, no policy for encodings or binary files, a size check that ran after the whole file had been read, and a CORS configuration that was more permissive than it looked. For each one, I’ll show what went wrong, how I fixed it, and which tests I added. The original implementation is available in commit 358ad58.

1. A size limit that returned a 500

I traced the problem by following the code path for oversized uploads. This is the original check:

if file_size > MAX_FILE_SIZE_BITES:
    # (logging line omitted)
    raise HTTPException(
        status_code=status.HTTP_413_REQUEST_TOO_LARGE,
        detail=f"File size exceeds the maximum allowed limit of {MAX_FILE_SIZE_MB}MB."
    )
Enter fullscreen mode Exit fullscreen mode

The exception used status.HTTP_413_REQUEST_TOO_LARGE. I checked the status constants in the project’s Starlette version (0.36.3) and found that no such name existed. The correct constant was HTTP_413_REQUEST_ENTITY_TOO_LARGE. The row-limit check a few lines later already used the correct constant, which made the wrong name in the file-size check easier to overlook.

When Python reaches that line, it evaluates status.HTTP_413_REQUEST_TOO_LARGE to build the arguments for HTTPException. That name doesn’t exist in Starlette’s status module, so Python raises an AttributeError before the HTTPException is ever created. The service only registered a handler for HTTPException, so nothing caught the AttributeError. It bubbled up to Starlette’s default error handler, which returns a plain-text 500 Internal Server Error instead of the expected 413. I didn’t expect the cause to be a name that looked right but didn’t exist.

I fixed this by raising the service’s own ClientError. The handler now raises ClientError(413, ...), and a dedicated exception handler converts it into the API’s standard JSON error response.

I also added test_rejects_oversized_file_in_handler. It uses monkeypatch to lower MAX_FILE_SIZE_BYTES to 100, uploads a file larger than that, and checks that the response has status 413 and an error message mentioning the limit. This way, the rejection path can be tested without generating a multi-megabyte file.

Errors like this often go unnoticed because error-handling code rarely runs during normal requests. As long as users upload valid files, the endpoint works and the tests pass. The problem only appears when someone crosses a limit, and that’s exactly the scenario I forgot to test.

2. Encoding and binary files

The original implementation made a simple assumption: if a file had a .csv extension, it could be treated as UTF-8 text. This is how it parsed the upload:

buffer = io.BytesIO(contents)
df = pd.read_csv(buffer, nrows=MAX_ROWS + 1)
Enter fullscreen mode Exit fullscreen mode

To see what that meant in practice, I considered two inputs: a Latin-1 CSV containing characters such as é, and a binary file renamed with a .csv extension. The code passed the uploaded bytes straight to pd.read_csv() without specifying an encoding or checking whether the contents were text. The route only had specific handlers for EmptyDataError and ParserError, so any other exception, including a decoding error, reached the general except Exception block, which returned a 500. The application had no explicit policy for either case.

I fixed this by introducing decode_bytes(). It first rejects files containing NUL bytes, then attempts UTF-8 decoding (including files with a BOM), and falls back to Latin-1 if UTF-8 fails:

def decode_bytes(raw: bytes) -> tuple[str, str]:
    """Decode uploaded bytes. Tries UTF-8 first, then falls back to latin-1."""
    if b"\x00" in raw:
        raise ClientError(400, "The file looks binary, not like a text CSV.")
    try:
        return raw.decode("utf-8-sig"), "utf-8"
    except UnicodeDecodeError:
        return raw.decode("latin-1"), "latin-1 (fallback)"
Enter fullscreen mode Exit fullscreen mode

This gives the endpoint an explicit decoding policy instead of relying on Pandas’ defaults. A Latin-1 file is now accepted and reported as latin-1 (fallback), while a binary file gets a 400. Two regression tests cover these cases: test_latin1_file_is_decoded and test_rejects_binary_file_renamed_to_csv.

There are two limitations worth keeping in mind. Latin-1 can decode any byte sequence, so successfully decoding a file doesn’t guarantee that its contents are a valid CSV. And the NUL-byte check is a heuristic, not a real file-type detector: a binary file with no NUL bytes would get past it.

3. Reading the entire file before checking its size

The original code read the entire uploaded file into memory and only then compared its size with the limit:

try:
    contents = await file.read()
    file_size = len(contents)
Enter fullscreen mode Exit fullscreen mode

This meant the size limit was checked after the file had already been loaded. If someone uploaded a file much larger than the allowed limit, the application could consume memory before rejecting it. The check was there, but it happened too late to protect the application from the cost of reading the upload.

I addressed this with two separate checks. First, a middleware inspects the declared Content-Length header before the request reaches the endpoint, allowing an additional 64 KB for multipart overhead (the boundaries and headers that make the request slightly larger than the file itself):

if int(declared) > MAX_FILE_SIZE_BYTES + MULTIPART_OVERHEAD_BYTES:
    response = error_response(
        413, f"Request body exceeds the {MAX_FILE_SIZE_MB} MB limit."
    )
Enter fullscreen mode Exit fullscreen mode

This check only applies when the client sends a Content-Length header. Then, the endpoint reads only MAX_FILE_SIZE_BYTES + 1 bytes:

raw = file.file.read(MAX_FILE_SIZE_BYTES + 1)

if len(raw) > MAX_FILE_SIZE_BYTES:
    raise ClientError(413, f"File exceeds the {MAX_FILE_SIZE_MB} MB limit.")

Enter fullscreen mode Exit fullscreen mode

The extra byte is important: reading exactly the maximum would not tell us whether the file ended at that point or contained more data. One additional byte makes that distinction without loading the rest of the file. Each check has its own test, test_rejects_oversized_file_in_handler and test_rejects_oversized_request_in_middleware.

There is still an important deployment consideration: these checks do not replace an upload limit at the reverse proxy. By the time the endpoint runs, the multipart parser has already received the upload and may have stored it in a temporary file. For a public deployment, I would also configure an actual request-body limit in front of the application, such as client_max_body_size in Nginx.

4. CORS was not as restrictive as it looked

The original CORS configuration came with a comment saying it was restrictive:

#configuración de CORS restrictiva
ALLOWED_ORIGINS = os.getenv("ALLOWED_ORIGINIS", "*").split(",")
app.add_middleware(
    CORSMiddleware,
    allow_origins=ALLOWED_ORIGINS,
    allow_credentials=True,
    allow_methods=["GET", "POST"],
    allow_headers=["*"],
)
Enter fullscreen mode Exit fullscreen mode

It had two problems. First, it read the environment variable ALLOWED_ORIGINIS instead of ALLOWED_ORIGINS, so an operator setting the correctly spelled variable would have no effect. Second, the default was "*", while allow_credentials was set to True. In Starlette 0.36.3, this combination causes preflight responses to reflect the requesting origin in Access-Control-Allow-Origin rather than returning a wildcard, together with Access-Control-Allow-Credentials: true. In other words, an origin such as http://evil.com could be echoed in the response, with credentials allowed.

I fixed this by correcting the environment variable name, changing its default to an empty list, and installing CORSMiddleware only when allowed origins are explicitly configured. I also disabled credential support:

if ALLOWED_ORIGINS:
    app.add_middleware(
        CORSMiddleware,
        allow_origins=ALLOWED_ORIGINS,
        allow_credentials=False,
        allow_methods=["GET", "POST"],
        allow_headers=["Content-Type"],
    )
Enter fullscreen mode Exit fullscreen mode

The existing test, test_cors_is_closed_by_default, checks that an unconfigured origin does not receive an Access-Control-Allow-Origin header. There is no test yet for an explicitly configured origin.

One important distinction: CORS is a browser security mechanism, not authentication. It does not prevent scripts or other HTTP clients from calling the API directly.

The CORS configuration surprised me for a different reason. I had actually labeled it “restrictive” in a comment, but the wildcard default, enabled credentials, and misspelled environment variable told a very different story. It was a good reminder that comments describe what we intend the code to do, not necessarily what it actually does.

Tests and CI

The original test_main.py had only five tests: one for the health endpoint, one for a successful CSV upload, one for an invalid file extension, one for an empty file, and one for the security headers. They covered the happy path and a few basic validations, but did not cover file-size limits, row limits, character encodings, binary content, or CORS. I expanded the suite to 19 tests, adding cases that deliberately push the application beyond its limits instead of only checking that valid uploads work.

The CI workflow runs two jobs on pushes to main and pull requests. The test job installs requirements-dev.txt, runs pytest -q, and audits the production dependencies with pip-audit. The docker job builds the image, starts a container, and polls /health until it responds or the retry limit is reached. That second job matters because passing tests do not prove that the packaged service actually starts.

My first push exposed a small naming mistake: I had called the development requirements file requirements_dev.txt, but the workflow expected requirements-dev.txt, so I renamed it to match. I noticed the venv directory by accident. I ran git ls-files to check a filename, and the output was full of venv/ paths. I hadn’t realized my local virtual environment had been committed along with the actual project files. It was one of those mistakes that becomes obvious only when you take a closer look at what you’re pushing to the repository. I removed the tracked environment files and added a venv/ ignore rule. After a couple of fixes, the CI run passed both jobs, confirming that the tests and the container startup check worked in a clean environment.

What is still not solved

The service has no authentication or rate limiting, so anyone who can reach the endpoint can submit requests as often as they like. It also does not sanitize cell values against spreadsheet formulas.

clean_dataframe() trims whitespace, removes empty and duplicate rows, and infers numeric types, but it does not neutralize text that starts with =, +, - or @. That matters if the returned data is later exported to a spreadsheet. Memory is another concern: the size and row limits bound each request, but each one still reads the uploaded bytes, decodes them and builds a Pandas DataFrame, so many concurrent uploads add up. I have not measured a safe concurrency budget.

The outer upload limit belongs in the reverse proxy, as noted in section 3. The changes make the endpoint more deliberate when handling invalid input, but that does not make it production-ready on its own.

Repository: proyecto-fastapi-csv

Top comments (0)