inspector,http: support builtin http request bodies#62915
Open
GrinZero wants to merge 1 commit intonodejs:mainfrom
Open
inspector,http: support builtin http request bodies#62915GrinZero wants to merge 1 commit intonodejs:mainfrom
GrinZero wants to merge 1 commit intonodejs:mainfrom
Conversation
Signed-off-by: GrinZero <774933704@qq.com>
Collaborator
|
Review requested:
|
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
This PR adds builtin
http/httpsrequest-body support to networkinspection so
Network.getRequestPostDataworks for text request bodies,and keeps the existing rejection behavior for binary request bodies.
It also moves builtin
httpresponse-body tracking to a raw-byte hookbefore
IncomingMessagedecoding, so response inspection remains correcteven when user code calls
response.setEncoding(...).This is intended to close part of the remaining
postDatagap tracked inthe network-inspection stabilization issue.
Problem
Builtin
http/httpsnetwork inspection currently emitsNetwork.requestWillBeSent,Network.responseReceived, andNetwork.loadingFinished, but it does not emit request-body data for thehttpclient path. As a result,Network.getRequestPostDatacannot returnPOST data for builtin
http/httpsrequests.In the tracking issue for stabilizing network inspection, builtin
http/httpsrequestpostDatais still marked as needing furtherinvestigation. 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 astable source of raw bytes, because
response.setEncoding(...)changes thechunks observed by userland from
Bufferobjects into strings. Theinspector protocol expects byte-oriented payloads for
Network.dataReceivedand
Network.dataSent, so reconstructing bytes from already-decodedstrings 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
httprequest-body diagnostics eventsThe builtin
httpclient now publishes:http.client.request.bodyChunkSenthttp.client.request.bodySentThe events are emitted from the
ClientRequestwrite path, before HTTPframing is applied, so the inspector sees the original user payload rather
than chunked-transfer framing bytes.
That makes the behavior consistent with the existing
undiciandhttp2network-inspection implementations.
3. Capture builtin
httpresponse bytes before decodingFor responses, this PR intentionally avoids relying on
IncomingMessage.on('data')innetwork_http.js.Instead, it adds:
http.client.response.bodyChunkReceivedfrom the HTTP parser body callback in
_http_common.js.That hook runs before
Readable.setEncoding()transforms chunks foruserland, so the inspector always receives raw bytes. This avoids issues
such as:
dataLength/ wrong event shape when user code receives stringsNetwork.dataReceived4. 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:setEncoding()are no longer raw bytesreconstruction
So the final implementation moves the builtin
httpresponse path to thesame 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 aroundBufferdata rather than post-decoding string chunks.Behavior
After this change:
httpandhttpsPOST requests with UTF-8 text bodies areavailable through
Network.getRequestPostDatabehavior
httpresponse inspection continues to work even if user codecalls
response.setEncoding('utf8')Tests
This PR adds and extends coverage in:
test/parallel/test-diagnostics-channel-http.jstest/parallel/test-inspector-network-http.jsThe updated tests cover:
write()+end()httpandhttpsNetwork.getRequestPostDatafor text bodiesresponse.setEncoding('utf8')Verification
Verified locally with:
Both tests passed locally with the built
out/Release/node.Refs: #53946