Skip to content

Commit 7909ef4

Browse files
authored
Merge pull request #352 from RhysSullivan/rs/openapi-encode-non-json-bodies
openapi: encode non-JSON request bodies correctly
2 parents f6e0adf + ec2ebb0 commit 7909ef4

2 files changed

Lines changed: 193 additions & 3 deletions

File tree

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// ---------------------------------------------------------------------------
2+
// Regression test for non-JSON request-body serialization.
3+
//
4+
// Before the fix, the invoke path only had two branches — JSON, or
5+
// `String(bodyValue)` with whatever content-type the spec declared. For an
6+
// object body that meant shipping the literal string `[object Object]`
7+
// with `Content-Type: application/x-www-form-urlencoded`, which servers
8+
// reject or hold open waiting for valid framing.
9+
//
10+
// Now we dispatch on content-type: form-urlencoded → bodyUrlParams,
11+
// multipart → bodyFormDataRecord, string passthrough for pre-serialized
12+
// bodies, JSON.stringify as a last-resort fallback (never `[object Object]`).
13+
// ---------------------------------------------------------------------------
14+
15+
import { describe, expect, it } from "@effect/vitest";
16+
import { Effect } from "effect";
17+
import { FetchHttpClient } from "@effect/platform";
18+
import { createServer } from "node:http";
19+
import type { AddressInfo } from "node:net";
20+
21+
import {
22+
createExecutor,
23+
definePlugin,
24+
makeTestConfig,
25+
type InvokeOptions,
26+
type SecretProvider,
27+
} from "@executor/sdk";
28+
29+
import { openApiPlugin } from "./plugin";
30+
31+
const autoApprove: InvokeOptions = { onElicitation: "accept-all" };
32+
const TEST_SCOPE = "test-scope";
33+
34+
const memoryProvider: SecretProvider = (() => {
35+
const store = new Map<string, string>();
36+
return {
37+
key: "memory",
38+
writable: true,
39+
get: (id, scope) =>
40+
Effect.sync(() => store.get(`${scope}\u0000${id}`) ?? null),
41+
set: (id, value, scope) =>
42+
Effect.sync(() => {
43+
store.set(`${scope}\u0000${id}`, value);
44+
}),
45+
delete: (id, scope) =>
46+
Effect.sync(() => store.delete(`${scope}\u0000${id}`)),
47+
list: () => Effect.sync(() => []),
48+
};
49+
})();
50+
51+
const memorySecretsPlugin = definePlugin(() => ({
52+
id: "memory-secrets" as const,
53+
storage: () => ({}),
54+
secretProviders: [memoryProvider],
55+
}));
56+
57+
type Captured = {
58+
contentType: string;
59+
body: string;
60+
};
61+
62+
const startEchoServer = () =>
63+
Effect.acquireRelease(
64+
Effect.async<{ baseUrl: string; captured: Captured; close: () => void }>(
65+
(resume) => {
66+
const captured: Captured = { contentType: "", body: "" };
67+
const server = createServer((req, res) => {
68+
const chunks: Buffer[] = [];
69+
req.on("data", (c: Buffer) => chunks.push(c));
70+
req.on("end", () => {
71+
captured.contentType = req.headers["content-type"] ?? "";
72+
captured.body = Buffer.concat(chunks).toString("utf8");
73+
res.writeHead(200, { "content-type": "application/json" });
74+
res.end(JSON.stringify({ ok: true }));
75+
});
76+
});
77+
server.listen(0, "127.0.0.1", () => {
78+
const port = (server.address() as AddressInfo).port;
79+
resume(
80+
Effect.succeed({
81+
baseUrl: `http://127.0.0.1:${port}`,
82+
captured,
83+
close: () => server.close(),
84+
}),
85+
);
86+
});
87+
},
88+
),
89+
(s) => Effect.sync(() => s.close()),
90+
);
91+
92+
const formSpec = JSON.stringify({
93+
openapi: "3.0.0",
94+
info: { title: "FormTest", version: "1.0.0" },
95+
paths: {
96+
"/submit": {
97+
post: {
98+
operationId: "submit",
99+
tags: ["forms"],
100+
requestBody: {
101+
required: true,
102+
content: {
103+
"application/x-www-form-urlencoded": {
104+
schema: {
105+
type: "object",
106+
properties: {
107+
name: { type: "string" },
108+
email: { type: "string" },
109+
},
110+
},
111+
},
112+
},
113+
},
114+
responses: {
115+
"200": {
116+
description: "ok",
117+
content: {
118+
"application/json": {
119+
schema: {
120+
type: "object",
121+
properties: { ok: { type: "boolean" } },
122+
},
123+
},
124+
},
125+
},
126+
},
127+
},
128+
},
129+
},
130+
});
131+
132+
describe("OpenAPI non-JSON request body serialization", () => {
133+
it.scoped(
134+
"form-urlencoded object body is properly encoded (no '[object Object]')",
135+
() =>
136+
Effect.gen(function* () {
137+
const { baseUrl, captured } = yield* startEchoServer();
138+
139+
const executor = yield* createExecutor(
140+
makeTestConfig({
141+
plugins: [
142+
openApiPlugin({ httpClientLayer: FetchHttpClient.layer }),
143+
memorySecretsPlugin(),
144+
] as const,
145+
}),
146+
);
147+
148+
yield* executor.openapi.addSpec({
149+
spec: formSpec,
150+
scope: TEST_SCOPE,
151+
namespace: "form",
152+
baseUrl,
153+
});
154+
155+
yield* executor.tools.invoke(
156+
"form.forms.submit",
157+
{ body: { name: "Acme", email: "a@b.com" } },
158+
autoApprove,
159+
);
160+
161+
expect(captured.contentType).toBe("application/x-www-form-urlencoded");
162+
expect(captured.body).not.toBe("[object Object]");
163+
164+
const parsed = new URLSearchParams(captured.body);
165+
expect(parsed.get("name")).toBe("Acme");
166+
expect(parsed.get("email")).toBe("a@b.com");
167+
}),
168+
);
169+
});

‎packages/plugins/openapi/src/sdk/invoke.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,23 @@ const applyHeaders = (
158158
// Response helpers
159159
// ---------------------------------------------------------------------------
160160

161+
const normalizeContentType = (ct: string | null | undefined): string =>
162+
ct?.split(";")[0]?.trim().toLowerCase() ?? "";
163+
161164
const isJsonContentType = (ct: string | null | undefined): boolean => {
162-
if (!ct) return false;
163-
const normalized = ct.split(";")[0]?.trim().toLowerCase() ?? "";
165+
const normalized = normalizeContentType(ct);
166+
if (!normalized) return false;
164167
return (
165168
normalized === "application/json" || normalized.includes("+json") || normalized.includes("json")
166169
);
167170
};
168171

172+
const isFormUrlEncoded = (ct: string | null | undefined): boolean =>
173+
normalizeContentType(ct) === "application/x-www-form-urlencoded";
174+
175+
const isMultipartFormData = (ct: string | null | undefined): boolean =>
176+
normalizeContentType(ct).startsWith("multipart/form-data");
177+
169178
// ---------------------------------------------------------------------------
170179
// Public API — invoke a single operation
171180
// ---------------------------------------------------------------------------
@@ -211,8 +220,20 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
211220
if (bodyValue !== undefined) {
212221
if (isJsonContentType(rb.contentType)) {
213222
request = HttpClientRequest.bodyUnsafeJson(request, bodyValue);
223+
} else if (typeof bodyValue === "string") {
224+
request = HttpClientRequest.bodyText(request, bodyValue, rb.contentType);
225+
} else if (isFormUrlEncoded(rb.contentType)) {
226+
request = HttpClientRequest.bodyUrlParams(
227+
request,
228+
bodyValue as Parameters<typeof HttpClientRequest.bodyUrlParams>[1],
229+
);
230+
} else if (isMultipartFormData(rb.contentType)) {
231+
request = HttpClientRequest.bodyFormDataRecord(
232+
request,
233+
bodyValue as Parameters<typeof HttpClientRequest.bodyFormDataRecord>[1],
234+
);
214235
} else {
215-
request = HttpClientRequest.bodyText(request, String(bodyValue), rb.contentType);
236+
request = HttpClientRequest.bodyText(request, JSON.stringify(bodyValue), rb.contentType);
216237
}
217238
}
218239
}

0 commit comments

Comments
 (0)