Fix bug with ?_data request on non-matching route (#6820)

Co-authored-by: Vlad Goldman <ledniy@gmail.com>

.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

contributors.yml

@@ -559,3 +559,4 @@
 - ally1002
 - defjosiah
 - AlemTuzlak
+- ledniy

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";

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,
         });
       }