Skip to content

Commit

Permalink
Return correct HTTP status for disallowed HTTP methods
Browse files Browse the repository at this point in the history
Make sure that we return the correct HTTP status for disallowed
HTTP methods:

- For pages that do not exist, return 404.
- For pages that do exist but do not support the requested method,
  return 405.

A fact of life on the Internet is that public sites are constantly
being probed for vulnerabilities or misconfigurations of common
frameworks. These requests often attempt to POST or PUT to
irrelevant endpoints. Before this patch, we were logging something
like this for each such request:

```
2024-12-08T13:52:31.557Z	48395885-e0a7-4cda-a750-3d5267bb20d1	ERROR	Error: You made a POST request to "/wp-admin/admin-ajax.php" but did not provide an `action` for route "routes/$", so there is no way to handle the request.
    at sa (/var/task/index.cjs:49:10806)
    at u (/var/task/index.cjs:48:47306)
    at c (/var/task/index.cjs:48:46995)
    at Object.s [as query] (/var/task/index.cjs:48:46027)
    at Nvt (/var/task/index.cjs:64:17420)
    at /var/task/index.cjs:64:15716
    at /var/task/index.cjs:137:7577
    at Runtime.hOr [as handler] (/var/task/index.cjs:2326:74163)
2024-12-08T13:52:31.616Z	48395885-e0a7-4cda-a750-3d5267bb20d1	ERROR	Error: Invariant failed
    at Ln (/var/task/index.cjs:318:39742)
    at lm (/var/task/index.cjs:340:1264)
    at O2 (/var/task/index.cjs:340:1930)
    at Blr (/var/task/index.cjs:340:2261)
    at y8e (/var/task/index.cjs:192:3805)
    at rZ (/var/task/index.cjs:192:4617)
    at gu (/var/task/index.cjs:192:7257)
    at rZ (/var/task/index.cjs:192:6651)
    at gu (/var/task/index.cjs:192:7257)
    at rZ (/var/task/index.cjs:192:6651)
2024-12-08T13:52:31.619Z	48395885-e0a7-4cda-a750-3d5267bb20d1	ERROR	Error: Invariant failed
    at Ln (/var/task/index.cjs:318:39742)
    at lm (/var/task/index.cjs:340:1264)
    at O2 (/var/task/index.cjs:340:1930)
    at Blr (/var/task/index.cjs:340:2261)
    at y8e (/var/task/index.cjs:192:3805)
    at rZ (/var/task/index.cjs:192:4617)
    at gu (/var/task/index.cjs:192:7257)
    at rZ (/var/task/index.cjs:192:6651)
    at gu (/var/task/index.cjs:192:7257)
    at rZ (/var/task/index.cjs:192:6651)
```

Don't treat these as server errors and don't log them. Do return
the correct HTTP status.
  • Loading branch information
lpsinger committed Dec 9, 2024
1 parent f6dc9c9 commit 7ef038e
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 26 deletions.
2 changes: 1 addition & 1 deletion app/components/CircularsEmailAddress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useHostname } from '~/root'

export function useCircularsEmailAddress() {
let hostname = useHostname()
if (!hostname.endsWith('gcn.nasa.gov')) hostname = 'test.gcn.nasa.gov'
if (!hostname?.endsWith('gcn.nasa.gov')) hostname = 'test.gcn.nasa.gov'
return `circulars@${hostname}`
}

Expand Down
2 changes: 1 addition & 1 deletion app/components/NoticeFormat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function NoticeFormatInput({
onChange,
}: {
name: string
showJson: boolean
showJson?: boolean
value?: NoticeFormat
onChange?: (arg: NoticeFormat) => void
}) {
Expand Down
2 changes: 1 addition & 1 deletion app/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function getFormDataString(formData: FormData, key: string) {
}
}

export function getEnvBannerHeaderAndDescription(hostname: string) {
export function getEnvBannerHeaderAndDescription(hostname?: string) {
const production_hostname = 'gcn.nasa.gov'
let heading, description
if (hostname === `dev.${production_hostname}`) {
Expand Down
38 changes: 16 additions & 22 deletions app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,39 +138,31 @@ export function shouldRevalidate() {
}

function useLoaderDataRoot() {
const result = useRouteLoaderData<typeof loader>('root')
invariant(result)
return result
return useRouteLoaderData<typeof loader>('root')

Check warning on line 141 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L141

Added line #L141 was not covered by tests
}

export function useUserIdp() {
const { idp } = useLoaderDataRoot()
return idp
return useLoaderDataRoot()?.idp

Check warning on line 145 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L145

Added line #L145 was not covered by tests
}

export function useEmail() {
const { email } = useLoaderDataRoot()
return email
return useLoaderDataRoot()?.email

Check warning on line 149 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L149

Added line #L149 was not covered by tests
}

export function useName() {
const { name } = useLoaderDataRoot()
return name
return useLoaderDataRoot()?.name

Check warning on line 153 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L153

Added line #L153 was not covered by tests
}

export function useModStatus() {
const { userIsMod } = useLoaderDataRoot()
return userIsMod
return useLoaderDataRoot()?.userIsMod

Check warning on line 157 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L157

Added line #L157 was not covered by tests
}

export function useSubmitterStatus() {
const { userIsVerifiedSubmitter } = useLoaderDataRoot()
return userIsVerifiedSubmitter
return useLoaderDataRoot()?.userIsVerifiedSubmitter

Check warning on line 161 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L161

Added line #L161 was not covered by tests
}

export function useRecaptchaSiteKey() {
const { recaptchaSiteKey } = useLoaderDataRoot()
return recaptchaSiteKey
return useLoaderDataRoot()?.recaptchaSiteKey

Check warning on line 165 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L165

Added line #L165 was not covered by tests
}

/**
Expand All @@ -183,7 +175,7 @@ export function useRecaptchaSiteKey() {
*/
export function useFeature(feature: string) {
const featureUppercase = feature.toUpperCase()
return useLoaderDataRoot().features.includes(featureUppercase)
return useLoaderDataRoot()?.features.includes(featureUppercase)

Check warning on line 178 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L178

Added line #L178 was not covered by tests
}

export function WithFeature({
Expand All @@ -195,8 +187,13 @@ export function WithFeature({
return <>{useFeature(Object.keys(features)[0]) && children}</>
}

export function useOrigin() {
return useLoaderDataRoot()?.origin

Check warning on line 191 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L190-L191

Added lines #L190 - L191 were not covered by tests
}

export function useUrl() {
const { origin } = useLoaderDataRoot()
const origin = useOrigin()
invariant(origin)

Check warning on line 196 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L195-L196

Added lines #L195 - L196 were not covered by tests
const { pathname, search, hash } = useLocation()
const url = new URL(origin)
url.pathname = pathname
Expand All @@ -205,12 +202,9 @@ export function useUrl() {
return url.toString()
}

export function useOrigin() {
return useLoaderDataRoot().origin
}

export function useHostname() {
return new URL(useLoaderDataRoot().origin).hostname
const origin = useOrigin()

Check warning on line 206 in app/root.tsx

View check run for this annotation

Codecov / codecov/patch

app/root.tsx#L206

Added line #L206 was not covered by tests
if (origin) return new URL(origin).hostname
}

export function useDomain() {
Expand Down
6 changes: 5 additions & 1 deletion app/routes/$.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { SEOHandle } from '@nasa-gcn/remix-seo'
import type { LoaderFunction } from '@remix-run/node'
import type { ActionFunction, LoaderFunction } from '@remix-run/node'

import type { BreadcrumbHandle } from '~/root/Title'

Expand All @@ -12,4 +12,8 @@ export const loader: LoaderFunction = function () {
throw new Response(null, { status: 404 })
}

export const action: ActionFunction = function () {
throw new Response(null, { status: 404 })

Check warning on line 16 in app/routes/$.ts

View check run for this annotation

Codecov / codecov/patch

app/routes/$.ts#L15-L16

Added lines #L15 - L16 were not covered by tests
}

export default function () {}

0 comments on commit 7ef038e

Please sign in to comment.