Skip to content

Conversation

agoose77
Copy link
Contributor

I performed a review of #592, but without admin privs the changes were only on my fork. This PR follows up #592 with my suggestions!

@agoose77 agoose77 force-pushed the connect_to_unix_socket branch from 3f0b937 to c5c081e Compare September 19, 2025 13:38
Comment on lines +400 to +405
} else if (target.protocol.startsWith("https")) {
proxyOptions.secure = true;
proxyOptions.agent = this.httpsAgent;
} else if (target.protocol.startsWith("http")) {
proxyOptions.secure = false;
proxyOptions.agent = this.httpAgent;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is more robust to close the set of protocols.

var proxyOptions = { target };

if (target.protocol.startsWith("unix")) {
// No need for agents for unix sockets
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment belongs here.

Comment on lines +19 to +22
proto = "http";
target = "unix+" + proto + "://" + unixSocketPath;
} else {
proto = sslOptions ? "https" : "http";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Force HTTP for unix

@minrk
Copy link
Member

minrk commented Sep 19, 2025

Great, thanks!

@minrk minrk merged commit 1a6094a into jupyterhub:main Sep 19, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants