On Fri, Jan 13, 2017 at 1:09 AM, Cory Benfield <c...@lukasa.co.uk> wrote: > > On 13 Jan 2017, at 09:07, Nathaniel Smith <n...@pobox.com> wrote: > > I was imagining something along the lines of > > ClientContext({...}).wrap_socket(...) > > where ClientContext.__init__ would do the validation. Or there are > various other ways to slot the pieces together, but in general > validation doesn't need to be done incrementally; there's going to be > some place where the config dict hits the library and it can/should be > validated at that point. And before that point we get all the dict > niceties for free. > > > What are the dict niceties, exactly? I’m still not seeing what the advantage > is in throwing away a typed and restricted API for a dict is.
Getters & setters (right now the spec has setters for everything, but a getter for validate_certificates only -- that's kinda weird?), an obvious serialization story for the parts of the config that use basic types (e.g. to/from JSON/TOML/YAML), an obvious/natural way to represent "here are the config changes I want to make in response to that ClientHello" for the sni callback as a declarative object, a nice __repr__, familiarity, ... obviously these are all things one can put into the spec piece by piece, but you're the one who said you wanted to simplify things :-). I get what you're saying about a typed API -- basically in the current setup, the way _BaseContext is written as a bunch of Python methods means that the interpreter will automatically catch if someone tries to call set_cihpers, whereas in the dict version each implementation would have to catch this itself. But in practice this difference seems really minimal to me -- either way it's a runtime check, and it's really difficult to write a loop like for key, value in config.items(): do_some_quirky_cffi_thing_based_on_key() that doesn't implicitly validate the keys anyway. There's also the option of using e.g. JSON-Schema to write down a formal description of what goes in the dict -- this could actually be substantially stricter than what Python gets you, because you can actually statically enforce that validate_certificates is a bool. For whatever that's worth -- probably not much. If we really want to simplify it down, then as Alex noted in the PR, TLSWrappedSockets are superfluous, so we could make it just: # Simple functions, not methods: client_wrap_buffers(config, incoming, outgoing) -> TLSWrappedBuffers server_wrap_buffers(config, incoming, outgoing) -> TLSWrappedBuffers class TLSWrappedBuffers: ... read and all that stuff ... @property @abstractmethod def get_config(self) -> dict: pass def update_config(self, new_settings): # Attempt to do the equivalent of self.config.update(new_settings); # raises TLSError if any of the settings can't be changed given # current backend or current connection state. Anyway, I'm not actually sure if this is a good idea, but it seemed worth putting the two approaches up where we could look at them and compare :-). -n -- Nathaniel J. Smith -- https://vorpus.org _______________________________________________ Security-SIG mailing list Security-SIG@python.org https://mail.python.org/mailman/listinfo/security-sig