-
Notifications
You must be signed in to change notification settings - Fork 575
Fix port validation to allow same port for different protocols (#992) #1000
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
Fix port validation to allow same port for different protocols (#992) #1000
Conversation
627dc96 to
cdf10b1
Compare
| @Test | ||
| func testPublishPortsSamePortDifferentProtocols() throws { | ||
| let result = try Parser.publishPorts([ | ||
| "8080:8080/tcp", |
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.
Can you add a test case where the protocol isn't chosen so we can verify the default (tcp)? e.g.
"8081:8081",
"8081:8081/udp"
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.
Sure
|
@iko1 Can you fix the build issues? |
I don't think the current build issue is related to my change. |
5a8d8ce to
d085d67
Compare
|
@iko1 it's definitely not you. There's some weirdness in the OSS header checks due to the new year, and to the CI builds merging PRs against main before the header checks. |
jglogan
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, thank you for the fix!
|
@iko1 it looks like you might need to push this again such that all commits have verified signatures |
d085d67 to
4d3c396
Compare
4d3c396 to
5112162
Compare
Type of Change
Motivation and Context
Fixes: #992
Port validation previously rejected valid configurations when the same port number was used for different protocols (TCP and UDP). For example:
-p 1024:1024/udp -p 1024:1024/tcpAlthough this is a valid and common use case, the validation logic treated it as a conflict.
To fix this, I updated the validation key to include the protocol name. The validation now checks for overlapping port numbers only within the same protocol, rather than across all protocols.
This change enables binding the same port number for both TCP and UDP, aligning the validation behavior with real-world networking requirements.
Testing