[WIP] Add RESTful API for mitmweb option #2395
Conversation
There was a problem hiding this comment.
Good stuff! All comments are really complaints on a high level. :)
I haven't seen travis fail here, but I guess this is due to @Kriechi's draconian fantastic coverage checks. We should add a test for the API call in https://github.com/mitmproxy/mitmproxy/blob/master/test/mitmproxy/tools/web/test_app.py. :)
mitmproxy/optmanager.py
Outdated
| """ | ||
| Dumps the options into a list of dict object. | ||
|
|
||
| Return: A list like: [ { name: "anticahce", type: "bool", default: false, value: true, help: "help text"}] |
mitmproxy/optmanager.py
Outdated
| """ | ||
| options_list = [] | ||
| for k in sorted(opts.keys()): | ||
| o = opts._options[k] |
There was a problem hiding this comment.
You can simplify this a tiny bit:
for k, o in sorted(opts.items()):There was a problem hiding this comment.
I'm afraid we could not do this, opts here is not a dict, but a OptManager, or do you think we should add a .items method for it?
There was a problem hiding this comment.
Ah, I see. Let's keep it as is then. :)
mitmproxy/optmanager.py
Outdated
| options_list = [] | ||
| for k in sorted(opts.keys()): | ||
| o = opts._options[k] | ||
| option = {'name': k, 'type': o.typespec.__name__, 'default': o.default, 'value': o.current(), 'help': o.help.strip()} |
There was a problem hiding this comment.
Maybe add some line breaks here to make this easier to read?
There was a problem hiding this comment.
Also, the help.strip() here should be un-necessary - we dedent and format help text on initialisation. Let's keep the help formatting in one place.
There was a problem hiding this comment.
Good catch. This also needs to support .choices.
mitmproxy/optmanager.py
Outdated
| options_list = [] | ||
| for k in sorted(opts.keys()): | ||
| o = opts._options[k] | ||
| if o.typespec in (str, int, bool): |
There was a problem hiding this comment.
Move this typespec to string conversion into its own function in mitmproxy/utils/typecheck? :)
|
|
||
| def test_option_update(self): | ||
| assert self.put_json("/options", {"anticache": True}).code == 200 | ||
| assert self.put_json("/options", {"wtf": True}).code == 400 |
There was a problem hiding this comment.
We should also test type confusion here (e.g. {"anticache": "foo"})
| @@ -98,3 +98,15 @@ def check_option_type(name: str, value: typing.Any, typeinfo: typing.Any) -> Non | |||
| return | |||
| elif not isinstance(value, typeinfo): | |||
| raise e | |||
|
|
|||
|
|
|||
| def typespec_to_str(typespec: typing.Any) -> str: | |||
There was a problem hiding this comment.
We should also use this in dump_defaults
mitmproxy/utils/typecheck.py
Outdated
| if typespec in (str, int, bool): | ||
| t = typespec.__name__ | ||
| elif typespec == typing.Optional[str]: | ||
| t = 'Union' |
There was a problem hiding this comment.
Make this "optional str" (and "sequence of str" below this) to match what is currently being done in dump_defaults
The return json looks like this:
[ { "default": false, "help": "Add all certificates of the upstream server to the certificate chain that will be served to the proxy client, as extras.", "name": "add_upstream_certs_to_client_chain", "type": "bool", "value": false }, { "default": false, "help": "Strip out request headers that might cause the server to return 304-not-modified.", "name": "anticache", "type": "bool", "value": false }, ...]The tests passed on my local repo, but kept falling on Travis... Is that because of some compatibility problem?