Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NO_PROXY test that ensures dot prefixes are considered a wildcard #6560

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qaisjp
Copy link

@qaisjp qaisjp commented Jan 8, 2025

Specifying .example.com instead of *.example.com is a valid way of specifying a wildcard, but this is not currently tested by DelegatingAgent.

Test plan

  • Adds unit tests only

Changelog

Comment on lines 153 to +156
// They shouldn't use the same agent
expect(noProxiedAgent).not.toBe(wildcardNoProxyAgent)
expect(noProxiedAgent).not.toBe(dotNoProxyAgent)
expect(wildcardNoProxyAgent).not.toBe(dotNoProxyAgent)
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure what the significance of this test is, I just cargo-culted it. Maybe it's possible for agent.connect to cache & return the same agent, so we want to make sure each proxy configuration returns a different agent?

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes, describe('DelegatingAgent caching')

@qaisjp qaisjp changed the title Fix NO_PROXY as dot prefixes are also considered a wildcard Add NO_PROXY test that ensures dot prefixes are considered a wildcard Jan 8, 2025
@qaisjp qaisjp marked this pull request as ready for review January 8, 2025 22:24
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.

2 participants