Skip to content

inspector,http: support builtin http request bodies#62915

Open
GrinZero wants to merge 1 commit intonodejs:mainfrom
GrinZero:fix-http-inspector-request-body
Open

inspector,http: support builtin http request bodies#62915
GrinZero wants to merge 1 commit intonodejs:mainfrom
GrinZero:fix-http-inspector-request-body

Conversation

@GrinZero
Copy link
Copy Markdown

Summary

This PR adds builtin http/https request-body support to network
inspection so Network.getRequestPostData works for text request bodies,
and keeps the existing rejection behavior for binary request bodies.

It also moves builtin http response-body tracking to a raw-byte hook
before IncomingMessage decoding, so response inspection remains correct
even when user code calls response.setEncoding(...).

This is intended to close part of the remaining postData gap tracked in
the network-inspection stabilization issue.

Problem

Builtin http/https network inspection currently emits
Network.requestWillBeSent, Network.responseReceived, and
Network.loadingFinished, but it does not emit request-body data for the
http client path. As a result, Network.getRequestPostData cannot return
POST data for builtin http/https requests.

In the tracking issue for stabilizing network inspection, builtin
http/https request postData is still marked as needing further
investigation. This change targets that specific gap.

While working on that, there was another important edge case to address on
the response side: listening to IncomingMessage 'data' events is not a
stable source of raw bytes, because response.setEncoding(...) changes the
chunks observed by userland from Buffer objects into strings. The
inspector protocol expects byte-oriented payloads for Network.dataReceived
and Network.dataSent, so reconstructing bytes from already-decoded
strings is only a best-effort fallback and can lose the original payload.

Approach

1. Reuse the existing request buffering pipeline

This change does not modify the CDP schema or the C++ buffering logic in
NetworkAgent.

Instead, it reuses the existing:

Network.dataSent(...) -> NetworkAgent::getRequestPostData(...)

pipeline that is already used by other transports.

2. Add builtin http request-body diagnostics events

The builtin http client now publishes:

  • http.client.request.bodyChunkSent
  • http.client.request.bodySent

The events are emitted from the ClientRequest write path, before HTTP
framing is applied, so the inspector sees the original user payload rather
than chunked-transfer framing bytes.

That makes the behavior consistent with the existing undici and http2
network-inspection implementations.

3. Capture builtin http response bytes before decoding

For responses, this PR intentionally avoids relying on
IncomingMessage.on('data') in network_http.js.

Instead, it adds:

  • http.client.response.bodyChunkReceived

from the HTTP parser body callback in _http_common.js.

That hook runs before Readable.setEncoding() transforms chunks for
userland, so the inspector always receives raw bytes. This avoids issues
such as:

  • missing dataLength / wrong event shape when user code receives strings
  • loss of byte-for-byte fidelity when decoded strings are re-encoded
  • protocol mismatches for Network.dataReceived

4. Why this is better than a string re-encoding compatibility layer

A temporary compatibility fix can convert string chunks back into
Buffers, but that is not equivalent to preserving the original bytes:

  • text decoding may already have normalized or replaced invalid sequences
  • binary responses observed after setEncoding() are no longer raw bytes
  • the inspector should reflect transport-level bytes, not a post-decoding
    reconstruction

So the final implementation moves the builtin http response path to the
same principle used by external tooling: capture bytes first, decode later
if needed.

This is also consistent with the general handling in
node-network-devtools, where network payload processing is built around
Buffer data rather than post-decoding string chunks.

Behavior

After this change:

  • builtin http and https POST requests with UTF-8 text bodies are
    available through Network.getRequestPostData
  • binary request bodies still reject with the existing inspector error
    behavior
  • builtin http response inspection continues to work even if user code
    calls response.setEncoding('utf8')

Tests

This PR adds and extends coverage in:

  • test/parallel/test-diagnostics-channel-http.js
  • test/parallel/test-inspector-network-http.js

The updated tests cover:

  • request body chunk and request body finished diagnostics events
  • text request bodies split across write() + end()
  • binary request bodies
  • http and https Network.getRequestPostData for text bodies
  • binary request-body rejection semantics
  • response inspection when the client calls response.setEncoding('utf8')

Verification

Verified locally with:

python3 tools/test.py \
  parallel/test-diagnostics-channel-http \
  parallel/test-inspector-network-http

Both tests passed locally with the built out/Release/node.

Refs: #53946

Signed-off-by: GrinZero <774933704@qq.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/inspector
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 23, 2026
@GrinZero
Copy link
Copy Markdown
Author

GrinZero commented Apr 23, 2026

image E2E Test:
const http = require("http");

const DEFAULT_TARGET_URL = "http://jsonplaceholder.typicode.com/posts";
const DEFAULT_PORT = Number(process.env.PORT || 3000);
const DEFAULT_HOST = "127.0.0.1";
const targetUrl = process.argv[2] || process.env.TARGET_URL || DEFAULT_TARGET_URL;
const defaultPayload = {
  title: "node inspect demo",
  body: "post payload from local trigger service",
  userId: 123,
};

if (!targetUrl.startsWith("http://")) {
  console.error(
    `[config] This script only uses the http module for outbound requests. Use an http:// URL, got: ${targetUrl}`
  );
  process.exit(1);
}

function readRequestBody(req) {
  return new Promise((resolve, reject) => {
    let body = "";

    req.setEncoding("utf8");
    req.on("data", (chunk) => {
      body += chunk;
    });
    req.on("end", () => {
      resolve(body);
    });
    req.on("error", reject);
  });
}

function buildPayload(rawBody) {
  if (!rawBody || !rawBody.trim()) {
    return defaultPayload;
  }

  const parsed = JSON.parse(rawBody);
  if (!parsed || Array.isArray(parsed) || typeof parsed !== "object") {
    throw new Error("payload must be a JSON object");
  }

  return {
    ...defaultPayload,
    ...parsed,
  };
}

function sendOutboundPost(url, payload) {
  return new Promise((resolve, reject) => {
    const body = JSON.stringify(payload);

    console.log(`\n[outbound] request -> POST ${url}`);
    console.log(`[outbound] request payload: ${body}`);

    const outboundReq = http.request(
      url,
      {
        method: "POST",
        headers: {
          "user-agent": "node-inspect-http-demo",
          accept: "application/json,text/plain,*/*",
          "content-type": "application/json; charset=utf-8",
          "content-length": Buffer.byteLength(body),
        },
      },
      (outboundRes) => {
        let responseBody = "";
        outboundRes.setEncoding("utf8");

        console.log(
          `[outbound] response <- ${outboundRes.statusCode} ${outboundRes.statusMessage || ""}`.trim()
        );
        console.log("[outbound] response headers:", outboundRes.headers);

        outboundRes.on("data", (chunk) => {
          responseBody += chunk;
        });

        outboundRes.on("end", () => {
          console.log("[outbound] body preview:");
          console.log(responseBody.slice(0, 300) || "<empty>");

          resolve({
            statusCode: outboundRes.statusCode,
            statusMessage: outboundRes.statusMessage,
            headers: outboundRes.headers,
            bodyPreview: responseBody.slice(0, 300),
          });
        });
      }
    );

    outboundReq.on("socket", (socket) => {
      socket.on("connect", () => {
        console.log(
          `[outbound] socket connected -> ${socket.remoteAddress}:${socket.remotePort}`
        );
      });
    });

    outboundReq.on("finish", () => {
      console.log("[outbound] request finished");
    });

    outboundReq.on("error", (error) => {
      console.error("[outbound] request error:", error.message);
      reject(error);
    });

    outboundReq.write(body);
    outboundReq.end();
  });
}

function writeJson(res, statusCode, payload) {
  const body = JSON.stringify(payload, null, 2);
  res.writeHead(statusCode, {
    "content-type": "application/json; charset=utf-8",
    "content-length": Buffer.byteLength(body),
  });
  res.end(body);
}

const server = http.createServer(async (req, res) => {
  const url = new URL(req.url || "/", `http://${req.headers.host || "127.0.0.1"}`);

  if (req.method === "GET" && url.pathname === "/") {
    return writeJson(res, 200, {
      ok: true,
      message: "local trigger service is running",
      endpoints: {
        trigger: "POST /trigger",
      },
      targetUrl,
      defaultPayload,
    });
  }

  if (req.method === "POST" && url.pathname === "/trigger") {
    console.log(`\n[inbound] trigger <- ${req.method} ${url.pathname}`);

    try {
      const rawBody = await readRequestBody(req);
      const payload = buildPayload(rawBody);
      const outbound = await sendOutboundPost(targetUrl, payload);

      return writeJson(res, 200, {
        ok: true,
        targetUrl,
        payload,
        outbound,
      });
    } catch (error) {
      return writeJson(res, 500, {
        ok: false,
        error: error.message,
      });
    }
  }

  return writeJson(res, 404, {
    ok: false,
    error: "not found",
  });
});

server.listen(DEFAULT_PORT, DEFAULT_HOST, () => {
  console.log(`[server] listening on http://${DEFAULT_HOST}:${DEFAULT_PORT}`);
  console.log(`[server] outbound target: ${targetUrl}`);
  console.log("[usage] node --inspect-wait --experimental-network-inspection inspect-http.js");
  console.log(
    `[usage] curl -X POST http://${DEFAULT_HOST}:${DEFAULT_PORT}/trigger -H 'content-type: application/json' -d '{"title":"manual trigger"}'`
  );
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants