Skip to content
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

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

Open
BenjaminPelletier opened this issue Jun 30, 2020 · 3 comments
Labels

Comments

@BenjaminPelletier
Copy link

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'].

@danmichaelo
Copy link
Member

danmichaelo commented Jul 8, 2020

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

@BenjaminPelletier
Copy link
Author

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 :)

@AdamWill AdamWill added this to the 1.0.0 milestone Jan 28, 2024
@AdamWill AdamWill changed the title Login fails for MW 1.24.1 site.login(username, password) fails between mediawiki 1.24.1 and 1.27.0 Jan 28, 2024
@AdamWill
Copy link
Member

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 AdamWill removed this from the 1.0.0 milestone Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants