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

Adding sockaddr #88

Merged
merged 2 commits into from
Sep 4, 2023
Merged

Adding sockaddr #88

merged 2 commits into from
Sep 4, 2023

Conversation

kazu-yamamoto
Copy link
Owner

My project (DNS over HTTP/2) requires to log a pair of socket addresses.

This provides my SockAddr and peer's SockAddr through Aux to Server.
Config has breaking changes.
If http2-tls is used, these changes are not visible.

Cc: @edsko @epoberezkin

@edsko
Copy link
Collaborator

edsko commented Sep 1, 2023

Thanks for the heads up! I haven't migrated to http2-tls yet, still using my own integration. It's on my list of things to do :)

@kazu-yamamoto kazu-yamamoto merged commit 46e6ae5 into master Sep 4, 2023
10 checks passed
@kazu-yamamoto kazu-yamamoto deleted the sockaddr branch September 4, 2023 01:48
@epoberezkin
Copy link

Found it :) Thanks for the heads up.

The problem is that by the time you have TLS the sockets are well hidden inside backend closures... Could these be Maybe, so they are optional?

We don't plan to move to http-tls as we do custom CA verification and there is another use case for a more precise control of TLS sessions... For now migrated to 4.1.4, but looking at some addressed vulnerability recently it would be great to move to the latest...

@kazu-yamamoto
Copy link
Owner Author

If SockAddr is not available, please specify a dummy one (e.g. SockAddrInet 0 0).

@epoberezkin
Copy link

yep, that's what I ended iup doing :)

Testing 4.2.2 now

@epoberezkin
Copy link

epoberezkin commented Nov 1, 2023

It fixed the problem with the exception in streaming not getting to the call! 🚀

Hopefully it'll fix some of the file sending errors we have

@kazu-yamamoto
Copy link
Owner Author

Great!

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.

3 participants