-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Description
What version of React Router are you using?
Steps to Reproduce
This is the _index.tsx page of a default Remix app. Clicking the button if running locally will trigger this issue. I have also included an example of the Link component because the issue appears to exist in the same place on different hosting provider such as AWS CloudFront. In AWS CloudFront it appears that empty requests are sent back as Content-Type: application/json which includes client-side navigation with Link components.
import { Link, useFetcher } from "@remix-run/react";
import { type ActionFunctionArgs } from "@remix-run/node";
export async function action({ request }: ActionFunctionArgs) {
const body = await request.formData();
if (request.method === "POST") {
return new Response(null, { headers: [["Content-Type", "application/json"]] });
}
return new Response(null, { status: 405, statusText: "Method Not Allowed" });
}
export default function Index() {
const action = useFetcher();
return (
<div>
{/* Link Issue Parsing Empty Response
---------------------------------
When using a Link component, if the response is an empty response from the server with
Content-Type: application/json an error will be thrown that the json could not parse correctly.
NOTE: The Link example will not reproduce locally because the Content-Type header will not be set to
application/json, but on other providers such as AWS CloudFront, empty responses have application/json set
by default! */}
<li>
<Link to="/non-existing-route">Will Cause Error</Link>
</li>
{/* Fetcher Issue Parsing Empty Response
------------------------------------
When using a fetcher, if an empty response is sent from the server with Content-Type: application/json
an error will be thrown that the json could not parse correctly. */}
<action.Form method="post">
<button type="submit">Fetcher Issue</button>
</action.Form>
</div>
);
}Expected Behavior
Issue Location
The issue comes from this line in the @remix-run/router package:
react-router/packages/router/router.ts
Line 4101 in e4f663c
| data = await result.json(); |
Proposed Solution
In this section, we are checking if the Content-Type: application/json exists, and then parsing for a JSON response. However, if the response body is null an error will be thrown. I believe adding a check for Content-Length: 0 before attempting to parse for JSON would be good.
Reason For Solution
I have looked in RFC#7231 and RFC#2616 and the way this package is parsing for the Content-Type: application/json certainly is correct - however a bit of a blindspot in the RFCs is how to deal appropriately with Empty responses:
- Some guidance is to not have a Body and not include a
Content-Typeheader - Some guidance is to check for the
Content-Length: 0header value. - Some guidance is to only use the
Content-Typevalue.
The RFCs also indicate that the Content-Type is an optional field. It SHOULD be sent but is not required as the client may attempt to parse the payload format on it's own. In this issue I have highlighted where I am recieving an issue from AWS CloudFront which seems to be relying on checking the Content-Length: 0 header.
For these reasons, I think it would be a good improvement to check the Content-Length: 0 even though the current implementation is valid.
Actual Behavior
Currently there is no check for an empty body before attempting to parse for JSON. Right now if the Content-Type: application/json header is sent then React Router/Remix will parse for JSON.