-
-
Notifications
You must be signed in to change notification settings - Fork 56
Feature: Add folder support to .proto parser #273
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
Conversation
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 for the PR! Some small nits on error handling, otherwise looks good. 😃
mitmproxy-contentviews/src/protobuf/existing_proto_definitions.rs
Outdated
Show resolved
Hide resolved
mitmproxy-contentviews/src/protobuf/existing_proto_definitions.rs
Outdated
Show resolved
Hide resolved
mitmproxy-contentviews/src/protobuf/existing_proto_definitions.rs
Outdated
Show resolved
Hide resolved
mitmproxy-contentviews/src/protobuf/existing_proto_definitions.rs
Outdated
Show resolved
Hide resolved
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! 🍰
|
Thanks for the recommendations on the error handling! Rust's error handling is a bit foreign to me compared to other languages, so I didn't realize that the functions I was using would cause panics. 😅 There is one other thing I would like to ask about that though. I tried experimenting with manipulating the URL path string in a custom contentview script as we discussed here, but almost all my .proto files only include messages without any services, so for method in file.messages() {
if method.proto().name() != rpc.method {
continue;
}
return Some(method);
}to line 80 so that mitmproxy will attempt to find a matching message if no matching services are found. This way, there will be one final attempt to load the content using a message that doesn't have a service before fallbacks, and a contentview script can manipulate the URL path to be able to load messages that don't have services. If you think this change is fine, I can open another PR for it. |
|
Sure - that sounds great! |
This pull request adds folder support to the .proto file parsing code. If the path given in
protobuf_definitionsis a folder, mitmproxy_rs will walk the entire directory searching for files ending with .proto and, if found, will pass them into the parser. This should allow for developers to have an easier time using mitmproxy in case their projects uses protobuf/gRPC messages that are split over multiple files.In my testing, a folder that has ~800 .proto files can take about a second or so to process and show the output in mitmweb. So, it's recommended to only add the .proto files necessary for decoding requests/responses, as anything extra will only increase processing time before the results are shown.
My Rust knowledge is very limited, so if there are better ways of doing this, please let me know!