Check port parameter value in listen() in UDPServer and TCPServer - #120522
Open
NoNormalDev wants to merge 2 commits into
Open
Check port parameter value in listen() in UDPServer and TCPServer#120522NoNormalDev wants to merge 2 commits into
port parameter value in listen() in UDPServer and TCPServer#120522NoNormalDev wants to merge 2 commits into
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Member
It's solvable. See https://docs.godotengine.org/en/stable/engine_details/development/handling_compatibility_breakages.html (and the various API-break examples throughout the codebase). |
NoNormalDev
changed the title
port parameter value in listen(), in UDPServer and TCPServer to avoid integer underflow/overflowport parameter value in listen() in UDPServer and TCPServer
Calinou
reviewed
Jun 30, 2026
NoNormalDev
force-pushed
the
listenport
branch
3 times, most recently
from
July 1, 2026 10:50
0bbf2ee to
0399a77
Compare
NoNormalDev
marked this pull request as ready for review
July 1, 2026 10:50
Ivorforce
reviewed
Jul 1, 2026
Comment on lines
+4
to
+7
| Validate extension JSON: Error: Field 'classes/TCPServer/methods/listen/arguments/0': meta changed value in new API, from "uint16" to "int64". | ||
| Validate extension JSON: Error: Field 'classes/UDPServer/methods/listen/arguments/0': meta changed value in new API, from "uint16" to "int64". | ||
|
|
||
| Changed the "port" parameter type in TCPServer.listen() and UDPServer.listen() from uint16_t to int64_t. Now it checks if it lies in the range 0-65535, and returns an error if not. |
Member
There was a problem hiding this comment.
I think you need to provide compat methods, even though the underlying Variant type will be the same, because of ptrcall and call implications.
cc @dsnopek
Contributor
Author
There was a problem hiding this comment.
I've removed the compat methods based on https://chat.godotengine.org/channel/new-contributors?msg=aT4RZYy2hKmDX6AWd, which fixed the checks failing... Really unsure now.
Contributor
Author
There was a problem hiding this comment.
Added back the compat methods, and now .NET is complaining...
NoNormalDev
force-pushed
the
listenport
branch
from
August 23, 2026 10:26
0399a77 to
f1bf788
Compare
Collaborator
|
What's the reason for changing uint64_t to int64_t (which in turn forces us
to add a compatibility method), instead of just checking `< 65535`?
Ciao,
Fabio
…On Sun, Aug 23, 2026, 11:52 Dominik Jochymek ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In misc/extension_api_validation/4.7-stable/GH-120522.txt
<#120522 (comment)>:
> +Validate extension JSON: Error: Field 'classes/TCPServer/methods/listen/arguments/0': meta changed value in new API, from "uint16" to "int64".
+Validate extension JSON: Error: Field 'classes/UDPServer/methods/listen/arguments/0': meta changed value in new API, from "uint16" to "int64".
+
+Changed the "port" parameter type in TCPServer.listen() and UDPServer.listen() from uint16_t to int64_t. Now it checks if it lies in the range 0-65535, and returns an error if not.
Added back the compat methods, and now .NET is complaining...
—
Reply to this email directly, view it on GitHub
<#120522?email_source=notifications&email_token=AAM4C3RYIWIX2DEKIIVF55T5LLEFJA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBQGIZDCNBSGQY2M4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#discussion_r3838317919>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM4C3W6KKLD62NARQS3BTD5LLEFJAVCNFSNUABEKJSXA33TNF2G64TZHMYTKNRTGQ4TQMJ3JFZXG5LFHM2DOMJRGEZDKNBUG6QXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AAM4C3QXEIDTZ7F5SHME3S35LLEFJA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBQGIZDCNBSGQY2M4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AAM4C3UGDMNB775HMFOSZCT5LLEFJA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBQGIZDCNBSGQY2M4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Contributor
Author
Hi, well it's a change from |
Collaborator
|
Oh, my bad, I misread the diff, sorry
Ciao,
Fabio
…On Sun, Aug 23, 2026, 15:08 Dominik Jochymek ***@***.***> wrote:
*NoNormalDev* left a comment (godotengine/godot#120522)
<#120522 (comment)>
What's the reason for changing uint64_t to int64_t (which in turn forces
us to add a compatibility method), instead of just checking < 65535?
Ciao, Fabio
Hi, well it's a change from uint16, not uint64.
—
Reply to this email directly, view it on GitHub
<#120522?email_source=notifications&email_token=AAM4C3X7FPRW7M2I2VFZCWD5LL3F7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZYGY2DENJUGQ2KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5386425444>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM4C3QWKAJVMB2IPYQH3235LL3F7AVCNFSNUABEKJSXA33TNF2G64TZHMYTKNRTGQ4TQMJ3JFZXG5LFHM2DOMJRGEZDKNBUG6QXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AAM4C3UOQNDUAJCOJIF6E535LL3F7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZYGY2DENJUGQ2KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AAM4C3WPXW7ZPZ5KTT6O2Y35LL3F7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZYGY2DENJUGQ2KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because your review was requested.Message ID:
***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem(s) does this PR solve?
UDPServer.listen()andTCPServer.listen()accept port numbers outside of the 0-65535 range #120521Additional information
Changed the parameter type of
porttoint64_t(a 64-bit signed integer, the same type asintin GDScript).Performs a check if the value is in the range 0-65535 (inclusive).