Fix proxying to unix socket #617
Open
+62
−31
Conversation
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
- no need to add request's url to the request path. This already exists and leads to duplication. - fix custom error routes with unix socket by setting target hostname to localhost.
dometto
force-pushed
the
fix_unix_sockets
branch
from
8e381f6 to
ea3d808
Compare
for more information, see https://pre-commit.ci
minrk
reviewed
Dec 5, 2025
Member
minrk
left a comment
minrk
left a comment
There was a problem hiding this comment.
other than one question about changing the logging config in tests, looks great, thank you!
| options = options || {}; | ||
| options.authToken = "secret"; | ||
| options.log = defaultLogger({ level: "error" }); | ||
| //options.log = defaultLogger({ level: "error" }); |
Member
There was a problem hiding this comment.
Can this be restored? Or was there a reason it was removed/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
While testing the implementation of #592 a bit more thoroughly in combination with JupyterHub, I noticed two issues:
errorTargetoption was not working correctly with unix socket/foowere being transformed into/foo/foo.Error target not working correctly
Was seeing
invalid urlerrors because unix socket paths, e.g.%2Ftmp%2Ftest.sock, was being passed as a URL. Added logic to rewrite the hostname tolocalhost. I also added a test of theerrorTargetfunctionality with unix sockets.URLs with subpaths
I traced the issue to this line in the
http-proxylibrary.Basically,
targetin this library'sproxyOptsForTargetfunction already contains path info. By also settingtarget.pathnameinproxyOptsForTarget, the result is that the existingpathandpathnameare joined together byhttp-proxy.The fix is to remove the additional setting of
target.pathnameinproxyOptsForTarget. This also means we can refactorproxyOptsForTargetto not require thereqUrlargument.I've modified the existing test with unix socket to use a subpath.