From df68994fc1501d25c4ba4768c59c15e2f8f0143e Mon Sep 17 00:00:00 2001 From: swapnil-nagar Date: Sun, 26 Jul 2026 20:53:48 -0700 Subject: [PATCH 1/3] Fix GET/HEAD request bodies failing before the handler --- src/http/HttpRequest.ts | 17 ++- src/http/HttpResponse.ts | 13 +- test/converters/toRpcHttp.test.ts | 12 ++ test/http/HttpRequest.test.ts | 219 ++++++++++++++++++++++++++++++ test/http/HttpResponse.test.ts | 96 +++++++++++++ 5 files changed, 350 insertions(+), 7 deletions(-) diff --git a/src/http/HttpRequest.ts b/src/http/HttpRequest.ts index f0b0d40..ad9c2b8 100644 --- a/src/http/HttpRequest.ts +++ b/src/http/HttpRequest.ts @@ -33,17 +33,24 @@ export class HttpRequest implements types.HttpRequest { let nativeReq = init.nativeRequest; if (!nativeReq) { const url = nonNullProp(init, 'url'); + const method = nonNullProp(init, 'method'); + // GET/HEAD requests cannot have a body. The global Request constructor throws if one is + // provided, so omit it for these methods to match the streaming path (createStreamRequest). + // See https://github.com/Azure/azure-functions-nodejs-library/issues/458 let body: Buffer | string | undefined; - if (init.body?.bytes) { - body = Buffer.from(init.body?.bytes); - } else if (init.body?.string) { - body = init.body.string; + const lowerMethod = method.toLowerCase(); + if (lowerMethod !== 'get' && lowerMethod !== 'head') { + if (init.body?.bytes) { + body = Buffer.from(init.body?.bytes); + } else if (init.body?.string) { + body = init.body.string; + } } nativeReq = new Request(url, { body, - method: nonNullProp(init, 'method'), + method, headers: fromNullableMapping(init.nullableHeaders, init.headers), }); } diff --git a/src/http/HttpResponse.ts b/src/http/HttpResponse.ts index 45acf56..ad98f39 100644 --- a/src/http/HttpResponse.ts +++ b/src/http/HttpResponse.ts @@ -11,6 +11,10 @@ interface InternalHttpResponseInit extends HttpResponseInit { nativeResponse?: Response; } +// Statuses that cannot have a body per the WHATWG fetch spec ("null body status"), limited to the +// valid Response status range (200-599). The global Response constructor throws if a body is provided. +const nullBodyStatuses = new Set([204, 205, 304]); + export class HttpResponse implements types.HttpResponse { readonly cookies: types.Cookie[]; readonly enableContentNegotiation: boolean; @@ -26,7 +30,12 @@ export class HttpResponse implements types.HttpResponse { this.#nativeRes = init.nativeResponse; } else { const resInit: ResponseInit = { status: init.status, headers: init.headers }; - if (isDefined(init.jsonBody)) { + // 204/205/304 responses cannot have a body. The global Response constructor throws if one + // is provided, so omit it to avoid crashing when converting a handler's response. This + // mirrors the GET/HEAD body handling in HttpRequest. + // See https://github.com/Azure/azure-functions-nodejs-library/issues/458 + const isNullBodyStatus = isDefined(init.status) && nullBodyStatuses.has(init.status); + if (isDefined(init.jsonBody) && !isNullBodyStatus) { // Response.json is not available in all versions, so we create it manually const jsonBody = JSON.stringify(init.jsonBody); const jsonHeaders = new Headers(resInit.headers); @@ -38,7 +47,7 @@ export class HttpResponse implements types.HttpResponse { // Cast to any to satisfy the native Response constructor // Our HttpResponseBodyInit type is compatible with what Node.js accepts // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - this.#nativeRes = new Response(init.body as any, resInit); + this.#nativeRes = new Response(isNullBodyStatus ? undefined : (init.body as any), resInit); } } diff --git a/test/converters/toRpcHttp.test.ts b/test/converters/toRpcHttp.test.ts index 2de39ce..9634b37 100644 --- a/test/converters/toRpcHttp.test.ts +++ b/test/converters/toRpcHttp.test.ts @@ -95,6 +95,18 @@ describe('toRpcHttp', () => { await expect(toRpcHttp('invocId', { status: {} })).to.eventually.be.rejectedWith(/status/i); }); + // https://github.com/Azure/azure-functions-nodejs-library/issues/458 (Response-side parallel): + // null-body statuses must not crash conversion when a handler returns a body. + it('status 204 with a body converts to an empty body', async () => { + const result = await toRpcHttp('invocId', { status: 204, body: 'dropped' }); + expect(result).to.deep.equal(getExpectedRpcHttp('', {}, '204')); + }); + + it('status 304 with a jsonBody converts to an empty body', async () => { + const result = await toRpcHttp('invocId', { status: 304, jsonBody: { a: 1 } }); + expect(result).to.deep.equal(getExpectedRpcHttp('', {}, '304')); + }); + it('headers object', async () => { const result = await toRpcHttp('invocId', { headers: { diff --git a/test/http/HttpRequest.test.ts b/test/http/HttpRequest.test.ts index 52ae8c5..d8fa48f 100644 --- a/test/http/HttpRequest.test.ts +++ b/test/http/HttpRequest.test.ts @@ -426,4 +426,223 @@ value2 } }); }); + + // Regression tests for https://github.com/Azure/azure-functions-nodejs-library/issues/458 + // The RPC (non-streaming) constructor must not pass a body to the WHATWG Request for GET/HEAD, + // otherwise it throws "Request with GET/HEAD method cannot have body." before the handler runs. + describe('GET/HEAD body handling', () => { + for (const method of ['GET', 'HEAD']) { + it(`does not throw and omits string body for ${method}`, async () => { + const req = new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { string: 'x' }, + }); + expect(req.body).to.be.null; + expect(await req.text()).to.equal(''); + }); + + it(`does not throw and omits bytes body for ${method}`, async () => { + const req = new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { bytes: Buffer.from('x') }, + }); + expect(req.body).to.be.null; + expect(await req.text()).to.equal(''); + }); + + it(`does not throw for zero-length bytes body for ${method}`, () => { + expect(() => { + new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { bytes: Buffer.from([]) }, + }); + }).to.not.throw(); + }); + } + + for (const method of ['get', 'Get', 'gEt', 'head', 'Head', 'hEaD']) { + it(`does not throw for mixed-case method "${method}" with a body`, () => { + expect(() => { + new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { string: 'x' }, + }); + }).to.not.throw(); + }); + } + + for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + it(`preserves string body for ${method}`, async () => { + const req = new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { string: 'preserved-body' }, + }); + expect(await req.text()).to.equal('preserved-body'); + }); + + it(`preserves bytes body for ${method}`, async () => { + const bodyContent = 'preserved-bytes-body'; + const req = new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { bytes: Buffer.from(bodyContent) }, + }); + expect(await req.text()).to.equal(bodyContent); + }); + } + }); + + // Edge cases around the native (WHATWG) Request the library now wraps after removing the + // standalone undici dependency. These lock in behavior that differs between undici versions. + describe('body and content parsing (native fetch edge cases)', () => { + it('parses a JSON string body via json()', async () => { + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/json', + body: { string: JSON.stringify({ a: 1, b: [2, 3] }) }, + headers: { 'content-type': 'application/json' }, + }); + expect(await req.json()).to.deep.equal({ a: 1, b: [2, 3] }); + }); + + it('reads a bytes body via arrayBuffer()', async () => { + const bytes = Buffer.from([0x00, 0x01, 0x02, 0xff]); + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/bin', + body: { bytes }, + }); + expect(Buffer.from(await req.arrayBuffer())).to.deep.equal(bytes); + }); + + it('reads a body via blob()', async () => { + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/blob', + body: { string: 'blob-body' }, + }); + const blob = await req.blob(); + expect(await blob.text()).to.equal('blob-body'); + }); + + it('prefers bytes over string when both are provided', async () => { + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/both', + body: { bytes: Buffer.from('from-bytes'), string: 'from-string' }, + }); + expect(await req.text()).to.equal('from-bytes'); + }); + + it('treats an empty-string body as no body', async () => { + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/empty', + body: { string: '' }, + }); + expect(req.body).to.be.null; + expect(await req.text()).to.equal(''); + }); + + it('exposes a readable body stream for non-empty bodies', async () => { + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/stream', + body: { string: 'streamed' }, + }); + const body = req.body; + expect(body).to.not.be.null; + if (body === null) { + throw new Error('expected a non-null body stream'); + } + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + for (;;) { + const { done, value } = await reader.read(); + if (done) { + break; + } + chunks.push(value); + } + expect(Buffer.concat(chunks).toString()).to.equal('streamed'); + }); + + it('marks bodyUsed after consuming the body', async () => { + const req = new HttpRequest({ + method: 'POST', + url: 'http://localhost:7071/api/used', + body: { string: 'consume-me' }, + }); + expect(req.bodyUsed).to.be.false; + await req.text(); + expect(req.bodyUsed).to.be.true; + }); + }); + + describe('headers, query, and method (native fetch edge cases)', () => { + it('normalizes known method casing to uppercase', () => { + const req = new HttpRequest({ + method: 'post', + url: 'http://localhost:7071/api/x', + body: { string: 'x' }, + }); + expect(req.method).to.equal('POST'); + }); + + it('preserves custom (non-standard) methods', () => { + const req = new HttpRequest({ + method: 'PURGE', + url: 'http://localhost:7071/api/x', + }); + expect(req.method).to.equal('PURGE'); + }); + + it('parses repeated query params from the URL when none are provided', () => { + const req = new HttpRequest({ + method: 'GET', + url: 'http://localhost:7071/api/x?a=1&b=2&a=3', + }); + expect(req.query.get('a')).to.equal('1'); + expect(req.query.getAll('a')).to.deep.equal(['1', '3']); + expect(req.query.get('b')).to.equal('2'); + }); + + it('explicit query replaces the URL query string', () => { + const req = new HttpRequest({ + method: 'GET', + url: 'http://localhost:7071/api/x?fromUrl=1', + query: { fromInit: 'yes' }, + }); + expect(req.query.get('fromInit')).to.equal('yes'); + expect(req.query.has('fromUrl')).to.be.false; + }); + + it('treats header names case-insensitively', () => { + const req = new HttpRequest({ + method: 'GET', + url: 'http://localhost:7071/api/x', + headers: { 'Content-Type': 'application/json' }, + }); + expect(req.headers.get('content-type')).to.equal('application/json'); + expect(req.headers.get('CONTENT-TYPE')).to.equal('application/json'); + }); + + it('converts null nullableHeaders values to empty-string headers', () => { + const req = new HttpRequest({ + method: 'GET', + url: 'http://localhost:7071/api/x', + nullableHeaders: { + present: { value: 'yes' }, + absent: { value: null }, + }, + }); + expect(req.headers.get('present')).to.equal('yes'); + expect(req.headers.get('absent')).to.equal(''); + }); + }); }); diff --git a/test/http/HttpResponse.test.ts b/test/http/HttpResponse.test.ts index 15cacad..1dc71dc 100644 --- a/test/http/HttpResponse.test.ts +++ b/test/http/HttpResponse.test.ts @@ -606,4 +606,100 @@ describe('HttpResponse', () => { expect(res.headers.get('X-Custom-Header')).to.equal('map-value'); }); }); + + // Edge cases around the native (WHATWG) Response the library now wraps after removing the + // standalone undici dependency. The newer global Response is stricter than older undici versions. + describe('status and null-body handling (native fetch edge cases)', () => { + it('defaults status to 200 when not provided', () => { + const res = new HttpResponse({ body: 'x' }); + expect(res.status).to.equal(200); + }); + + it('uses the provided status', () => { + const res = new HttpResponse({ status: 201, body: 'created' }); + expect(res.status).to.equal(201); + }); + + // Regression parallel to https://github.com/Azure/azure-functions-nodejs-library/issues/458: + // 204/205/304 responses cannot have a body, and the global Response constructor throws if one + // is provided. The library must omit the body rather than turn the handler's response into a 500. + for (const status of [204, 205, 304]) { + it(`omits a string body for null-body status ${status} instead of throwing`, async () => { + const res = new HttpResponse({ status, body: 'should-be-dropped' }); + expect(res.status).to.equal(status); + expect(res.body).to.be.null; + expect(await res.text()).to.equal(''); + }); + + it(`omits a jsonBody for null-body status ${status} instead of throwing`, () => { + const res = new HttpResponse({ status, jsonBody: { dropped: true } }); + expect(res.status).to.equal(status); + expect(res.body).to.be.null; + }); + + it(`clone preserves null-body status ${status} with no body`, () => { + const res = new HttpResponse({ status, body: 'x' }); + const cloned = res.clone(); + expect(cloned.status).to.equal(status); + expect(cloned.body).to.be.null; + }); + } + + it('still preserves the body for regular statuses', async () => { + const res = new HttpResponse({ status: 200, body: 'ok' }); + expect(await res.text()).to.equal('ok'); + }); + + // The global Response constructor rejects statuses outside 200-599 with a RangeError. + // Documented here so the constraint is visible. + for (const status of [100, 199, 600, 1000]) { + it(`throws a RangeError for out-of-range status ${status}`, () => { + expect(() => new HttpResponse({ status, body: 'x' })).to.throw(RangeError); + }); + } + }); + + describe('jsonBody edge cases', () => { + it('serializes falsy-but-defined jsonBody (0)', async () => { + const res = new HttpResponse({ jsonBody: 0 }); + expect(res.headers.get('content-type')).to.equal('application/json'); + const clone = res.clone(); + expect(await res.text()).to.equal('0'); + expect(await clone.json()).to.equal(0); + }); + + it('serializes falsy-but-defined jsonBody (false)', async () => { + const res = new HttpResponse({ jsonBody: false }); + expect(await res.text()).to.equal('false'); + }); + + it('serializes falsy-but-defined jsonBody (empty string)', async () => { + const res = new HttpResponse({ jsonBody: '' }); + expect(res.headers.get('content-type')).to.equal('application/json'); + expect(await res.text()).to.equal('""'); + }); + + it('treats null jsonBody as no body without forcing a content-type', () => { + const res = new HttpResponse({ jsonBody: null }); + expect(res.body).to.be.null; + expect(res.headers.get('content-type')).to.be.null; + }); + + it('does not override an explicit content-type for jsonBody', async () => { + const res = new HttpResponse({ + jsonBody: { a: 1 }, + headers: { 'content-type': 'application/problem+json' }, + }); + expect(res.headers.get('content-type')).to.equal('application/problem+json'); + expect(await res.json()).to.deep.equal({ a: 1 }); + }); + + it('respects an explicit content-type regardless of casing', () => { + const res = new HttpResponse({ + jsonBody: { a: 1 }, + headers: { 'Content-Type': 'text/plain' }, + }); + expect(res.headers.get('content-type')).to.equal('text/plain'); + }); + }); }); From f3e6cdb608f7cc4fa8e0d486e5f360dc18352592 Mon Sep 17 00:00:00 2001 From: swapnil-nagar Date: Sun, 26 Jul 2026 21:05:58 -0700 Subject: [PATCH 2/3] Updating Library version --- package-lock.json | 4 ++-- package.json | 2 +- src/constants.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index a011964..6affc51 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@azure/functions", - "version": "4.16.2", + "version": "4.16.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@azure/functions", - "version": "4.16.2", + "version": "4.16.3", "license": "MIT", "dependencies": { "@azure/functions-extensions-base": "0.3.0", diff --git a/package.json b/package.json index 464a815..713f6aa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@azure/functions", - "version": "4.16.2", + "version": "4.16.3", "description": "Microsoft Azure Functions NodeJS Framework", "keywords": [ "azure", diff --git a/src/constants.ts b/src/constants.ts index 768de58..bd1f439 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,6 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. -export const version = '4.16.2'; +export const version = '4.16.3'; export const returnBindingKey = '$return'; From cec3edd18d7c7a8cbab280ae30b95f00c36127fc Mon Sep 17 00:00:00 2001 From: swapnil-nagar Date: Mon, 27 Jul 2026 10:43:30 -0700 Subject: [PATCH 3/3] Code review comments and adding log --- src/http/HttpRequest.ts | 8 ++++ test/converters/toRpcHttp.test.ts | 5 +++ test/http/HttpRequest.test.ts | 65 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/src/http/HttpRequest.ts b/src/http/HttpRequest.ts index ad9c2b8..dbcf20d 100644 --- a/src/http/HttpRequest.ts +++ b/src/http/HttpRequest.ts @@ -13,6 +13,7 @@ import { fromNullableMapping } from '../converters/fromRpcNullable'; import { fromRpcTypedData } from '../converters/fromRpcTypedData'; import { AzFuncSystemError } from '../errors'; import { isDefined, nonNullProp } from '../utils/nonNull'; +import { workerSystemLog } from '../utils/workerSystemLog'; import { extractHttpUserFromHeaders } from './extractHttpUserFromHeaders'; interface InternalHttpRequestInit extends RpcHttpData { @@ -46,6 +47,13 @@ export class HttpRequest implements types.HttpRequest { } else if (init.body?.string) { body = init.body.string; } + } else if (init.body?.bytes?.length || init.body?.string) { + // The incoming GET/HEAD request has a body, which the global Request constructor forbids. + // We drop it to avoid crashing, but log it so the behavior isn't silent and is easier to debug. + workerSystemLog( + 'warning', + `Discarding the body of the incoming ${method} request because GET and HEAD requests cannot have a body.` + ); } nativeReq = new Request(url, { diff --git a/test/converters/toRpcHttp.test.ts b/test/converters/toRpcHttp.test.ts index 9634b37..b8321b3 100644 --- a/test/converters/toRpcHttp.test.ts +++ b/test/converters/toRpcHttp.test.ts @@ -102,6 +102,11 @@ describe('toRpcHttp', () => { expect(result).to.deep.equal(getExpectedRpcHttp('', {}, '204')); }); + it('status 205 with a body converts to an empty body', async () => { + const result = await toRpcHttp('invocId', { status: 205, body: 'dropped' }); + expect(result).to.deep.equal(getExpectedRpcHttp('', {}, '205')); + }); + it('status 304 with a jsonBody converts to an empty body', async () => { const result = await toRpcHttp('invocId', { status: 304, jsonBody: { a: 1 } }); expect(result).to.deep.equal(getExpectedRpcHttp('', {}, '304')); diff --git a/test/http/HttpRequest.test.ts b/test/http/HttpRequest.test.ts index d8fa48f..b7463b2 100644 --- a/test/http/HttpRequest.test.ts +++ b/test/http/HttpRequest.test.ts @@ -5,7 +5,9 @@ import 'mocha'; import * as chai from 'chai'; import { expect } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; +import * as sinon from 'sinon'; import { HttpRequest } from '../../src/http/HttpRequest'; +import * as workerLogModule from '../../src/utils/workerSystemLog'; chai.use(chaiAsPromised); @@ -431,6 +433,18 @@ value2 // The RPC (non-streaming) constructor must not pass a body to the WHATWG Request for GET/HEAD, // otherwise it throws "Request with GET/HEAD method cannot have body." before the handler runs. describe('GET/HEAD body handling', () => { + let workerSystemLogStub: sinon.SinonStub; + + beforeEach(() => { + // Stub the system logger so the discard warning (issue #458) doesn't spam test output, + // and so the logging tests below can assert on it. + workerSystemLogStub = sinon.stub(workerLogModule, 'workerSystemLog'); + }); + + afterEach(() => { + sinon.restore(); + }); + for (const method of ['GET', 'HEAD']) { it(`does not throw and omits string body for ${method}`, async () => { const req = new HttpRequest({ @@ -495,6 +509,57 @@ value2 expect(await req.text()).to.equal(bodyContent); }); } + + // The body is discarded for GET/HEAD, but that must not be silent (issue #458): it is logged + // as a warning so a dropped body is discoverable when debugging. + for (const method of ['GET', 'HEAD']) { + it(`logs a warning when a ${method} request carries a string body`, () => { + new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { string: 'discard me' }, + }); + expect(workerSystemLogStub.calledOnce).to.be.true; + expect(workerSystemLogStub.firstCall.args[0]).to.equal('warning'); + expect(workerSystemLogStub.firstCall.args[1]).to.include(method); + expect(workerSystemLogStub.firstCall.args[1]).to.match(/discard/i); + }); + + it(`logs a warning when a ${method} request carries a bytes body`, () => { + new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { bytes: Buffer.from('discard me') }, + }); + expect(workerSystemLogStub.calledOnce).to.be.true; + expect(workerSystemLogStub.firstCall.args[0]).to.equal('warning'); + }); + + it(`does not log when a ${method} request has no body`, () => { + new HttpRequest({ method, url: 'https://example.test/api/probe' }); + expect(workerSystemLogStub.called).to.be.false; + }); + + it(`does not log when a ${method} request has an empty bytes body`, () => { + new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { bytes: Buffer.from([]) }, + }); + expect(workerSystemLogStub.called).to.be.false; + }); + } + + for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + it(`does not log a discard warning when a ${method} request carries a body`, () => { + new HttpRequest({ + method, + url: 'https://example.test/api/probe', + body: { string: 'keep me' }, + }); + expect(workerSystemLogStub.called).to.be.false; + }); + } }); // Edge cases around the native (WHATWG) Request the library now wraps after removing the