Repository: remix-run/remix
Pull request #6820 (2023-07-12 20:02:48)

Fix bug with ?_data request on non-matching route
Closes https://github.com/remix-run/remix/pull/5738

changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 727752d79a7bd74430a273717ae1319e5cd03581

**The changes in this PR will be included in the next version bump.**

<details><summary>This PR includes changesets to release 18 packages</summary>

  | Name                          | Type  |
| ----------------------------- | ----- |
| @remix-run/server-runtime     | Patch |
| @remix-run/cloudflare         | Patch |
| @remix-run/deno               | Patch |
| @remix-run/dev                | Patch |
| @remix-run/node               | Patch |
| @remix-run/react              | Patch |
| @remix-run/cloudflare-pages   | Patch |
| @remix-run/cloudflare-workers | Patch |
| create-remix                  | Patch |
| @remix-run/architect          | Patch |
| @remix-run/express            | Patch |
| @remix-run/netlify            | Patch |
| @remix-run/serve              | Patch |
| @remix-run/testing            | Patch |
| @remix-run/vercel             | Patch |
| remix                         | Patch |
| @remix-run/css-bundle         | Patch |
| @remix-run/eslint-config      | Patch |

</details>

Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/remix-run/remix/new/brophdawg11/404-data-route?filename=.changeset/many-bikes-share.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fserve%22%3A%20patch%0A%22%40remix-run%2Fserver-runtime%22%3A%20patch%0A---%0A%0AFix%20bug%20with%20%3F_data%20request%20on%20non-matching%20route%0A)



Patch:
diff --git a/.changeset/shaggy-boats-explain.md b/.changeset/shaggy-boats-explain.md
new file mode 100644
index 000000000..75ea71c46
--- /dev/null
+++ b/.changeset/shaggy-boats-explain.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+Properly return a 404 for a `?_data` request that doesn't match routes
diff --git a/contributors.yml b/contributors.yml
index 753feecdc..8fb747d1f 100644
--- a/contributors.yml
+++ b/contributors.yml
@@ -559,3 +559,4 @@
 - ally1002
 - defjosiah
 - AlemTuzlak
+- ledniy
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index fc4e15485..1fa094616 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -82,10 +82,9 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
       );
 
       if (build.entry.module.handleDataRequest) {
-        let match = matches!.find((match) => match.route.id == routeId)!;
         response = await build.entry.module.handleDataRequest(response, {
           context: loadContext,
-          params: match ? match.params : {},
+          params: matches?.find((m) => m.route.id == routeId)?.params || {},
           request,
         });
       }


Test patch:
diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
index 4a0d4f91d..b550481ed 100644
--- a/packages/remix-server-runtime/__tests__/server-test.ts
+++ b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -412,7 +412,7 @@ describe("shared server runtime", () => {
   });
 
   describe("data requests", () => {
-    test("data request that does not match loader surfaces error for boundary", async () => {
+    test("data request that does not match loader surfaces 400 error for boundary", async () => {
       let build = mockServerBuild({
         root: {
           default: {},
@@ -434,6 +434,59 @@ describe("shared server runtime", () => {
       expect((await result.json()).message).toBeTruthy();
     });
 
+    test("data request that does not match routeId surfaces 403 error for boundary", async () => {
+      let build = mockServerBuild({
+        root: {
+          default: {},
+        },
+        "routes/index": {
+          parentId: "root",
+          index: true,
+          loader: () => null,
+        },
+      });
+      let handler = createRequestHandler(build, ServerMode.Test);
+
+      // This bug wasn't that the router wasn't returning a 404 (it was), but
+      // that we weren't defensive when looking at match.params when we went
+      // to call handleDataRequest(), - and that threw it's own uncaught
+      // exception triggering a 500.  We need to ensure that this build has a
+      // handleDataRequest implementation for this test to mean anything
+      expect(build.entry.module.handleDataRequest).toBeDefined();
+
+      let request = new Request(`${baseUrl}/?_data=routes/junk`, {
+        method: "get",
+      });
+
+      let result = await handler(request);
+      expect(result.status).toBe(403);
+      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect((await result.json()).message).toBeTruthy();
+    });
+
+    test("data request that does not match route surfaces 404 error for boundary", async () => {
+      let build = mockServerBuild({
+        root: {
+          default: {},
+        },
+        "routes/index": {
+          parentId: "root",
+          index: true,
+          loader: () => null,
+        },
+      });
+      let handler = createRequestHandler(build, ServerMode.Test);
+
+      let request = new Request(`${baseUrl}/junk?_data=routes/junk`, {
+        method: "get",
+      });
+
+      let result = await handler(request);
+      expect(result.status).toBe(404);
+      expect(result.headers.get("X-Remix-Error")).toBe("yes");
+      expect((await result.json()).message).toBeTruthy();
+    });
+
     test("data request calls loader", async () => {
       let rootLoader = jest.fn(() => {
         return "root";