Skip to content

Commit

Permalink
Rename isIdempotent to isSafe (#883)
Browse files Browse the repository at this point in the history
The method name was a bit misleading, as what we're actually checking
for here is whether the method is [safe][0].

We can also now make use of this method in a couple of places where we
clear the snapshot cache after form submissions. The logic there is to
clear the cache when performing an operation that is unsafe.

[0]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
  • Loading branch information
kevinmcconnell authored Feb 28, 2023
1 parent 39affe5 commit 7bd2ce8
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 11 deletions.
10 changes: 5 additions & 5 deletions src/core/drive/form_submission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ export class FormSubmission {
return formEnctypeFromString(this.submitter?.getAttribute("formenctype") || this.formElement.enctype)
}

get isIdempotent() {
return this.fetchRequest.isIdempotent
get isSafe() {
return this.fetchRequest.isSafe
}

get stringFormData() {
Expand Down Expand Up @@ -151,7 +151,7 @@ export class FormSubmission {
// Fetch request delegate

prepareRequest(request: FetchRequest) {
if (!request.isIdempotent) {
if (!request.isSafe) {
const token = getCookieValue(getMetaContent("csrf-param")) || getMetaContent("csrf-token")
if (token) {
request.headers["X-CSRF-Token"] = token
Expand Down Expand Up @@ -239,11 +239,11 @@ export class FormSubmission {
}

requestMustRedirect(request: FetchRequest) {
return !request.isIdempotent && this.mustRedirect
return !request.isSafe && this.mustRedirect
}

requestAcceptsTurboStreamResponse(request: FetchRequest) {
return !request.isIdempotent || hasAttribute("data-turbo-stream", this.submitter, this.formElement)
return !request.isSafe || hasAttribute("data-turbo-stream", this.submitter, this.formElement)
}

get submitsWith() {
Expand Down
3 changes: 1 addition & 2 deletions src/core/drive/navigator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Action } from "../types"
import { getVisitAction } from "../../util"
import { FetchMethod } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { FormSubmission } from "./form_submission"
import { expandURL, getAnchor, getRequestURL, Locatable, locationIsVisitable } from "../url"
Expand Down Expand Up @@ -85,7 +84,7 @@ export class Navigator {
if (formSubmission == this.formSubmission) {
const responseHTML = await fetchResponse.responseHTML
if (responseHTML) {
const shouldCacheSnapshot = formSubmission.method == FetchMethod.get
const shouldCacheSnapshot = formSubmission.isSafe
if (!shouldCacheSnapshot) {
this.view.clearSnapshotCache()
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ export class FrameController
frame.delegate.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)
frame.delegate.loadResponse(response)

if (formSubmission.method !== FetchMethod.get) {
if (!formSubmission.isSafe) {
session.clearCache()
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/http/fetch_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class FetchRequest {
credentials: "same-origin",
headers: this.headers,
redirect: "follow",
body: this.isIdempotent ? null : this.body,
body: this.isSafe ? null : this.body,
signal: this.abortSignal,
referrer: this.delegate.referrer?.href,
}
Expand All @@ -156,8 +156,8 @@ export class FetchRequest {
}
}

get isIdempotent() {
return this.method == FetchMethod.get
get isSafe() {
return this.method === FetchMethod.get
}

get abortSignal() {
Expand Down

0 comments on commit 7bd2ce8

Please sign in to comment.