-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Added set_extra_headers() to WebSocketServer #40975
Conversation
Please merge this PR. I'm still using my revision on my server because I need sending CORS headers for my game to connect my server from itch.io. |
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.
See comments (const
usage and p_
parameters prefix).
Looks overall good but needs a rebase 👍 .
While the PR scope is okay, I strongly advise (STRONGLY!) to put an HTTP proxy in front of it to manage both the extra headers and requests sanitization, e.g. with nginx |
You mean using proxy for just sending additional headers? We can just send CORS headers when it is neccessary, why would we need to use a proxy? |
Should I resolve conflicts or you do that? |
I can't resolve the conflicts on your branch, please rebase and squash the commits, see here for instructions: |
Thank you @Faless I will resolve conflicts soon. |
As a bare minimum (it can offer more than that):
I.e. a basic form of protection against the dozens of malicious requests (port scanners/exploit checkers/etc) you'll receive every day and that will otherwise end up being consumed by godot (most likely at a higher cost, and resulting in more errors in the godot log unrelated to the actual game). |
Hiiiii @Faless I resolved conflicts with latest revision. |
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.
Good job 👍
See review comment on adding the override
keyword to set_extra_headers
when overriding.
The commits also needs to be squashed together https://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#the-interactive-rebase
Hiiii @Faless I squashed it into one commit. |
CI is failing: https://github.com/godotengine/godot/runs/5964150172?check_suite_focus=true . |
Meow.. I did it. |
I think CI will fail again, the doctool seem to not have worked properly 😿 . diff --git a/modules/websocket/doc_classes/WebSocketServer.xml b/modules/websocket/doc_classes/WebSocketServer.xml
index ef3279aac4..46b0274de3 100644
--- a/modules/websocket/doc_classes/WebSocketServer.xml
+++ b/modules/websocket/doc_classes/WebSocketServer.xml
@@ -60,6 +60,13 @@
If [code]false[/code] is passed instead (default), you must call [PacketPeer] functions ([code]put_packet[/code], [code]get_packet[/code], etc.), on the [WebSocketPeer] returned via [code]get_peer(id)[/code] to communicate with the peer with given [code]id[/code] (e.g. [code]get_peer(id).get_available_packet_count[/code]).
</description>
</method>
+ <method name="set_extra_headers">
+ <return type="void" />
+ <argument index="0" name="headers" type="PackedStringArray" default="PackedStringArray()" />
+ <description>
+ Sets additional headers to be sent to clients during the HTTP handshake.
+ </description>
+ </method>
<method name="stop">
<return type="void" />
<description> |
Done. I'm on Windows btw maybe about it? |
Thanks! 🐈 🏆 |
Cherry-picked for 3.5. |
Hi, I'm resending PR to
master
branch with changes.