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 an overridable socket_factory to dns.quic (like dns.query has) #1059

Closed
tykling opened this issue Feb 21, 2024 · 3 comments
Closed

Add an overridable socket_factory to dns.quic (like dns.query has) #1059

tykling opened this issue Feb 21, 2024 · 3 comments

Comments

@tykling
Copy link
Contributor

tykling commented Feb 21, 2024

Motivation
dns.query allows me to override the socket creation function for tcp, udp, doh by overriding socket_factory here:

socket_factory = socket.socket

This works great!

But the quic code uses socket.socket directly:

self._socket = socket.socket(self._af, socket.SOCK_DGRAM, 0)

Locally I am using a tiny 2 line patch to quic/sync.py which just changes the direct use of socket.socket to be use of socket_factory instead, just like in dns.query. I can then override socket_factory in my own code for proxyfication.

Describe the solution you'd like.
I would like to be able to override the socket used to easily use a proxy server with dnspython. The solution above works great for sync but something else/more is needed for the async cases. I haven't played around with async at all while testing this.

I can open a PR with the patch for _sync.py but I wanted to gauge the interest first.

Thanks!

@rthalley
Copy link
Owner

I'd be ok with adding a socket_factory to quic/_sync.py; I think it has to be there (rather than reusing query's socket_factory) for circularity reasons. I don't think you have to worry about async as it already abstracts socket creation with the make_socket() backend method, so if someone needed to change it they could subclass the backend.

@tykling
Copy link
Contributor Author

tykling commented Feb 21, 2024

Great! I've opened a small PR let me know if you need anything else, and thank you for the fast response :)

@rthalley
Copy link
Owner

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants