Page MenuHomePhabricator

Cannot redeclare class SpecialMultiUpload
Open, LowPublic

Description

When opening Special:SpecialPages I have the following message in debug mode.

Fatal error: Cannot redeclare class SpecialMultiUpload in /data/web/6c/03/7c/grondin.tuxfamily.org/htdocs/extensions/MultiUpload/SpecialMultiUpload.php on line 120.

I've the latest version of MultiUpload downloaded from GIT today.

Event Timeline

Grondin created this task.Mar 19 2015, 6:31 PM
Grondin raised the priority of this task from to Needs Triage.
Grondin updated the task description. (Show Details)
Grondin added a subscriber: Grondin.
Restricted Application added a project: Multimedia. · View Herald TranscriptMar 19 2015, 6:31 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Aklapper set Security to None.

I've the latest version of MultiUpload downloaded from GIT today.

Which branch of the extension?
And which version of MediaWiki is this about?

Mediawiki version 1.24

See the attachment file about git log

bertrand@portable-bertrand:~/Mediawiki/extensions/MultiUpload/.git$ git branch

  • master
Tgr updated the task description. (Show Details)Mar 20 2015, 5:38 PM
Tgr added a subscriber: Tgr.
Tgr added a comment.Mar 20 2015, 5:47 PM

Please grep your codebase for the duplicate class (install ack, run ack 'class\s+SpecialMultiUpload' ~/Mediawiki).
Also run ack 'SpecialMultiUpload.php' ~/Mediawiki.

How did you install the extension? Using composer?

Yes, i was at [http://grondin.tuxfamily.org/ grondin.tuxfamily.org].

I desinstalled the extension.

I've put it in extensions directory of my mediawiki.

result of ack 'SpecialMultiUpload.php' ~/Mediawiki :

with ack 'class\s+SpecialMultiUpload' ~/Mediawiki nothing appears

but with :
bertrand@portable-bertrand:~/Mediawiki/extensions/MultiUpload$ grep "SpecialMultiUpload" *
code:class SpecialMultiUpload extends SpecialUpload {
grep: i18n: est un dossier
MultiUpload.php:$wgSpecialPages['MultiUpload'] = 'SpecialMultiUpload';
MultiUpload.php:$wgAutoloadClasses['SpecialMultiUpload'] = DIR . '/SpecialMultiUpload.php';
MultiUpload.php:$wgAutoloadClasses['MultiUploadForm'] = DIR . '/SpecialMultiUpload.php';
MultiUpload.php:$wgAutoloadClasses['FauxWebRequestUpload'] = DIR . '/SpecialMultiUpload.php';
MultiUpload.php:$wgAutoloadClasses['DerivativeRequestWithFiles'] = DIR . '/SpecialMultiUpload.php';
MultiUpload.php:$wgAutoloadClasses['UploadRow'] = DIR . '/SpecialMultiUpload.php';
MultiUpload.php:$wgAutoloadClasses['UploadFormRow'] = DIR . '/SpecialMultiUpload.php';
grep: resources: est un dossier
SpecialMultiUpload.php:class SpecialMultiUpload extends SpecialUpload {

Tgr added a comment.Mar 20 2015, 9:03 PM

grep is not very useful because it does not recurse into subdirectories. Anyway, those results don't make sense. Maybe you are using the wrong ack? It's called ack-grep on some systems.

I cannot do the command ack or ack-grep on the server on tuxfamily because the provider has not installed these commands.

I did a clone of the file on my own computer and I launched ack-grep

bertrand@portable-bertrand:~/Mediawiki/mediawiki-1.24.1$ ack-grep 'class\s+SpecialMultiUpload' ~/Mediawiki/mediawiki-1.24.1
/home/bertrand/Mediawiki/mediawiki-1.24.1/extensions/MultiUpload/SpecialMultiUpload.php
45:class SpecialMultiUpload extends SpecialUpload {

Also getting this on http://meta.brickimedia.org. Like Grondin, ack-grep just gives me the one expected find.

Tgr added a comment.Mar 31 2015, 4:27 PM

The autoloader uses require, not require_once; probably it loads SpecialMultiUpload.php twice due to some sort of circular dependency.

Can you install Xdebug and get a stack trace for that fatal?

@Grondin / @UltrasonicNXT: Can you install Xdebug and get a stack trace for that fatal?

@Aklapper @Tgr sorry it's late, here we are:

2015/05/18 08:42:59 [error] 28010#0: *618 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Cannot redeclare class SpecialMultiUpload in /var/www/core/extensions/MultiUpload/SpecialMultiUpload.php on line 120
PHP message: PHP Stack trace:
PHP message: PHP   1. {main}() /var/www/core/index.php:0
PHP message: PHP   2. MediaWiki->run() /var/www/core/index.php:48
PHP message: PHP   3. MediaWiki->main() /var/www/core/includes/MediaWiki.php:435
PHP message: PHP   4. MediaWiki->performRequest() /var/www/core/includes/MediaWiki.php:584
PHP message: PHP   5. SpecialPageFactory::executePath() /var/www/core/includes/MediaWiki.php:275
PHP message: PHP   6. SpecialPage->run() /var/www/core/includes/specialpage/SpecialPageFactory.php:584
PHP message: PHP   7. SpecialSpecialpages->execute() /var/www/core/includes/specialpage/SpecialPage.php:363
PHP message: PHP   8. SpecialSpecialpages->getPageGroups() /var/www/core/includes/specials/SpecialSpecialpages.php:42
PHP message: PHP   9. SpecialPageFactory::getUsablePages() /var/www/core/includes/specials/SpecialSpecialpages.php:52
PHP message: PHP  10. SpecialPageFactory::getPage() /var/www/core/includes/specialpage/SpecialPageFactory.php:452
PHP message: PHP  11. AutoLoader::autoload() /var/www/core/includes/specialpage/SpecialPageFactory.php:0
PHP message: PHP  12. require() /var/www/core/includes/AutoLoader.php:1282
PHP message: PHP  13. class_exists() /var/www/core/extensions/MultiUpload/SpecialMultiUpload.php:197
PHP message: PHP  14. AutoLoader::autoload() /var/www/core/extensions/MultiUpload/SpecialMultiUpload.php:0" while reading response header from upstream, client: 86.174.154.130, server: *.brickimedia.org, request: "GET /wiki/Special:SpecialPages HTTP/1.1", upstream: "fastcgi://unix:/tmp/php5-fpm.sock:", host: "meta.brickimedia.org", referrer: "http://meta.brickimedia.org/wiki/Special:SpecialPages"

I saw a very similiar issue with:
Fatal error: Class already declared: DerivativeRequestWithFiles in /srv/mediawiki/w/extensions/MultiUpload/SpecialMultiUpload.php on line 236 on HHVM 3.8.1.

SpecialMultiUpload.php gets loaded multiple times in MultiUpload.php due to the multiple $wgAutoloadClasses pointing to SpecialMultiUpload.php. If you just load it one time, this bug (and my issue too) should be fixed. I'll submit a patch for it.

Restricted Application added a subscriber: Matanya. · View Herald TranscriptAug 24 2015, 8:26 AM

SpecialMultiUpload.php gets loaded multiple times in MultiUpload.php due to the multiple $wgAutoloadClasses pointing to SpecialMultiUpload.php. If you just load it one time, this bug (and my issue too) should be fixed. I'll submit a patch for it.

The autoloader should only load classes if they aren't already loaded. Having multiple classes in a single file and each having a $wgAutoloadClasses entry should be fine.

Perhaps something is messed up in the AutoloadClasses defintion, like having an entry for a class that doesn't exist.

SpecialMultiUpload.php gets loaded multiple times in MultiUpload.php due to the multiple $wgAutoloadClasses pointing to SpecialMultiUpload.php. If you just load it one time, this bug (and my issue too) should be fixed. I'll submit a patch for it.

The autoloader should only load classes if they aren't already loaded. Having multiple classes in a single file and each having a $wgAutoloadClasses entry should be fine.
Perhaps something is messed up in the AutoloadClasses defintion, like having an entry for a class that doesn't exist.

Honestly, the "check if FauxWebRequestUpload exists", where it declares DerivativeRequestWithFiles (no matter if that class already exists or not) and the associated else statement (that is just "class DerivativeRequestWithFiles extends DerivativeRequest { }") looks a bit messy.

The only fix for me that worked without changing $wgAutoloadClasses is replacing https://github.com/wikimedia/mediawiki-extensions-MultiUpload/blob/master/SpecialMultiUpload.php#L236 with "class_alias( 'DerivativeRequestWithFiles', 'DerivativeRequest' );". However, that likely doesn't fix the original issue discussed in this task (even while I don't experience that issue at all).

Actually looking at the code, there are many things wrong with how this extension does autoloading

  • Modifying Autoload config in the global scope of an autoloaded file (Autoload config should be done before setup finishes)
  • Using class names that have the same name as core classes (Overriding core classes is fine, but one should use different names)
  • Conditionally defining a class depending on if it exists (There might be rare circumstances where that's ok. I don't particularly think its necessary in this case)

The conditional definition of that class is left over from when the class hadn't been accepted into core, IIRC, and it should be removed for more recent MW releases. I'll try to fix that if someone else doesn't get to it first.

Jdforrester-WMF triaged this task as Low priority.Sep 4 2015, 6:56 PM
Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 7:02 PM