Fixes proxy logic in RegistryClient.#314
Merged
crosbymichael merged 2 commits intoapple:mainfrom Oct 9, 2025
Merged
Conversation
crosbymichael
requested changes
Oct 7, 2025
Contributor
crosbymichael
left a comment
crosbymichael
left a comment
There was a problem hiding this comment.
With all the changes it feels pretty leaky all over the codebase. Maybe we can chat on this one real-time and figure something out. I'll spend some cycles on it today.
jglogan
force-pushed
the
image-proxy
branch
2 times, most recently
from
94823fd to
c57fe5e
Compare
jglogan
changed the title
| // proxy configuration assumes all client requests will go to `base` URL | ||
| self.proxyURL = ProxyUtils.proxyFromEnvironment(scheme: scheme, host: host) | ||
| if let proxyURL = self.proxyURL, let proxyHost = proxyURL.host { | ||
| httpConfiguration.proxy = HTTPClient.Configuration.Proxy.server(host: proxyHost, port: proxyURL.port ?? 8080) |
Contributor
There was a problem hiding this comment.
is 8080 correct here or should it be 80?
Contributor
Author
There was a problem hiding this comment.
That should be 80 given that I changed other things to be go-like. I'll rebase and fix.
Originally that's what I thought curl used and I was doing curl-style semantics. But 8080 is wrong there as well – curl actually defaults to 1080 🤷 (though the docs aren't clear whether this applies to just socks proxies)
Contributor
Author
There was a problem hiding this comment.
443 for https scheme, 80 otherwise now
- Fixes ProxyUtils so that the environment variable to be used for proxy selection is determined by the request scheme. - RegistryClient uses ProxyUtils to get the proxy URL used by the HTTPClient. - Tweak hostname resolution error message to avoid misleading output if the proxy hostname cannot be resolved.
crosbymichael
approved these changes
Oct 9, 2025
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
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.
HTTP_PROXYbut not other standard variables. #255.