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 flow support to relative paths #6562
base: main
Are you sure you want to change the base?
Conversation
Ok, this is partially working. I don't think I can get much further without making the js compile locally and running the full test suite. |
You can compile the JS with |
I am a bit baffled by the test results. Unfortunately am not at all proficient at npm. I was able to reproduce the npm test failure locally, but can't figure out why it is receiving a completely different POST command to what the test expects e.g.:
Have I messed something up with the JS compilation maybe? Please help :) |
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.
Sorry for the slow RTT. I was trying to get this done today, but there's a bunch of stuff that isn't mergable at the moment. The errors you are seeing are caused by test failures, check out the mentioned tests and things should become relative clear. Thanks!
@@ -124,7 +124,7 @@ describe("onKeyDown", () => { | |||
|
|||
it("should handle delete action", () => { | |||
store.dispatch(createKeyEvent("d")); | |||
expect(fetchApi).toBeCalledWith("/flows/1", { method: "DELETE" }); | |||
expect(fetchApi).toBeCalledWith("flows/1", { method: "DELETE" }); |
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.
Calls to fetchApi should still be prefixed with /
, fetchApi has explicit logic to make them relative.
@@ -70,9 +70,9 @@ export function updateUrlFromStore(store) { | |||
|
|||
let url; | |||
if (state.flows.selected.length > 0) { | |||
url = `/flows/${state.flows.selected[0]}/${state.ui.flow.tab}`; | |||
url = `flows/${state.flows.selected[0]}/${state.ui.flow.tab}`; |
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 breaks all existing links to mitmweb instances, which I'd like to avoid. location.hash does not need relative URLs.
Description
#6411 added relative paths to allow the web root for mitmproxy to be placed in locations other than '/'.
But some items were missed from this change, especially manipulating and saving flows, see #6525
This PR modifies several js components so that POST API features work even when the web root is not '/'.
Checklist