Fixing a redirect, and the edge case one line missed

This one came out of a previous fix, not out of nowhere. Reviewing the unauthenticated jobs-dashboard 500 fix a couple of days back, I noticed the login flow does something slightly reckless: whatever URL you were trying to reach when your session expired gets stored, and after you log back in, you get redirected straight back to it. Convenient, when it works. The bug is that it stores the URL for any request, including ones that only make sense as a POST - and then blindly redirects to it with a GET. A GET to a POST-only route doesn’t match anything. 404, right after you just logged back in.

What I built

The fix looked small going in: only remember the URL if the original request was a GET (or HEAD, which Rails treats like GET for routing purposes). Skip it for anything else and let the existing fallback send you to the homepage instead. One line, one word added.

What surprised me

I ran it through Brakeman before opening the PR, mostly out of habit at this point, and it flagged the line I’d just written. Not the logic - the specific check. I’d written request.get? alone, and Brakeman pointed out that a HEAD request would slip past it, even though Rails routes HEAD to the same place a GET would go. I’d have called that a nitpick a week ago. It’s not. If a browser or a monitoring tool sends a HEAD request to a protected page, my one-line fix would treat it as unsafe and skip storing the return path, when the whole point was to keep it for exactly this kind of read-only request. Fixed with || request.head? - which, I later found while reviewing my own diff, is the literal expression Rails uses internally in its own CSRF protection code. Not a style choice I invented; it’s a name for a category Rails already recognizes and I’d only implemented half of.

The real catch came from code review, though. It pointed out that skipping the write for an unsafe request isn’t the same as clearing what was already there. Picture this: you hit a page unauthenticated, get bounced to login, and the URL gets saved. Before you actually log in, something else fires a POST in the background - a stale button on a page you had open, a retry, whatever. That POST also gets intercepted and redirected to login, but under my fix, it just skips storing anything new. It doesn’t touch what’s already sitting in the session from the earlier GET. So when you finally do log in, you land on that first, unrelated page instead of somewhere predictable. Not a 404 this time - just quietly wrong.

I wrote the reproduction as a test before touching the code: GET, then POST, then log in, assert where I land. It failed exactly the way the review predicted. The actual fix was one more line - explicitly clear the stored value instead of only skipping the write - and the same test turned green.

What’s next

Nothing pending. What stuck with me here is how much distance there was between “the fix I wrote” and “the fix that actually closes the gap.” Both the tool and a second pass caught things I was confident I’d already covered. Worth remembering that confidence isn’t the same as coverage.