Anomie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/88766
Change subject: Fix ForeignAPIRepo::fileExistsBatch()
......................................................................
Fix ForeignAPIRepo::fileExistsBatch()
ForeignAPIRepo::fileExistsBatch() is extremely broken:
* If passed an array with numeric keys (e.g. as called from
FileRepo::fileExists()), it will cache the fact that the item at index
0 from the first call was found or not found, no matter what the
actual file being queried is!
* But it doesn't even use its cache correctly. Rather than checking the
value cached, it just assumes anything that was cached exists.
* It checks whether the page in the remote repository has a description
page, not whether a file actually exists for the title.
* But it doesn't do that right either, so in the vast majority of cases
it will decide everything exists.
All of this is offset by the fact that I can't find any code path that
would actually call this function.
Change-Id: I421e8892b642f5267b8dd755183ebd711db83152
---
M includes/filerepo/ForeignAPIRepo.php
1 file changed, 21 insertions(+), 5 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/66/88766/1
diff --git a/includes/filerepo/ForeignAPIRepo.php
b/includes/filerepo/ForeignAPIRepo.php
index b1a3962..5eec9a5 100644
--- a/includes/filerepo/ForeignAPIRepo.php
+++ b/includes/filerepo/ForeignAPIRepo.php
@@ -110,8 +110,8 @@
function fileExistsBatch( array $files ) {
$results = array();
foreach ( $files as $k => $f ) {
- if ( isset( $this->mFileExists[$k] ) ) {
- $results[$k] = true;
+ if ( isset( $this->mFileExists[$f] ) ) {
+ $results[$k] = $this->mFileExists[$f];
unset( $files[$k] );
} elseif ( self::isVirtualUrl( $f ) ) {
# @todo FIXME: We need to be able to handle
virtual
@@ -129,10 +129,26 @@
$data = $this->fetchImageQuery( array( 'titles' => implode(
$files, '|' ),
'prop' => 'imageinfo' ) );
if ( isset( $data['query']['pages'] ) ) {
- $i = 0;
+ # First, get results from the query. Note we only care
whether the image exists,
+ # not whether it has a description page.
+ foreach ( $data['query']['pages'] as $p ) {
+ $this->mFileExists[$p['title']] = (
$p['imagerepository'] !== '' );
+ }
+ # Second, copy the results to any redirects that were
queried
+ if ( isset( $data['query']['redirects'] ) ) {
+ foreach ( $data['query']['redirects'] as $r ) {
+ $this->mFileExists[$r['from']] =
$this->mFileExists[$r['to']];
+ }
+ }
+ # Third, copy the results to any non-normalized titles
that were queried
+ if ( isset( $data['query']['normalized'] ) ) {
+ foreach ( $data['query']['normalized'] as $n ) {
+ $this->mFileExists[$n['from']] =
$this->mFileExists[$n['to']];
+ }
+ }
+ # Finally, copy the results to the output
foreach ( $files as $key => $file ) {
- $results[$key] = $this->mFileExists[$key] =
!isset( $data['query']['pages'][$i]['missing'] );
- $i++;
+ $results[$key] = $this->mFileExists[$file];
}
}
return $results;
--
To view, visit https://gerrit.wikimedia.org/r/88766
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I421e8892b642f5267b8dd755183ebd711db83152
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
