Stream large bodies warn with modify body #6514
Conversation
…racts with modify_body in Modify Body overview
…racts with modify_body in Streaming overview
…e import when modify_body in use
…e import when stream_large_bodies in use
…s option documentation
mitmproxy/addons/proxyserver.py
Outdated
| @@ -211,6 +213,7 @@ def configure(self, updated) -> None: | |||
| if "stream_large_bodies" in updated: | |||
| try: | |||
| human.parse_size(ctx.options.stream_large_bodies) | |||
| logging.log(ALERT, "Cannot modify streamed bodies.") | |||
There was a problem hiding this comment.
| logging.log(ALERT, "Cannot modify streamed bodies.") | |
| logging.log(ALERT, "Both modify_body and stream_large_bodies are active. Streamed bodies will not be modified.") |
(further suggestions welcome if you feel this is worse - I feel that directly linking this to the respective option names is useful here)
There was a problem hiding this comment.
I like linking to the option names. One minor question the message now says that modify_body is active, but we are not checking for its presence. Are we simply assuming that the body modification could happen via a def response lifecycle event or the modify_body option?
There was a problem hiding this comment.
Ah, thanks for catching this. We should definitely check if modify_body is set, and only then emit an alert! :)
|
Thanks for your suggestions. I overall agree and have made your changes with one clarifying question. |
…n_with_modify_body' into stream_large_bodies_warn_with_modify_body
|
See comment above, otherwise good to go. :) |
…dify_body and stream_large_bodies are set
…dify_body and stream_large_bodies are set. Needs to be checked in both addons as either option can be set first
|
I have added the check for both options. After some testing I saw that I need to check for both options being set in both addsons as the options can come in any order. |
Description
Added alert logging for stream_large_bodies and modify_body and enhance documentation around these options ( #6479 )
Checklist