Skip to content

Commit 80e2976

Browse files
authored
fix: request with GET/HEAD method cannot have body. (#41)
1 parent 0ed71ef commit 80e2976

2 files changed

Lines changed: 120 additions & 17 deletions

File tree

src/adapters/node/request.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,103 @@ describe("getRequest", () => {
144144
JSON.stringify({ event: "user.created", data: { id: 1, name: "Test" } }),
145145
);
146146
});
147+
148+
it("should not include body for GET requests", async () => {
149+
const socket = new Socket();
150+
const req = new IncomingMessage(socket) as any;
151+
152+
req.url = "/api/users";
153+
req.method = "GET";
154+
req.headers = {
155+
host: "localhost:3000",
156+
"content-type": "application/json",
157+
"content-length": "20",
158+
};
159+
req.httpVersionMajor = 1;
160+
161+
// Mock the stream events
162+
req.on = vi.fn();
163+
req.pause = vi.fn();
164+
req.resume = vi.fn();
165+
req.destroy = vi.fn();
166+
req.destroyed = false;
167+
168+
const request = getRequest({
169+
request: req,
170+
base: "http://localhost:3000",
171+
});
172+
173+
expect(request).toBeInstanceOf(Request);
174+
expect(request.url).toBe("http://localhost:3000/api/users");
175+
expect(request.method).toBe("GET");
176+
177+
// Body should be null for GET requests
178+
expect(request.body).toBeNull();
179+
180+
// Stream handlers should not be set up for GET requests
181+
expect(req.on).not.toHaveBeenCalled();
182+
});
183+
184+
it("should not include body for HEAD requests", async () => {
185+
const socket = new Socket();
186+
const req = new IncomingMessage(socket) as any;
187+
188+
req.url = "/api/health";
189+
req.method = "HEAD";
190+
req.headers = {
191+
host: "localhost:3000",
192+
"content-type": "application/json",
193+
"content-length": "50",
194+
};
195+
req.httpVersionMajor = 1;
196+
197+
// Mock the stream events
198+
req.on = vi.fn();
199+
req.pause = vi.fn();
200+
req.resume = vi.fn();
201+
req.destroy = vi.fn();
202+
req.destroyed = false;
203+
204+
const request = getRequest({
205+
request: req,
206+
base: "http://localhost:3000",
207+
});
208+
209+
expect(request).toBeInstanceOf(Request);
210+
expect(request.url).toBe("http://localhost:3000/api/health");
211+
expect(request.method).toBe("HEAD");
212+
213+
// Body should be null for HEAD requests
214+
expect(request.body).toBeNull();
215+
216+
// Stream handlers should not be set up for HEAD requests
217+
expect(req.on).not.toHaveBeenCalled();
218+
});
219+
220+
it("should ignore pre-parsed body for GET requests", async () => {
221+
const socket = new Socket();
222+
const req = new IncomingMessage(socket) as any;
223+
224+
req.url = "/api/search";
225+
req.method = "GET";
226+
req.headers = {
227+
host: "localhost:3000",
228+
"content-type": "application/json",
229+
};
230+
231+
// Even if Express somehow attached a body to a GET request, it should be ignored
232+
req.body = { query: "test", limit: 10 };
233+
234+
const request = getRequest({
235+
request: req,
236+
base: "http://localhost:3000",
237+
});
238+
239+
expect(request).toBeInstanceOf(Request);
240+
expect(request.url).toBe("http://localhost:3000/api/search");
241+
expect(request.method).toBe("GET");
242+
243+
// Body should be null for GET requests even if req.body exists
244+
expect(request.body).toBeNull();
245+
});
147246
});

src/adapters/node/request.ts

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,23 +105,27 @@ export function getRequest({
105105
const maybeConsumedReq = request as any;
106106
let body = undefined;
107107

108-
// If body was already parsed by Express body-parser middleware
109-
if (maybeConsumedReq.body !== undefined) {
110-
// Convert parsed body back to a ReadableStream
111-
const bodyContent =
112-
typeof maybeConsumedReq.body === "string"
113-
? maybeConsumedReq.body
114-
: JSON.stringify(maybeConsumedReq.body);
115-
116-
body = new ReadableStream({
117-
start(controller) {
118-
controller.enqueue(new TextEncoder().encode(bodyContent));
119-
controller.close();
120-
},
121-
});
122-
} else {
123-
// Otherwise, get the raw body stream
124-
body = get_raw_body(request, bodySizeLimit);
108+
const method = request.method;
109+
// Request with GET/HEAD method cannot have body.
110+
if (method !== "GET" && method !== "HEAD") {
111+
// If body was already parsed by Express body-parser middleware
112+
if (maybeConsumedReq.body !== undefined) {
113+
// Convert parsed body back to a ReadableStream
114+
const bodyContent =
115+
typeof maybeConsumedReq.body === "string"
116+
? maybeConsumedReq.body
117+
: JSON.stringify(maybeConsumedReq.body);
118+
119+
body = new ReadableStream({
120+
start(controller) {
121+
controller.enqueue(new TextEncoder().encode(bodyContent));
122+
controller.close();
123+
},
124+
});
125+
} else {
126+
// Otherwise, get the raw body stream
127+
body = get_raw_body(request, bodySizeLimit);
128+
}
125129
}
126130

127131
return new Request(base + fullPath, {

0 commit comments

Comments
 (0)