Skip to content
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

first draft of read har. #6190

Merged
merged 20 commits into from
Jun 30, 2023
Merged

first draft of read har. #6190

merged 20 commits into from
Jun 30, 2023

Conversation

stanleygvi
Copy link
Contributor

Description

Added readhar to examples/contrib. Reads har files into mitmproxy.

Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff! Lots of feedback below, please extrapolate from my comments to the rest of your code! 😃

examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
Comment on lines 130 to 131
self.running()
self._read_har_task
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This here can be greatly simplified I think. You can just do this here instead and avoid the functions above:

Suggested change
self.running()
self._read_har_task
async def load_flows() -> None:
for flow in flows:
await ctx.master.load_flow(flow)
asyncio.create_task(load_flows())

The main underlying problem here is that commands cannot be async yet, otherwise we could just call await ctx.master.load_flow(flow) directly.

examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
@stanleygvi stanleygvi closed this Jun 27, 2023
@stanleygvi stanleygvi reopened this Jun 27, 2023
@stanleygvi
Copy link
Contributor Author

Thanks! Your feedback was super helpful! Here is the updated version of my code.

Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress! 🚀

examples/contrib/readhar.py Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
)

# 375:3 is default mitmproxy server_conn when making a new flow.
server_conn = connection.Server(address=("375", 3))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't comment and code here. Can you explain? :)

examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Please make sure that CI is passing (https://github.com/mitmproxy/mitmproxy/actions/runs/5405361567/jobs/9820804825), and then this is good to go! 😃

examples/contrib/readhar.py Outdated Show resolved Hide resolved
examples/contrib/readhar.py Outdated Show resolved Hide resolved
@mhils mhils enabled auto-merge (squash) June 29, 2023 15:02
Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

auto-merge was automatically disabled June 30, 2023 01:03

Head branch was pushed to by a user without write access

@mhils mhils merged commit 47021ba into mitmproxy:main Jun 30, 2023
20 checks passed
lasting-yang pushed a commit to lasting-yang/mitmproxy that referenced this pull request Aug 2, 2023
* first draft of read har.

* [autofix.ci] apply automated fixes

* Fixed using max's suggestions

* [autofix.ci] apply automated fixes

* updated to snake_case

* [autofix.ci] apply automated fixes

* Made max's changes.

* [autofix.ci] apply automated fixes

* gets server address from har file now

* [autofix.ci] apply automated fixes

* added support for if serverIPAdress wasnt found

* [autofix.ci] apply automated fixes

* editted docstring and changed peername and sockname values

* fix nits

* removed unused variable, should pass tox

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Maximilian Hils <git@maximilianhils.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants