Page MenuHomePhabricator

#ifexist does not say exist to existing redirects to file when using media pseudo-namespace
Closed, ResolvedPublic

Description

See url, the ifexist call for the file redirect (with Media:) say "not exist", but the image exist under the redirect target.


Version: 1.18.x
Severity: normal
URL: http://test.wikipedia.org/wiki/Ifexist_and_file_redirect

Details

Reference
bz32031

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:59 PM
bzimport set Reference to bz32031.
bzimport added a subscriber: Unknown Object (MLST).

Changing title of bug to reflect the issue is with media pseudo-namespace (to differentiate from redirects between different files)

Whoops, sorry. I totally misread what you wrote. Changing title back.

To clarify the issue is:

checking existence of Media:<some existing image, but possibly with no file description page, if for example from a foriegn image repo> returns true. but checking existence of Media:<some redirect to an image, possibly also on a foreign repo> returns false where we would expect it to return true. And checking existence of File:<something> just checks that the file description page exists.

So we would want Media:<some redirect to an image, possibly also on a foreign repo> to be considered existing.

Looking at ParserFunctions it looks like it should work fine, as long as wfFindFile is returning a sensible File object for the target -- it's documented that you have to pass an option to disable redirects on the lookup, so *should* "just work".

I'm having trouble repro'ing locally as I'm not receiving imageinfo API data for the redirect on Commons.... so something else may be awry as well. :P

bug 31849 covers some API issues which may be related; looking up the redir pages results in some missing data. Could be my ForeignAPIRepo problem, could also be related to the {{#ifexist}} itself.

Ok on a local test I can confirm that wfFindFile and its cousins resolve redirects for File: title object but, for some reason, not for Media: title objects.

Unfortunately it looks like there are a *LOT* of entry points into File / FileRepo functions that take 'string or Title object' and are .... sloppy in validation.

Strings usually get Title'ized via Title::makeTitleSafe with an NS_FILE, but a Title object passed directly in doesn't get validated for namespace.

This probably leads to an NS_MEDIA Title object floating around, and eventually some lookup goes to that in the page table and fails.

There should probably be a consistent input validator used by all of these functions, which will do string -> Title conversions *and* validate the namespace. This could alias Media to File and drop any other namespaces as invalid for files...

(In reply to comment #5)

a Title object passed directly in doesn't get validated for namespace.

r76437 add a check on RepoGroup::findFile, which is used by wfFindFile and that is the main entry point.

What is needed here after r102073?

Also, might this be related to bug 31732?

(In reply to comment #9)

Could use a parser test case!

How do fake a file existing in the DB? I think a more direct Media: w/ checkRedirect() test would work out better.

Change 134963 had a related patch set uploaded by Brian Wolff:
Add unit tests for #ifexist in NS_MEDIA with file redirects

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

Change 134963 merged by jenkins-bot:
Add unit tests for #ifexist in NS_MEDIA with file redirects

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

Gilles raised the priority of this task from Medium to Unbreak Now!.Dec 4 2014, 10:19 AM
Gilles added a project: Multimedia.
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to Medium.Dec 4 2014, 11:20 AM