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 Normal.
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.

brion added a comment.Nov 3 2011, 11:40 PM

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

brion added a comment.Nov 3 2011, 11:44 PM

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.

brion added a comment.Nov 4 2011, 12:04 AM

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.

aaron added a comment.Nov 8 2011, 7:05 PM

What is needed here after r102073?

aaron added a comment.Nov 8 2011, 7:05 PM

Also, might this be related to bug 31732?

brion added a comment.Nov 8 2011, 9:30 PM

Could use a parser test case!

aaron added a comment.Nov 8 2011, 11:17 PM

(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 moved this task from Untriaged to Done on the Multimedia board.
Gilles raised the priority of this task from Normal to Unbreak Now!.
Gilles lowered the priority of this task from Unbreak Now! to Normal.Dec 4 2014, 11:20 AM