Binary payload viewer
Categories
(DevTools :: Netmonitor, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: Honza, Unassigned)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
(Whiteboard: gsoc-2020)
Attachments
(5 files)
It should be possible to inspect binary WS frame payload. Such component could also be nicely reused for binary HTTP responses.
Honza
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
STR:
- Go to this test site and press Connect.
- Open Netmonitor > Click on request > Click on Messages panel.
- Send a non-ASCII message e.g. "б" or "他“.
- The message shown in DevTools is unreadable (refer to picture)
In nsIWebSocketListener.idl, the frame payload is defined to be of type ACString.
It would be nice if the payload returned is of a binary type.
Alternatively, it would be nice if the payload string returned is exactly the same string as what was sent over the wire. This way DevTools can parse it on the frontend by encoding the string into UTF-8 and then to a hex string to be shown to users.
:baku Do you know if this possible and easy to do? Thanks in advance :D
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 2•5 years ago
|
||
Firefox Send is also sending binary WS frames:
https://www.google.com/search?client=firefox-b-d&q=firefox+send
Honza
Comment 3•5 years ago
|
||
Comment 4•5 years ago
|
||
I also found the following issue:
- Launch Firefox browser
- Navigate to: http://demos.kaazing.com/echo/index.html
- Press F12 to open Devtools and go to the Network tab
- Press "Connect" button from the page opened at Step 2
- Click on "demos.kaazing.com" websocket request
- Click on "Messages" panel from the right side
- Mouseover the empty message
Actual result:
A tooltip with "Sent/Received" binary data is displayed. See screenshot
Expected result:
The message should not be empty.
Comment 5•5 years ago
|
||
Sorry for the delay!
Alternatively, it would be nice if the payload string returned is exactly the same string as what was sent over the wire. This way DevTools can parse it on the frontend by encoding the string into UTF-8 and then to a hex string to be shown to users.
nsIWebSocketListener is what dom/websocket uses. Based on the 'BinaryType' you can show the nsCString as a Blob or as an ArrayBuffer.
In nsIWebSocketEventListener, each webSocketMessageAvailable() as its type.
Is it enough to display the content correctly?
Reporter | ||
Comment 6•5 years ago
|
||
@Heng Yeow: does the comment from Andrea helps?
Honza
Reporter | ||
Comment 7•5 years ago
•
|
||
(In reply to Andrea Marchesini [:baku] from comment #5)
nsIWebSocketListener is what dom/websocket uses. Based on the 'BinaryType' you can show the nsCString as a Blob or as an ArrayBuffer.
In nsIWebSocketEventListener, each webSocketMessageAvailable() as its type.
I was testing this and I can confirm that webSocketMessageAvailable
provides the type
as an argument passed to this callback:
Here are the possible values:
https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/netwerk/protocol/websocket/nsIWebSocketEventService.idl#52-54
But, we need to have the type accessible in frameSent
and frameReceived
since webSocketMessageAvailable
doesn't provide access to the frame
and we don't know whether it's sent or received frame.
Andrea: it looks like we should be able to use frame.opCode
instead of type
, is that correct? Are there any advantages/advantages over in using opCode
over type
?
Also, I tried to get the raw data from the payload just simply doing the following:
var data = [];
for (var i=0, i<frame.payload.length; i++) {
data.push(frame.payload.charCodeAt(i));
}
console.log(data);
It seems to provide the right data but, is this what we should do? It doesn't feel ok.
Here is the place where I tried to get the raw data
https://searchfox.org/mozilla-central/rev/4d9cd186767978a99dafe77eb536a9525980e118/devtools/server/actors/network-monitor/websocket-actor.js#126
Honza
Comment 8•5 years ago
|
||
I prefer to have somebody from the necko team to answer this question.
Updated•5 years ago
|
Comment 9•5 years ago
|
||
(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #7)
Andrea: it looks like we should be able to use
frame.opCode
instead oftype
, is that correct? Are there any advantages/advantages over in usingopCode
overtype
?
You should use opcode because type isn't applicable to control frames. Opcode can be any of these codes https://searchfox.org/mozilla-central/rev/fa2df28a49883612bd7af4dacd80cdfedcccd2f6/netwerk/protocol/websocket/nsIWebSocketEventService.idl#29-36. See also https://tools.ietf.org/html/rfc6455#section-5.2.
Reporter | ||
Comment 10•5 years ago
|
||
Thanks Michal, also how we should process/handle binary data passed to the WS service listener?
I added a code snippet into the comment #7, but it doesn't feel like the right way to deal with binary data...
Honza
Comment 11•5 years ago
|
||
Sorry, I really don't know what's the best way to get raw data from string.
Comment 12•5 years ago
|
||
Honza, I was checking the protocols we support that use binary data:
WAMP already converts the string to a typed array: https://searchfox.org/mozilla-central/rev/fa2df28a49883612bd7af4dacd80cdfedcccd2f6/devtools/client/netmonitor/src/components/websockets/parsers/wamp/serializers.js#20
Reporter | ||
Comment 13•5 years ago
|
||
Thanks Harald so, it isn't far from what I tried.
Now, how can we get the original string from the frame.payload
?
Examples:
- sending
他
generates this payload (in dec):[ 228, 187, 150 ]
- sending
б
generates this payload (in dec):[ 208, 177 ]
It's correct from the binary perspective and corresponds to what I can see in Chrome.
But, how can we convert it back to string when showing it to the user?
Or any tips who to ask?
Honza
Comment 14•5 years ago
|
||
(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #13)
But, how can we convert it back to string when showing it to the user?
I don't think you should try to interpret the binary data. See end of the section 1.2 in rfc 6455 (https://tools.ietf.org/html/rfc6455#section-1.2):
Broadly speaking, there are types for textual data (which is interpreted as UTF-8 [RFC3629] text), binary data (whose interpretation is left up to the application), and control frames ...
Reporter | ||
Comment 15•5 years ago
|
||
(In reply to Michal Novotny [:michal] from comment #14)
I don't think you should try to interpret the binary data.
But, in my case the opCode
is 1
i.e. I just want to get the proper text
data so, it should be safe no?
Honza
Comment 16•5 years ago
|
||
If the frame is a text message, then simply use the payload as UTF-8 string.
Reporter | ||
Comment 17•5 years ago
|
||
@jlast pointed me to this TextDecoder MDN Page:
https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder
And it solves the problem I described in comment #13
let utf8decoder = new TextDecoder(); // default 'utf-8' or 'utf8'
let u8arr = new Uint8Array([ 228, 187, 150 ]);
console.log(utf8decoder.decode(u8arr));
Honza
Comment 18•5 years ago
|
||
AFAICS, TextDecoder() doesn't take string as an input, so you have to convert string to byte array and then back to string. You should probably do something like this https://searchfox.org/mozilla-central/rev/7fba7adfcd695343236de0c12e8d384c9b7cd237/third_party/webkit/PerformanceTests/SunSpider/sunspider-0.9.1/crypto-aes.js#359
Comment 19•5 years ago
•
|
||
Afaik, and please challenge me on that, the steps to Binary Viewer output in its many variations are the following:
- Take the utf-8
string
from the message and convert it back to a binary representation,Uint8Array
(see WAMP link) - Take
string
orbinary
data to generate various different formats- Hex: Iterate over
binary
to showbinary[i].toString(16)
in a 8 column hex table, aligned with utf-8 counterparts - Base64: Ignore binary data, show
btoa(string)
- UTF-8: Ignore binary data, just show the original utf-8
string
- Hex: Iterate over
Comment 20•5 years ago
|
||
Hi honza i want to work on this one, some points
i. I think we should add an extra property to the payload which will be utf8 representation of the payload. this property will be added in websocketFront(https://searchfox.org/mozilla-central/source/devtools/client/fronts/websocket.js#96) this will allow us to display strings correctly for both binary and text payload. The function for calculating utfString will be by utilizing TextDecorator
, (we should not convert the payload to utf8 string because for binary payload it is much easier to work with the payload we have)
ii.We should make a new component for showing binary data which go sidebyside with RawData.js and will be rendered if data is binary we could check that by opcode
, the component will display Hex,Base64 and UTF-8 representation of data , this will be quite similar to what google chorme have and we could use the harald's idea to implement (comment #19),
Reporter | ||
Comment 21•5 years ago
|
||
Pranav, thanks for the interest! But, this bug is supposed to be part of GSoC 2020 so, it needs to wait yet (it'd be opened for contribution in case it isn't done during the internship).
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 23•5 years ago
|
||
Just attaching a screenshot of standard binary viewer.
Honza
Reporter | ||
Updated•5 years ago
|
Updated•2 years ago
|
Comment 24•2 years ago
|
||
Hi, I recently ran into situations where this could've been handy, and hacked up a hex viewer similar to Chrome's that I could try to land. Is this open to contribution now, and if so, would something that looks like the attached image be OK?
Reporter | ||
Comment 25•2 years ago
|
||
This looks awesome!
I am going to add [devtools-triage]
keyword to this, so it goes through regular team triage where we discuss next steps.
If you have a patch we could try please attach it to this bug.
The screenshot looks great. I am curious about the performance. How it works for bigger binary responses?
Is your solution virtualized (i.e. only rendering what's visible)?
Thank you for the help!
Comment 26•2 years ago
|
||
Updated•2 years ago
|
Comment 27•2 years ago
|
||
If you have a patch we could try please attach it to this bug.
I have attached a phabricator request, apologies if you meant literally attach a diff file. The viewer I'd hacked up was basically just a simple/crude text area displaying a formatted string that looks like a hex display.
Is your solution virtualized (i.e. only rendering what's visible)?
No, even if text area only renders what's visible, it's not fully virtualized. As, what I'd hacked up ends up generating a hex display formatted string for the entire payload regardless of what's currently visible. Feel free to let me know if I should look into virtualizing the view for this to land.
I am curious about the performance. How it works for bigger binary responses?
Depending on what you mean by bigger, it could be fine or bad :D
I tested a binary frame against the default devtools.netmonitor.msg.messageDataLimit
(100 KB iirc) and nothing was noticeably different/slow/bad.
After raising the pref limit to avoid truncation, I also tested a ~111 MB binary frame. All of Firefox crashed when trying to use the hex viewer. However, I also tested it on the raw payload viewer on bookmarks/central and all of Firefox was also crashing there. So this could be an existing issue or my configuration/setup is bust. If someone has any pointers on things I can be looking at (eg. a log file somewhere?) to confirm that this is in fact a bug, I can try them after work and file a separate bug if necessary.
Out of curiosity, I also checked the same frame in Chrome and it was crashing there too (however, only the DevTools window was crashing).
Comment 28•2 years ago
|
||
Discussed this during triage, given the 100kB limitation by default, plus the fact that the current implementation only renders one textarea element, going for a first version without virtualization sounds good.
Maybe we can restrict this to websocket, this way packets are expected to be relatively small.
In case the user overrides the preference, we could still disable the viewer if the size of the packet is too big.
Comment 29•2 years ago
|
||
Discussed this during triage, given the 100kB limitation by default, plus the fact that the current implementation only renders one textarea element, going for a first version without virtualization sounds good.
Awesome, the patch I've posted is ready for review and continues with the one textarea approach for now. It depends on a patch that adds base64/hex copy options for binary websocket frames, since that patch has some test changes and constants that are being used in this patch. Feel free to let me know if it's easier to review without the dependency and I'll split it out.
Maybe we can restrict this to websocket, this way packets are expected to be relatively small.
I've restricted the viewer to only be shown for not parsed binary websocket frames.
In case the user overrides the preference, we could still disable the viewer if the size of the packet is too big.
I was unsure of what limit to use, so if you have any suggestions I can add it in to the patch.
Separately, I also looked into virtualizing it for fun and it shouldn't be too hard to do given that there is a VirtualizedTree which basically worked out of the box. Though it might be better if VirtualizedTree
is first changed or another similar/simpler component made to leverage knowing the total number of elements ahead of time. As VirtualizedTree
currently is, a new array of size bytes.length / BYTES_PER_ROW
would need to be constructed in getRoots
which wastes time and space. It would be easier if we could just pass a length and the prop functions instead operated on indices up to the length instead of actual items.
Comment 30•1 years ago
|
||
(In reply to Krishna Ravishankar from comment #29)
Discussed this during triage, given the 100kB limitation by default, plus the fact that the current implementation only renders one textarea element, going for a first version without virtualization sounds good.
Awesome, the patch I've posted is ready for review and continues with the one textarea approach for now. It depends on a patch that adds base64/hex copy options for binary websocket frames, since that patch has some test changes and constants that are being used in this patch. Feel free to let me know if it's easier to review without the dependency and I'll split it out.
Maybe we can restrict this to websocket, this way packets are expected to be relatively small.
I've restricted the viewer to only be shown for not parsed binary websocket frames.
In case the user overrides the preference, we could still disable the viewer if the size of the packet is too big.
I was unsure of what limit to use, so if you have any suggestions I can add it in to the patch.
Separately, I also looked into virtualizing it for fun and it shouldn't be too hard to do given that there is a VirtualizedTree which basically worked out of the box. Though it might be better if
VirtualizedTree
is first changed or another similar/simpler component made to leverage knowing the total number of elements ahead of time. AsVirtualizedTree
currently is, a new array of sizebytes.length / BYTES_PER_ROW
would need to be constructed ingetRoots
which wastes time and space. It would be easier if we could just pass a length and the prop functions instead operated on indices up to the length instead of actual items.
Thanks for the investigation!
I was unsure of what limit to use, so if you have any suggestions I can add it in to the patch.
It's something we can handle in a follow up bug, no need to implement that here.
Separately, I also looked into virtualizing it for fun
It would be amazing to try this out and see if it improves performance compared to the textarea approach.
Comment 31•1 years ago
|
||
It would be amazing to try this out and see if it improves performance compared to the textarea approach.
I've updated the attached phabricator patch to virtualize instead.
Depending on which performance metrics are considered, it is definitely an improvement over the textarea approach. The initial draw is significantly faster with virtualization for bigger payloads and I can load payload such as the earlier mentioned 110MB frame on the hex viewer. For sufficiently sized payloads, scrolling also seems to be fine, if it has gotten slower I wasn't able to notice it. I haven't profiled memory, but having an additional array linearly proportional to the size of the payload with nothing but indices does feel wasteful. Though, maybe this is one of those things that can be fixed in a follow up if it ever becomes a problem.
Another problem I noticed is that the scrollbar seems to break on big payloads (eg. ~110 MB payload). When the height of the invisible presentation div of the VirtualizedTree
is sufficiently large (eg. 124457000px) , the maxpos
of the vertical scrollbar
element ends up pretty low (eg. 31; as if some calculation result overflowed or such?). I am not sure if I am misusing VirtualizedTree
, or if I wrote some horrible CSS/code, or there's some browser bug.
Let me know what you think about virtualizing the view this way and which approach would be better to continue with to land a patch.
Comment 32•11 months ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Comment 33•9 months ago
|
||
Sorry, I dropped the need info here.
Krishna: By any chance are you still around and willing to work on this?
We should probably ping :nchevobbe then who conducted the first round of review.
Comment 34•9 months ago
|
||
Julian Descottes [:jdescottes](In reply to Julian Descottes [:jdescottes] from comment #33)
Krishna: By any chance are you still around and willing to work on this?
Yeah, I'm still around and I don't mind continuing on the patch if it's useful to others.
We should probably ping :nchevobbe then who conducted the first round of review.
I can try to rebase the patch this weekend and then ping :nchevobbe
How/where do I ping someone though?
Comment 35•9 months ago
|
||
(In reply to Krishna Ravishankar from comment #34)
I can try to rebase the patch this weekend and then ping :nchevobbe
How/where do I ping someone though?
With the "Request information from" feature.
For now, please rebase the patch, and we will see if Nicolas has the bandwidth to do the review. I will switch the reviewer flag to be the whole devtools group, that might be better.
Updated•9 months ago
|
Updated•8 months ago
|
Updated•8 months ago
|
Comment 36•8 months ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #35)
With the "Request information from" feature.
Ah, thanks!
For now, please rebase the patch, and we will see if Nicolas has the bandwidth to do the review. I will switch the reviewer flag to be the whole devtools group, that might be better.
Done.
Comment 37•2 months ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Description
•