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 peer host and port info for server SslHandler (4.x) #5346

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

ben1222
Copy link
Contributor

@ben1222 ben1222 commented Oct 8, 2024

Fixes #5290 , on 4.x branch.

@vietj
Copy link
Member

vietj commented Oct 9, 2024

looks good but I'd prefer use vertx type HostAndPort which is more suited for this internally

@ben1222
Copy link
Contributor Author

ben1222 commented Oct 11, 2024

@vietj Hmm... I'm not sure if it worth to do this extra conversion... it adds overhead...
What's the benefit? Holding minimal required data? What else?

Anyway, I'll try to use HostAndPort and update PR for you to review.

@ben1222
Copy link
Contributor Author

ben1222 commented Oct 14, 2024

@vietj I updated the PR to use HostAndPort, please review again.

@ben1222
Copy link
Contributor Author

ben1222 commented Oct 15, 2024

Hmm... looks like the vertx gen failed... sorry that I didn't notice it when testing in my IDE... seems it didn't do the vertx gen...
I added @GenIgnore(GenIgnore.PERMITTED_TYPE) on HostAndPort.fromSocketAddress to fix that problem.
@vietj please review again.

@ben1222
Copy link
Contributor Author

ben1222 commented Oct 18, 2024

@vietj Could you review? If it looks good to you, I'll update the PR for master branch to also use the HostAndPort.

@ben1222
Copy link
Contributor Author

ben1222 commented Oct 23, 2024

@vietj I moved the conversion method to HttpUtils and updated PR, please review.

@vietj vietj merged commit 032de2b into eclipse-vertx:4.x Oct 24, 2024
7 checks passed
@vietj
Copy link
Member

vietj commented Oct 24, 2024

@ben1222 thank you, can you port the changes to master ?

@vietj vietj added the bug label Oct 24, 2024
@ben1222
Copy link
Contributor Author

ben1222 commented Oct 25, 2024

@vietj Thank you, I updated the PR for master branch (#5347), please review.
By the way, I'm wondering if 4.5.11 will be available before the end of November?

@vietj
Copy link
Member

vietj commented Oct 25, 2024

@ben1222 it sounds reasonnable to do early november a 4.5.11 release given that there is a fair amount of issues which have been fixed

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

Successfully merging this pull request may close these issues.

2 participants