-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Rewrite mitmweb with a focus on modern UI/UX (WIP) #7784
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
base: main
Are you sure you want to change the base?
Conversation
This doesn't include any functionality; it's purely UI code using mocked data to render the initial design.
This name is better in terms of visibility. The `2` is hard to spot otherwise.
Kind of ugly this way but it will do for now.
…localhost` to ensure proper cookie usage for the auth token
…omponents This is fine 99% of the time for this type of application. Better turn it off than adding eslint overrides everywhere.
Resets to the options to the default state.
… version to `testState`
…hecks We don't need to track test coverage for the webnext tool. It's only a temporary utility for testing changes and will be removed eventually.
mhils
left a comment
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! As mentioned before, fantastic work and lots of stuff which I'd really like to see in mitmweb.
From a strategic perspective, I would like to minimize the work we need to do before we can ship improvements to users, i.e. let's explore the option of doing a gradual rewrite before committing to a from-the-ground rewrite.
My proposal would be as follows:
- Modernize the build tooling. I've left some operational questions below. This should be mostly straightforward, but I'll probably want to venture on some side quests to get rid of cruft right away that's intertwined with that (e.g. PEG parsing).
- Build a PoC that integrates Tailwind/Radix/shacdn/react-icons with our current stack. This is the part where I'm most worried on how that goes, but the best way is to just try it out.
If this proves to be successful, we can do the cool stuff in parallel:
- Cherry-pick your components in any order.
- Gradually kick out the legacy bits (fontawesome font, ...).
Unclear if all of this works, but I feel it's a much clearer path than going for feature parity in your rewrite. If we find that gradual does not work, we can regroup and evaluate our options. But being able to ship small improvements right away sounds exciting to me. :)
I'll set aside a few evenings to get started with the build tooling, will CC you on PRs!
| # The option to disable XSRF cookies is needed to test actions like replay requests from the vite dev server during development. | ||
| # In all other scenarios this should be turned ON, hence the warning. | ||
| disable_xsrf_cookies = os.environ.get("UNSAFE_DISABLE_XSRF_COOKIES", False) |
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.
Any intuition why this is necessary? Is that because of the different port?
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.
So as I briefly mentioned in private: hacks like this are indeed needed when you're dealing with 2 different applications running on different ports. In this case the vite dev server running on port 5173 and mitmweb running on port 8081.
The current mitmweb frontend doesn't suffer from this issue because it's statically served from the same tornado http server. Vite on the other hand requires its own development http server which comes with HMR out of the box.
Without disabling XSRF cookies you'd get the following error when performing actions like replay request:
I hope that makes sense. Play around with it for a bit and you'll hopefully get a better picture :)
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.
Without disabling XSRF cookies you'd get the following error when performing actions like replay request:
Yes, I didn't miss that. My question was more about whether we know why specifically Tornado doesn't like that. No worries though, I'll look into it. :)
| @@ -0,0 +1,63 @@ | |||
| """ | |||
| This mitmproxy serves the production build of the mitmwebnext interface and strips the Origin header from all outgoing requests to the mitmweb API. | |||
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.
What's the purpose of this addon? I understand what it does, but I don't understand why/what we want this for. 😄
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.
Related to my reply above: #7784 (comment)
Similarly, this was a hacky work around to properly test both production builds side by side. Probably no longer relevant soon.
| plugins: [ | ||
| react({ | ||
| babel: { | ||
| plugins: [["babel-plugin-react-compiler", ReactCompilerConfig]], |
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.
Any particular reason why babel and not swc?
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 is what comes with the react-vite template by default IIRC. This was recommended by the react docs back then to integrate react-compiler. (see this commit). Not much more thought was put into this, so no, no particular reason at all.
So far the compilation performance is fine with babel too, though I'm fine with switching to SWC if you prefer. No strong opinions there.
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.
Might need more research but so far it looks like the SWC plugin is not yet supported in combination with React Compiler: vitejs/vite-plugin-react#428.
See #7694 (comment) and the mitmproxy dev chat for context.
This PR serves as an entry-point to get some initial progress merged into a separate branch in this repository. I'll briefly document an overview of the most impactful changes below.
Important
Note to maintainers: please update this PR to target a new branch
webnext.Changes
New changes so far. The changes are ordered from oldest to newest, so you can follow along in the commit history.
Keep in mind that none of the new changes here are 100% completed. I wanted to leave some wiggle room so we can iterate, improve and swap out parts as the project evolves. For that reason I also didn't write any tests yet.
Tech stack
While the current
mitmwebstack is not bad (because clearly it works) it is overdue for an upgrade. Especially with potentially new contributors in mind (human or robot) it could benefit from a modern stack.Architecture
I created a new project directory for the new code to reside in and added an extra build output to the
webproject so it can be imported as a dependency in webnext. Things like redux state, the websocket connection and keyboard shortcuts can be shared that way. There are some exceptions though, like the split request/response panel requiring changes to the state handlers, but overall the amount of changes made to the original web are minimal. This also means you should be able to run (and develop!) both projects independent from each other.Theme
Support for the following themes out of the box:
Going forward all screenshots below will have the dark theme applied.
Welcome view
Complete overhaul of CaptureSetup.tsx:
Note
Most of what you see are placeholders. For example the buttons and links are non-functional. To be replaced / updated later once the overall picture of the project and new location of its views becomes clear.
Flow list
As showcased in https://youtu.be/5cBCouydg1I:
With a split request/response panel:
And a context menu (very basic at the moment but the idea is this should replace all 'actions' from the 'Flow' tab when one is selected):
Intercept filters
Added an 'easy mode' for building filters graphically:
And an 'advanced' mode with autocomplete for power users who already know the syntax on top of their heads:
Note
Both input methods still suffer form a validation discrepancy bug between client and server. A fix is NOT included in this PR.
Search
Search is currently reusing the same autocomple input field from the intercept filters above:
Note
This approach is a little outdated already. To benefit from the 'easy' and 'advanced' modes too, we could implement search in the same way as the intercept filters. Components already exist, so it's only a matter of reusing them.
Note
'Highlight' flows isn't implemented yet. It could be integrated into search, but I feel like it can be repurposed even further. So I skipped it for now. TBD at a later stage.
Options
New options menu with options divided into logical categories:
Note
Categories are currently mapped to options on the frontend only. Any new options that haven't been categorized yet will be placed under a miscellaneous tab called 'Other'.
Getting started
To try out the changes locally, please read webnext/README.md.
Next steps
Next steps for maintainers.
My proposal: