mwclient icon indicating copy to clipboard operation
mwclient copied to clipboard

`site.login(username, password)` fails between mediawiki 1.24.1 and 1.27.0

Open BenjaminPelletier opened this issue 5 years ago • 3 comments

With a MediaWiki 1.24.1 installation, site.login(username, password) fails. This happens because the raw text received back from the raw_api call is '{"warnings":{"tokens":{"*":"Unrecognized value for parameter \'type\': login"}},"query":{"tokens":[]}}' and then mwclient attempts to treat the content of tokens as a dict resulting in a TypeError (list indices must be integers or slices, not str) rather than the expected KeyError. A simple fix might be to catch TypeError here, or it might be good to explicitly check for empty info['query']['tokens'].

BenjaminPelletier avatar Jun 30 '20 07:06 BenjaminPelletier

Hm, that's odd, I thought the login token was introduced in MW 1.24.0. Do you know if it's working in later versions in the 1.24 line? Never mind, it was introduced in 1.27.

Yeah, it's tempting to just catch TypeError as well. But since TypeError is such a widely used error, there's certainly a risk it could end up masking other errors in the future, so I think I'm leaning towards introducing an explicit check.

Do you see a more elegant way to do it than this?

tokens = info['query']['tokens']
if not isinstance(tokens, dict) or '%stoken' % type not in tokens:
    raise NoSuchToken()  # New error class

danmichaelo avatar Jul 08 '20 19:07 danmichaelo

That looks reasonable to me (and better than implicitly accepting KeyError). Just throwing a KeyError with !isinstance(tokens, dict) would be less work, but not as strictly-correct as a new error class.

I fixed things locally by just accepting the TypeError, which is worse of a hack than throwing KeyError :)

BenjaminPelletier avatar Jul 08 '20 23:07 BenjaminPelletier

Hum. I think I have a better handle on this now, as the new description indicates: this only happens if the version is higher than 1.24 (when we started trying to get the token) and lower than 1.27 (when mediawiki started actually returning it).

Given that 1.26 went EOL in late 2016 - over seven years ago - this doesn't seem super important, so dropping the 1.0.0 milestone. Not sure if it's still worth fixing for people running super old and insecure mediawiki versions.

AdamWill avatar Jan 28 '24 16:01 AdamWill