-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 native websocket support #2350
base: dev
Are you sure you want to change the base?
Conversation
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.
Thank you for bringing this in! There are several things we need to consider before merging which I've mentioned inline.
Hi @frankli0324
|
||
Examples: | ||
|
||
>>> ws = wstube('wss://echo.websocket.events') |
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 we host a basic echo server ourselves in ci instead of relying on an external service? Those tend to be flakey and disturb the testing flow from time to time.
@@ -69,7 +69,7 @@ The table below shows which release corresponds to each branch, and what date th | |||
| [2.2.0](#220) | | Jan 5, 2015 | |||
|
|||
## 4.13.0 (`dev`) | |||
|
|||
- [#2350][2350] add native websocket support |
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.
Please add new entries to the bottom of the list
.. testsetup:: * | ||
|
||
from pwn import * | ||
from websocket import WebSocket, ABNF, WebSocketException, WebSocketTimeoutException |
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.
This isn't necessary since it's not used in the doc tests themselves
|
||
Arguments: | ||
url (str): The websocket server's URL to connect to. | ||
headers (dict): The same headers as the websocket protocol. |
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.
Aren't those additional headers to pass around?
How could we test this?
self.closed = False | ||
self.sock = WebSocket() | ||
self.url = url | ||
self.sock.connect(url, header=headers) |
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.
I think it is worth it to forward other useful arguments to the constructor like host
, origin
, cookie
, and subprotocols
. As well as the other ones.
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.
Handling the proxy configured in context.proxy
would be great too!
self.url = url | ||
self.sock.connect(url, header=headers) | ||
|
||
|
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.
Maybe add a magic __call__
handler to forward calls to the underlying web socket instance like ping()
Some __getattr__
forward to e.g. get the optional close reason of the remote after the server closed the connection would be cool too.
except: | ||
return False | ||
|
||
def close(self): |
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.
It should be possible to specify the web socket close reason
self.sock.connect(url, header=headers) | ||
|
||
|
||
def recv_raw(self, numb): |
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_recv_raw
needs to be implemented too to handle timeouts in the tube model.
Is anyone working on this? I hope it will be merged by the third 2024 Q3 at the latest. If there are no contributors working on it, I will try it. |
Add native support for websocket.
this file are derived from pwntools-tube-websocket by frankli0324 under the MIT License.
https://gist.github.com/frankli0324/795162a14be988a01e0efa0531f7ac5a