http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

http: fix perf_hooks detail.req.url port and proxied path - #64311

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url
Jul 12, 2026
Merged

http: fix perf_hooks detail.req.url port and proxied path#64311
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
stefanobaghino:perf_hooks/fix-http-detail-url

Conversation

@stefanobaghino

Copy link
Copy Markdown
Contributor

The detail.req.url reported on http performance entries for HttpClient misrepresents the request URL in two ways: the port is dropped when it is non-default (http://localhost:3000/foo is reported as http://localhost/foo), and when the request path has been rewritten to absolute-form for HTTP proxying it is appended to the protocol and hostname again (http://localhosthttp://localhost:3000/foo).

The URL is now built from the connection authority captured at request creation (which carries the non-default port and IPv6 brackets, same as the auto-generated Host header), and when the path has been rewritten to absolute-form for proxying it is reported as-is, since it is already the full normalized URL.

Fixes: #59625

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added http Issues and PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 5, 2026

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
@stefanobaghino
stefanobaghinoforce-pushed the perf_hooks/fix-http-detail-url branch from f627005 to 9e4d4b0CompareJuly 6, 2026 16:20
@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Amended the commit to comply with the Signed-off-by requirement (and re-wrapped the message body at 72 columns), which is what made the commit message linting fail.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

🤔 The failure looks like an unrelated flaky test.

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Is there some way in which I can trigger CI myself? 🙂

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74664/ 💚

@stefanobaghino

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews, everyone! This looks ready to land whenever a collaborator has a chance. Very happy to help move it along in any way I can — rebasing, amending the commit, or anything else — just let me know what would be useful.

@jasnelljasnell added the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Jul 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 2d7fee0 into nodejs:mainJul 12, 2026
70 of 71 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 2d7fee0

aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
bartlomieju added a commit to badgerbees/deno that referenced this pull request Aug 5, 2026
Follow-up cleanups to the kAuthority/kProxyRewrittenToAbsolute port so it
matches nodejs/node#64311 more closely.
Initialize kProxyRewrittenToAbsolute next to kPath rather than after the
proxy rewrite block. Node can initialize it late because its rewrite
happens in rewriteForProxiedHttp() after construction, but here the
rewrite is inline in the constructor, so `x = x || false` was load-bearing
on statement order. Setting it before the rewrite drops that coupling.
Hoist posColon out of the bare block it was wrapped in and restore the
RFC 3986 comment from upstream.
Drop the `|| host` fallback in the perf entry. `host` always resolves to
at least "localhost", so kAuthority is never empty and the fallback was
dead - it only obscured that a caller-supplied Host header no longer
determines the reported URL, which is what Node does.
Apply the same fix to the OpenTelemetry client span, which built url.full
the same way and so carried the same proxy path duplication. This part has
no upstream counterpart; Node has no OTel instrumentation here.
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
The perf_hooks HTTP client entry built the reported URL from the bare
hostname, dropping non-default ports and IPv6 brackets, and appended
the request path even after it had been rewritten to absolute-form for
proxying, duplicating the protocol and authority.
Report the connection authority captured at request creation and, for
proxied requests, use the rewritten absolute-form target as-is.
Fixes: #59625
Signed-off-by: Stefano Baghino <stefano@baghino.me>
PR-URL: #64311
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues and PRs related to the http subsystem.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port missing from url in http perf_hooks detail

7 participants

@stefanobaghino@nodejs-github-bot@mcollina@jasnell@joyeecheung@theanarkh@richardlau