Fix saving of websocket flows

[txrp0x9]

Description

WebSocket flows when saved would return an empty file (cut.save) , and export raw command would not show the ws messages (#6721). This was because the cut.save command uses response.content and response.content is kept empty in websocket, with the actual content (messages) being saved in websocket.messages, while export raw has no support for ws messages.

This PR fixes this issue by storing formatted messages in response.content as they appear on websocket.messages
This also removes the "No content" default text on websocket response/request (If we don't do this, then response.content will be shown, but we already have WebSocket Messages tab so showing it again would be redundant)

This PR changes export addon and adds websocket message serialization support to the raw export format.
This also adds a fix to cut.save keybind (b), by modifying the cut.save command to save websocket messages whenever request.content or response.content is saved for a websocket flow.

Checklist

• [✔️ ] I have updated tests where applicable.
• [✔️ ] I have added an entry to the CHANGELOG.

[mhils]

Thanks for the PR!

This PR fixes this issue by storing formatted messages in response.content as they appear on websocket.messages

Architecturally that doesn't fly, sorry. WebSocket messages belong in flow.websocket.messages, we definitely do not want to duplicate them in flow.response.content. That's semantically incorrect and also doubles the size of saved flows.

I'm afraid this needs changes to the cut addon. I'm also open to UX improvements w.r.t. displaying "no content", but we can't just go and butcher response.content. :)

[txrp0x9]

I'm afraid this needs changes to the cut addon. I'm also open to UX improvements w.r.t. displaying "no content", but we can't just go and butcher response.content. :)

Yeah, I was honestly confused about what approach to follow but thats okay, I'll redo this and change cut addon instead. I've mentioned 2 approaches I thought of here, this PR followed approach 2. Can you take a look at approach 1 and tell me if its on the right path? It modifies the cut addon, but it just saves formatted websocket.messages whenever response.content is requested.

[txrp0x9]

@mhils I've added support for websocket message serialization in the raw format for the export addon. I've also added another new format called "websocket" which only saves the websocket messages (if the flow is a ws).
For the cut.save keybind (b), since response.content is hardcoded in the keybind itself, I've added code to the cut.save command that saves websocket messages whenever response.content or request.content is used with ws flows in cut.save arguments.

[mhils]

Suggested change

	if (
	hasattr(f, "websocket")
	and f.websocket is not None
	and (cut == "response.content" or cut == "request.content")
	):
	# Hack for https://github.com/mitmproxy/mitmproxy/issues/6721:
	# Make "save body" keybind work for WebSocket flows.
	# Ideally the keybind would be smarter and this here can get removed.
	if (
	getattr(f, "websocket", None)
	and cut == "response.content"
	):

[txrp0x9]

hasattr(f, "websocket")
and f.websocket is not None

Both above lines seems to be required, there is a test which fails otherwise. In the test scenario, websocket attribute exists but is None. Your change includes getattr(f, "websocket", None), but that just seems checks if the websocket attribute is present (otherwise returns None as default).

Also for cut.save both request.content and response.content should save websocket messages. Considering sent messages might be considered as request.content too.

[mhils]

otherwise returns None as default

That is true, but bool(None) evaluates to false and the condition fails. :)

The intention behind just doing it for response.content was to limit the impact of this workaround, but I don't feel particularly strongly about this one. Let's keep it as-is if you think that's better. :)

[mhils]

Is this some known standard, or is this something we're making up here?

In the latter case I'd favor making this _format_ws_message (and same below), I don't want to commit here to a public API prematurely.

[txrp0x9]

I've added complete test coverage and added changes you suggested @mhils

[mhils]

Thanks! I simplified things a bit more, let me know if there's something you believe we should change. I removed raw_messages because that's a bit confusing when it appears in the list for plain HTTP. But the export is covered with raw. 🍰

[mhils]

otherwise returns None as default

That is true, but bool(None) evaluates to false and the condition fails. :)

The intention behind just doing it for response.content was to limit the impact of this workaround, but I don't feel particularly strongly about this one. Let's keep it as-is if you think that's better. :)