-
Notifications
You must be signed in to change notification settings - Fork 44
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
Various minute improvements #30
Conversation
LGTM |
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, but next time don't forget to adjust the Copyright statement
@@ -49,6 +49,8 @@ def flow2pwn(flow): | |||
|
|||
for message in flow['flow']: | |||
data = base64.b64decode(message["b64"]) | |||
if finder := match(b'bd-executor: (.{50})', data): |
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, +1 for the 50 character limit to prevent DoS
@@ -49,6 +49,8 @@ def flow2pwn(flow): | |||
|
|||
for message in flow['flow']: | |||
data = base64.b64decode(message["b64"]) | |||
if finder := match(b'bd-executor: (.{50})', data): |
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.
meme
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 your first contribution!
I do feel however that your patch is lacking a little bit of documentation. Coukd you explain what bug you're trying to fix, and how you observed this bug? We at the Tulip Development Sweatshop really take feedback seriously, so unfortunately we can't accept patches that don't have any feedback attached with them.
Also, if you have the time, please take a look at #17, it looks to me like these are probably related.
Ping me once you got that covered, and I'll gladly merge!
Nice try ;) |
As the title states.