Skip to content

destroy() should not follow redirects by default (can re-issue DELETE on the Location) #93

Open

Description

@brunofrank

Summary

FetchRequest defaults to redirect: "follow", including for the destroy shorthand. Following redirects after a DELETE is surprising and dangerous: if the server responds with 302 to another resource that also accepts DELETE, clients can end up destroying the redirect target instead of (or in addition to) the intended record.

This is the same class of footgun as Turbo Drive + Rails redirect_to without status: :see_other (rails/rails#45292).

Why this is a problem

A very common Rails pattern after destroying a nested resource is:

def destroy
  @comment.destroy
  redirect_to article_path(@article) # defaults to 302 Found
end

With Request.JS:

import { destroy } from "@rails/request.js"
await destroy(`/articles/${articleId}/comments/${commentId}`)
// redirect defaults to "follow"

Flow:

DELETE /articles/1/comments/2 → 302 Location: /articles/1

Fetch follows the redirect

Depending on browser / status code, the follow may reuse DELETE on /articles/1
That hits ArticlesController#destroy and deletes the parent (and often dependents)
We hit this in production-shaped nesting: removing an addon group from a product redirected to catalog_path, and a followed DELETE soft-deleted the entire catalog.

Minimal reproduction

Controller:

class CommentsController < ApplicationController
  def destroy
    @article = Article.find(params[:article_id])
    @comment = @article.comments.find(params[:id])
    @comment.destroy
    redirect_to article_path(@article) # 302
  end
end

JS:

import { destroy } from "@rails/request.js"
const response = await destroy(`/articles/${articleId}/comments/${commentId}`)
console.log(response.redirected, response.statusCode)
Expected

Only the comment is deleted
Either no automatic follow, or a follow that is always a GET of the article

Actual:

Request.JS follows the redirect by default
With a 302, some stacks re-request the Location with DELETE
The article (redirect target) can be destroyed
Server logs look like:

Started DELETE "/articles/1/comments/2"
Redirected to http://localhost:3000/articles/1
Completed 302 Found
Started DELETE "/articles/1"
Processing by ArticlesController#destroy

Current workaround

Callers must remember:

await destroy(url, { redirect: "manual" })

and/or servers must always:

redirect_to article_path(@article), status: :see_other # 303 → follow as GET

Both are easy to miss. destroy() is specifically for deletions; silently following redirects is a poor default for that verb.

Proposal

Default redirect: "manual" for destroy (and ideally put / patch too), or
Keep Fetch’s default but document loudly that destroy + Rails 302 redirects is unsafe, and show redirect: "manual" + status: :see_other in the README

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions