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

On "debug" Connection Events and UX #19

Open
Tokarak opened this issue Jun 19, 2022 · 2 comments
Open

On "debug" Connection Events and UX #19

Tokarak opened this issue Jun 19, 2022 · 2 comments

Comments

@Tokarak
Copy link

Tokarak commented Jun 19, 2022

This issue reports several linked problematic behaviours.

I am currently playing around with your tutorial/demo Client+Server. In one of your demonstrations (in HBT2018 if you remember) you had 6 events, while I am getting 45. My set up is the same, except that the obsoleted RelayInterceptHandler is InterceptHandler>RelayHandler>InterceptHandler (like in the default project).
The messages consist of layer-4 communication events like "READ", "CHANNEL_ACTIVE", "OPEN" - not useful when you only care about the data transported by the application-layer protocol; Your demo does not have this superfluous information. Perhaps there should be a way of filtering it, or maybe even hiding it by default. Maybe each node of the flowchart should have a tickbox to show wether its events should be intercepted (perphaps deprecating the unintuitive InterceptHandler); this could be done by creating a wrapper class for nodes of the flowchart, also allowing configuration like a default style, and maybe future node-specific configuration; In my head this seams doable, but if, for example, Events are by default part of one large centralised pool, it may be harder to filter them. (edit: looks like this can be easily resolved. I heavily commend your codebase.) Comments on this would be helpful.

This is problematic not only because it hinders clear reading, but Intercept functionality has no keyboard control, meaning each event has to be clicked, then the send button has to be clicked, for each event, no matter how minor. Here it would be useful a.) to let Enter key send the packet (I don't know the terminology but its when a button is highlighted blue, showing it's the backup responder if there is no first responder being interacted with (like a text field)) b.) have the selected Event auto-scroll on arrival of a new Event, if and only if the current selected event is currently the latest (perphaps there is already an implementation of this in your UI framework too, I can't be sure). On the same topic, shortcut keys are lacking in the graph editor (ctrl C, ctrl S, arrow navigation throughout the whole program etc.). (There is a small chance that this could be caused by macOS, not famous for great support outside of its "ecosystem".)

Since every message gets sent to the proxy, then forwarded, I can reduce the messages by half by removing one InterceptHandler. This, however, causes all direction messages to be "Client to Proxy", not useful and (for me, who is not familiar with the details of layer 4 transport very well) confusing even if it may be correct. In your 2018 demo, direction was conveyed through compact and intuitive arrows, which I would prefer; I have a suspicion that this relied on the combined RelayInterceptHandler.
Why you opened up layer 4 transport is beyond me. There should be no reason to manually modify or see tcp-layer communication; one could perhaps transform protocols through the graph (which in itself is dangerous and purposeless), but never manually. Without layer 4 communication seen, there log should combine events into a continuous stream from client to server (like your -> <- arrows), with the proxy editing requests but remaining transparent. Like Burp.
If for whatever reason you want to keep the distinction between the two sides of the proxy, I think the best way to show the direction is this template: C<-->P<-->S; Fill out the blanks as needed. e.g
|C -->P |
| P -->S|
| P<-- S|
|C<-- P |
Obviously my demonstration failed, but it should look amazing in a monospace font, where the letters would be lines up. Simple, compact, descriptive, intuitive, and scannable by the eyes; I will be quite proud of this idea, if I ever get to implement it.

May I also add, if the collumns are squished beyond the size of the text field, the truncation is notated with "...": that is three characters (one actually) which could be replaced by more useful information. Even Wireshark, where the table is almost full screen compared to the tiny table here, does not do that (unless I am under some sort of Mandela Effect).

I don't know when you will respond, but when you do, could you point me to a task I could learn from? Meanwhile, I will poke around a little, attempting some of the easier tasks.

@Tokarak
Copy link
Author

Tokarak commented Jun 19, 2022

BTW, since I have an ego big enough to consider myself traction on this project, do you think you could add a license?

@RoganDawes
Copy link
Collaborator

Hi Nazar.

Thanks for the detailed feedback. I agree, having the READ, etc events that do not contain meaningful data really do just clutter up the user interface. I have been contemplating implementing a filter on the ListModel which is used to hold the events, so that you can choose which events you see. Any events which are not visible should then automatically be forwarded, even if the "Intercept" flag is selected.

If I am honest, this hasn't really been a high priority for me, because I tend to do all of my modifications using a ScriptHandler, rather than by hand. For many reasons, timeouts being one, but the crappy user experience probably also contributes!

If this is something that you would like to work on, I'd welcome contributions. The license should be GPL-3.0. I'll add it as soon as I get a chance.

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

No branches or pull requests

2 participants