The "misleading" case occurs when the session is lost, resulting in a new token being created. While this is historical behavior, it's probably about time to change it. It's much more likely that the client is having a bug in cookie handling than that there was a transient session loss on the server side.

Change 586448 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] api: Report Failed rather than NeedToken on session loss for action=login

https://gerrit.wikimedia.org/r/586448

This issue has broken logins on VicuñaUploader, which still uses the deprecated method. The last upload from the tool was at 2020-04-05 21:27:08 UTC.

Thank you @Anomie,

You mentioned the cookies. As far I can see when requesting action=query&meta=tokens&type=login the server returns these cookies:

set-cookie: itwikiSession=asdasd; path=/; secure; HttpOnly
set-cookie: forceHTTPS=true; path=/; HttpOnly
Set-Cookie: WMF-Last-Access=07-Apr-2020;Path=/;HttpOnly;secure;Expires=Sat, 09 May 2020 00:00:00 GMT
Set-Cookie: WMF-Last-Access-Global=07-Apr-2020;Path=/;Domain=.wikipedia.org;HttpOnly;secure;Expires=Sat, 09 May 2020 00:00:00 GMT
Set-Cookie: GeoIP=IT:21:Turin:1.2:3.4:v4; Path=/; secure; Domain=.wikipedia.org

Having said I'm not 100.00% sure about my inspection - I think that one of my problems is that the server was replying some headers with "not-canonical" fields. See the set-cookie instead of the traditional Set-Cookie. This is maybe weird but legitimate as per RFC2616 so it was a client code fault.

What do you think about that "not-canonical" set-cookie? Can you reproduce it?

In T249526#6035209, @valerio.bozzolan wrote:

(...)
Having said I'm not 100.00% sure about my inspection - I think that one of my problems is that the server was replying some headers with "not-canonical" fields. See the set-cookie instead of the traditional Set-Cookie. This is maybe weird but legitimate as per RFC2616 so it was a client code fault.

What do you think about that "not-canonical" set-cookie? Can you reproduce it?

I faced this issue at T224712#6026486, my client is not prepared to handle set-cookie fields (it expects Set-Cookie instead) and thus fails to log in. By the way, letter case inconsistency affects several header fields (stringified Java Map with List values):

{date=[Tue, 07 Apr 2020 13:40:32 GMT],
null=[HTTP/1.1 200 OK],
X-Cache=[cp3054 miss, cp3060 pass],
server=[mw1400.eqiad.wmnet],
vary=[Accept-Encoding],
Server-Timing=[cache;desc="pass"],
x-frame-options=[DENY],
Connection=[keep-alive],
p3p=[CP="See https://pl.wiktionary.org/wiki/Special:CentralAutoLogin/P3P for more info."],
X-Client-IP=[<...>],
Accept-Ranges=[bytes],
set-cookie=[forceHTTPS=true; path=/; HttpOnly, plwiktionarySession=<...>; path=/; secure; HttpOnly],
Strict-Transport-Security=[max-age=106384710; includeSubDomains; preload],
X-Cache-Status=[pass],
content-disposition=[inline; filename=api-result.xml],
x-content-type-options=[nosniff],
Content-Encoding=[gzip],
Set-Cookie=[GeoIP=<...>; Path=/; secure; Domain=.wiktionary.org, WMF-Last-Access-Global=07-Apr-2020;Path=/;Domain=.wiktionary.org;HttpOnly;secure;Expires=Sat, 09 May 2020 12:00:00 GMT, WMF-Last-Access=07-Apr-2020;Path=/;HttpOnly;secure;Expires=Sat, 09 May 2020 12:00:00 GMT],
content-type=[text/xml; charset=utf-8],
Content-Length=[134],
cache-control=[private, must-revalidate, max-age=0],
Age=[0]}

In T249526#6035000, @AntiCompositeNumber wrote:

This issue has broken logins on VicuñaUploader, which still uses the deprecated method. The last upload from the tool was at 2020-04-05 21:27:08 UTC.

This issue can't have broken anything. This task is requesting a change, not reporting on something that broke.

In T249526#6035209, @valerio.bozzolan wrote:

What do you think about that "not-canonical" set-cookie? Can you reproduce it?

Per the RFCs, header names are case-insensitive. If some client code is expecting a particular case, that client code is broken.

The cookie issue caused VicunaUploader to break as well. Rewriting the header using mitmproxy made it work again.

This task is about the API reporting NeedToken when a token was supplied but the session was lost. Session loss may be caused by cookie handling issues, but this task isn't about that.

I fixed VicunaUploader by this commit: https://github.com/michal-josef-spacek/vicuna/commit/c4640dda4ad8b99bd88d38547afc9d7e27c3ccbd
Problem was with case-insensitive parsing of Set-Cookie.

The fix for this issue might also be the cause for T249680, Wikidata-Toolkit not being able to log in anymore.

In T249526#6039031, @Pintoch wrote:

The fix for this issue might also be the cause for T249680, Wikidata-Toolkit not being able to log in anymore.

Since the fix isn't merged yet, it can't be the cause.

Change 586448 merged by jenkins-bot:
[mediawiki/core@master] api: Report Failed rather than NeedToken on session loss for action=login

https://gerrit.wikimedia.org/r/586448

In T249526#6036972, @Anomie wrote:

Per the RFCs, header names are case-insensitive. If some client code is expecting a particular case, that client code is broken.

Having said I 100% agree with your sentence I just asked if—from the MediaWiki perspective—it's that good to adopt not-canonical header fields and... oh, brilliant! It seems the right occasion to quote the holy Postel's law:

[…] be conservative in what you do, be liberal in what you accept from others.

That's why—having said that it's 100% client side fault—I still think that the server should not act like this.

@valerio.bozzolan I suggest you move this comment to T249680 instead, as this ticket is about something else.

@valerio.bozzolan: FYI, in HTTP/2 "set-cookie" is the canonical casing of the header field, and it turns out the change is the result of an upgrade to new software using the HTTP/2 semantics. More details are available in T249680: Clients failing API login due to dependence on "Set-Cookie" header name casing.

In T249526#6039214, @gerritbot wrote:

Change 586448 merged by jenkins-bot:
[mediawiki/core@master] api: Report Failed rather than NeedToken on session loss for action=login

https://gerrit.wikimedia.org/r/586448

Should we be backporting this?

In T249526#6042873, @Reedy wrote:

Should we be backporting this?

To 1.35.0-wmf.27, or to previous releases, or both?

The main effect it'll have is that clients running into issues like T249680 or T145545 will be more likely to just report an error rather than winding up looping because they're no more likely to pick up the correct session cookies that they missed the first time around. OTOH, if there's an actual server-side session loss it'll also make clients more likely to report an error rather than retrying with the new session.

Unless we're getting hammered by looping clients from T249680, I see no particular reason to backport it to wmf.27 instead of letting it ride the wmf.28 train. I have no reason to recommend for or against backporting it to previous releases either.

Yeah, I was meaning release branches. Thanks for clarifying!