Page MenuHomePhabricator

uploading file to local wiki when file exists on shared repository (commons) gives spurious info in the warning message
Closed, ResolvedPublic

Description

Author: chzz

Description:
If a filename exists on commons, and a non-admin (ie without the (reupload-shared) right) tries to upload to the same name on enwiki, it displays;

Upload warning

A file with this name exists at the Wikimedia Commons. You can:

  • go back and upload this file to Wikipedia using a different name.
  • upload it to Commons, if your intent is to replace the image that already exists with a better version.

File:$1

$1

...ie literally, "$1" instead of, presumably, showing the file.

I've also documented this issue on the enwiki page where this problem came up;

http://en.wikipedia.org/wiki/Wikipedia_talk:File_mover#Shadow_image

(It'd be nice if, when fixed, someone could append a little note there)


Version: unspecified
Severity: minor

Details

Reference
bz28034

Event Timeline

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

Guessing it's due to...

SpecialUpload.php line 450 onwards

		// Verify permissions for this title
		$permErrors = $this->mUpload->verifyPermissions( $wgUser );
		if( $permErrors !== true ) {
			$code = array_shift( $permErrors[0] );
			$this->showRecoverableUploadError( wfMsgExt( $code,
					'parseinline', $permErrors[0] ) );
			return;
		}

We don't seem to pass any sort of parameter...

r70135 looks to be the cause...

I'm having trouble actually reproducing this one...

http://commons.wikimedia.org/wiki/File:ReedyTestLol.png and then uploading http://en.wikipedia.org/wiki/File:ReedyTestLol.png to enwiki, it told me it existed (ie like "A file with this name exists already. Please check File:Chzz test image.jpg before overwriting"), I just continued, and it let me upload it in place...

This is a WFM at the moment. Can't reproduce it locally, it just doesn't seem to care much.

Halp?

So step 3 I'm currently struggling with. We know the first error message is correct, and also, so is the "can't move over commons image", it's just the one in the middle.

Will ping on the listed talk page also.

chzz wrote:

(In reply to comment #3)

I'm having trouble actually reproducing this one...

http://commons.wikimedia.org/wiki/File:ReedyTestLol.png and then uploading
http://en.wikipedia.org/wiki/File:ReedyTestLol.png to enwiki, it told me it
existed (ie like "A file with this name exists already. Please check File:Chzz
test image.jpg before overwriting"), I just continued, and it let me upload it
in place...

This is a WFM at the moment. Can't reproduce it locally, it just doesn't seem
to care much.

Halp?

So step 3 I'm currently struggling with. We know the first error message is
correct, and also, so is the "can't move over commons image", it's just the one
in the middle.

Will ping on the listed talk page also.

I was pung! Thanks;

It will be because of SysOp - more specifically, because SysOp has the (<tt>reupload-shared</tt>) right. So you'd have to test it with a non-admin account. But, it'd have to be a (<tt>confirmed</tt>) or 'auto-confirmed' account to allow upload.

If there's anything I can do, let me know. Thanks for looking.

Bingo, got it using my bot account. Taa! :)

Created attachment 8295
image showing issue

Attached:

bug28043.png (894×1 px, 95 KB)

Definitely http://en.wikipedia.org/wiki/MediaWiki:Fileexists-shared-forbidden

atm it is:
A file with this name exists at the [[Wikimedia Commons]]. You can:

  • go back and upload this file to Wikipedia using a different name.
  • [[:commons:Special:Upload|upload it to Commons]], if your intent is to replace the image that already exists with a better version. [[File:$1|thumb|center|$1]]

Raw message is:
'fileexists-shared-forbidden' => 'A file with this name exists already in the shared file repository.
If you still want to upload your file, please go back and use a new name.
[[File:$1|thumb|center|$1]]',

Need to reproduce it locally now :)

Ok, had to hack the core to get it to through me that error (the check for $user->isAllowed( 'reupload-shared' )

The issue seems to be the way

$code = array_shift( $permErrors[0] );
$this->showRecoverableUploadError( wfMsgExt( $code,

'parseinline', $permErrors[0] ) );

is working

We're only currently returning a string, not the file name. I see the same issue for 'fileexists-forbidden' in the same checkOverwrite method.

The fix I can see is changing it

from
return 'fileexists-shared-forbidden';

to

return array( 'fileexists-shared-forbidden', $file->getName() );

Which then gets double array wrapped by verifyPermissions

		$overwriteError = $this->checkOverwrite( $user );
		if ( $overwriteError !== true ) {
			return array( array( $overwriteError ) );
		}

Changing the code in SpecialUpload to

			$code = array_shift( $permErrors[0] );

			$this->showRecoverableUploadError( wfMsgExt( $code[0],
					'parseinline', $code[1] ) );

will then fix it.

I've just no idea if/how that'll actually break the more generic permission checks. I'm not sure on their format, but I'm guessing it probably will.

Will attach a patch with these propose changed

The fix might actually be changing

			$code = array_shift( $permErrors[0] );
			$this->showRecoverableUploadError( wfMsgExt( $code,
					'parseinline', $permErrors[0] ) );

into something that the $permErrors[0] is replaced by some variant of $file->getName();

Need to defer to Bryan/thedj for advice on whether I'll break the current error handling (which looks vague at best currently). Patch incoming

Created attachment 8296
Somewhat proposed fix...

Attached:

Permissions error from no protect rights..

array

0 => 
  array
    0 => string 'badaccess-groups' (length=16)
    1 => string '[[ReedyDevWiki:Administrators|Administrators]]' (length=46)
    2 => int 1

I'm taking my fix as being right...

'badaccess-groups' => 'The action you have requested is limited to users in {{PLURAL:$2|the group|one of the groups}}: $1.',

Message is index 0, $1 is index 1

Committed with 1 minor tweak in r83979

Gilles raised the priority of this task from Medium to Unbreak Now!.Dec 4 2014, 10:29 AM
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:22 AM