Code Review of a Multi-File HTTP API: Wrong Method, Payload and Validation Bugs

In a one-hour code-review round you are handed several source files that together implement an HTTP API, and you are asked to find the bugs in them. You may either explain each bug aloud or edit the code to fix it. Some defects can be spotted at a glance, and they included at least these three kinds: - an endpoint declared as GET that should be POST; - a problem with the request payload; - missing input validation. Other, less obvious defects were present as well. The original files are not reproduced here, so this practice version asks you to show how you would find, explain and fix each of these kinds of defect across a multi-file API, and how you would run the review within the hour. ### Clarifying Questions - Should I aim to list every issue I can find, or to fix the most severe ones properly? - Can I run the code or its tests, or is this a read-only review? - Do existing clients already call these endpoints, so that changing a method or a payload shape would break them? - Which language and framework are the files written in, and how is the request body parsed? ### Part 1 — An endpoint that should be POST One endpoint is declared as GET but should be POST. Explain what concretely goes wrong while it stays GET, how you would confirm from the code that POST is the right method, and what you would change across the files so that the fix is complete. ```hint What GET promises Consider what HTTP lets browsers, proxies, crawlers and logs assume about a GET request, then read what this handler actually does. ``` #### What This Part Should Cover - The semantic contract of GET versus POST, and the concrete failures that breaking it causes here - The evidence in the handler that decides the method - A complete fix: the route, every caller, the tests and docs, and compatibility for existing callers ### Part 2 — The payload problem The request payload has a defect. Describe how you would trace a request's payload from the caller, through the route handler, to the model or service layer, which mismatches you would look for along the way, and how each would show up at runtime. ```hint Follow one field Pick a single field and follow its name, its type and where it travels (body, query string or path) through every file that touches it. ``` #### What This Part Should Cover - Where the handler reads the data from, compared with where the caller sends it - Field names, required fields, types and serialization agreeing across files - How each mismatch surfaces: a crash, a silent default, or wrong stored data - The response payload and status codes as part of the same contract ### Part 3 — Input validation The API does not validate its input. Where should validation live in this multi-file layout, what should it check, and what should the API return when a request fails it? ```hint Trust boundary Ask which file is the first to touch data from outside the service, and what every later file assumes about that data. ``` #### What This Part Should Cover - Validation at the boundary, before any business logic or write, using a declared schema rather than scattered checks - The kinds of checks needed, from presence and type to business rules and size limits - Error responses: status codes, field-level messages, and no leaked internals - Tests that pin the fix ### Part 4 — Running the review in an hour You have several files, an hour, and a choice between explaining and fixing. How do you decide what to read first, when to fix rather than describe, and how to report what you found? ```hint Read like a request Consider reading the files in the order a request flows through them, rather than one file at a time from top to bottom. ``` #### What This Part Should Cover - A reading order that builds a model of the API before hunting for bugs - Prioritization by severity - When to fix in code and when to describe, and how to verify a fix - Narrating each finding with its impact so the interviewer can follow ### What a Strong Answer Covers - Each bug explained by its impact on callers, data or security, not just named - Fixes that are complete across every file that shares the contract - Awareness of backward compatibility for existing clients - Steady prioritization within the hour, with lower-severity issues noted briefly - A scan for the less obvious defects once the obvious ones are handled ### Follow-up Questions - Old clients still send GET to this endpoint. How would you migrate them without keeping the bug alive? - After the switch to POST, a client retries on a timeout and creates a duplicate. How do you prevent that? - What would you add to the codebase or the CI pipeline so that this class of bug does not reach review again?