matrix-org / sydent

Sydent: Reference Matrix Identity Server
http://matrix.org
Apache License 2.0
293 stars 83 forks source link

GET from MsisdnValidateCodeServlet broken? #445

Closed DMRobertson closed 2 years ago

DMRobertson commented 2 years ago

Inside render_GET we call get_args and destructure the result to a 2-tuple:

https://github.com/matrix-org/sydent/blob/567eb1a89dbf13c37b857a82d91dac62a484e528/sydent/http/servlets/msisdnservlet.py#L131

get_args returns a dictionary, and has done since https://github.com/matrix-org/sydent/commit/687d774833b17a6e3b19dfb2ec50ef894e5e74f2. The destructing will fail unless the dictionary contains two keys.

https://github.com/matrix-org/sydent/blob/a6fff7b12aeb04689ccd98757adeeffbbdb66149/sydent/http/servlets/__init__.py#L46-L48

Everywhere else, we use args = get_args with no destructuring. I guess we missed this servelet in that change and either nobody noticed or the endpoint is unused.

DMRobertson commented 2 years ago

https://sentry.matrix.org/sentry/sydent/issues/16284/ spotted by @clokep

clokep commented 2 years ago

sentry.matrix.org/sentry/sydent/issues/16284 spotted by @clokep

I think this might actually be unrelated. I would have expected "not enough values to unpack" while that error is "too many values to unpack".

DMRobertson commented 2 years ago

I would have expected "not enough values to unpack" while that error is "too many values to unpack".

A dictionary with N keys will happily desugar into an N-tuple:

>>> d = {"a": 1, "b": 2, "c": 3}
>>> x, y, z = d
>>> x, y, z
('a', 'b', 'c')

So if get_args returned 3 or more args, we'd see "too many values to unpack".