Skip to content

Conversation

@yashvardhan-kukreja
Copy link
Contributor

Signed-off-by: Yashvardhan Kukreja [email protected]

Description

A clear and concise description of the features you're adding in this pull request.

Tests

Please describe any tests you've added. If you've added no tests, or left important behavior untested, please explain why not.

Additional context

Add any other context about the problem you're solving.

Metadata

  • Fixes #[Link to Issue]

@yashvardhan-kukreja yashvardhan-kukreja requested a review from a team as a code owner October 17, 2025 14:11
@yashvardhan-kukreja yashvardhan-kukreja marked this pull request as draft October 17, 2025 14:11
@yashvardhan-kukreja yashvardhan-kukreja marked this pull request as ready for review October 21, 2025 19:50
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2025

Codecov Report

❌ Patch coverage is 54.54545% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.58%. Comparing base (8370510) to head (fcf50b7).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
proxyd/backend.go 61.11% 6 Missing and 1 partial ⚠️
proxyd/proxyd.go 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #493      +/-   ##
==========================================
+ Coverage   58.51%   58.58%   +0.07%     
==========================================
  Files          91       91              
  Lines       12934    12962      +28     
==========================================
+ Hits         7568     7594      +26     
- Misses       4912     4914       +2     
  Partials      454      454              
Flag Coverage Δ
proxyd 72.08% <54.54%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
proxyd/config.go 54.76% <ø> (ø)
proxyd/server.go 79.97% <100.00%> (+0.35%) ⬆️
proxyd/proxyd.go 57.28% <0.00%> (-0.36%) ⬇️
proxyd/backend.go 82.15% <61.11%> (+0.28%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Yashvardhan Kukreja <[email protected]>
Signed-off-by: Yashvardhan Kukreja <[email protected]>
Signed-off-by: Yashvardhan Kukreja <[email protected]>
@yashvardhan-kukreja yashvardhan-kukreja force-pushed the yash/proxyd-security-fixes branch from 0ac6d65 to f1a2610 Compare October 23, 2025 19:53
if err != nil {
return nil, wrapErr(err, "error dialing backend")
}
backendConn.SetReadLimit(readLimit)

Choose a reason for hiding this comment

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

It could be interesting to leverage b.maxResponseSize which is used for the equivalent limit in regular http reqs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Was originally intending to match the limits of the front-end of the connection (clientConn) with the backend's one as s.maxBodySize, but yeah we can re-use the b.maxResponseSize and configure b.maxResponseSize to s.maxBodySize whenever need be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe we can fall back to server's maxBodySize if the b.maxResponseSize isn't provided.

…limit for proxying ws connection

Signed-off-by: Yashvardhan Kukreja <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants