-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Support all query types in DNS mode #6975
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
Conversation
|
How do I tell the tests to use the updated |
mitmproxy_rs's getaddrinfo to resolve domain namesmitmproxy_rs.getaddrinfo to resolve domain names
This currently requires us to ship a new release of mitmproxy_rs. There is an upper bound in https://github.com/mitmproxy/mitmproxy/blob/main/pyproject.toml#L45 though, so we can safely do that without affecting our users. |
|
Regarding this PR: Could we just expose a (We still need our getaddrinfo for mitmproxy.proxy.server - but the addon here we can maybe simplify dramatically 😃) |
This is available for UNIX only |
The docs are misleading here - it's available for Windows as well if you look at the sources. :) |
mitmproxy_rs.getaddrinfo to resolve domain namesmitmproxy_rs.DnsResolver to resolve domain names
mhils
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks! I'm curious whether we will run into any issues with overly long records or so, but for now this is a great way forward. 👍
I don't think we'll run into this issue practically since we're only handling A/AAAA records(they typically won't cross the UDP limit). That TODO is a remnant of the old code and theoretically speaking, we may have a response size greater than the UDP limit that would need to be handled. |
307d66e to
f649b1a
Compare
mitmproxy_rs.DnsResolver to resolve domain namese4d5e4c to
3c2e150
Compare
mhils
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % nits! 😃
|
#6975 (comment) (unresolved comment above), otherwise LGTM :) |
|
Sorry, messed up the commit history a bit while hopping between windows and linux. Should be fixed now |
|
No worries, this is why we squash! :) |
Description
Uses
mitmproxy_rs.DnsResolverto resolve domain names that need to be checked in the hosts file first and forwards any other type of query to the server specified in the optionsFixes #6739
Checklist
TODO: check and fix tests after we release latest version ofmitmproxy_rs