MimeAnalyzer::improveTypeFromExtension() must be of the type string, null given
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error message

Argument 2 passed to MimeAnalyzer::improveTypeFromExtension() must be of the type string, null given, called in /srv/mediawiki/php-1.36.0-wmf.28/includes/utils/MWFileProps.php on line 79

Stack Trace

Argument 2 passed to MimeAnalyzer::improveTypeFromExtension() must be of the type string, null given, called in /srv/mediawiki/php-1.36.0-wmf.28/includes/utils/MWFileProps.php on line 79
from /srv/mediawiki/php-1.36.0-wmf.28/includes/libs/mime/MimeAnalyzer.php(458)
#0 /srv/mediawiki/php-1.36.0-wmf.28/includes/utils/MWFileProps.php(79): MimeAnalyzer->improveTypeFromExtension(string, NULL)
#1 /srv/mediawiki/php-1.36.0-wmf.28/includes/upload/UploadBase.php(547): MWFileProps->getPropsFromPath(string, NULL)
#2 /srv/mediawiki/php-1.36.0-wmf.28/includes/upload/UploadBase.php(482): UploadBase->verifyPartialFile()
#3 /srv/mediawiki/php-1.36.0-wmf.28/includes/upload/UploadBase.php(390): UploadBase->verifyFile()
#4 /srv/mediawiki/php-1.36.0-wmf.28/includes/upload/UploadFromFile.php(96): UploadBase->verifyUpload()
#5 /srv/mediawiki/php-1.36.0-wmf.28/includes/api/ApiUpload.php(610): UploadFromFile->verifyUpload()
#6 /srv/mediawiki/php-1.36.0-wmf.28/includes/api/ApiUpload.php(87): ApiUpload->verifyUpload()
#7 /srv/mediawiki/php-1.36.0-wmf.28/includes/api/ApiMain.php(1612): ApiUpload->execute()
#8 /srv/mediawiki/php-1.36.0-wmf.28/includes/api/ApiMain.php(592): ApiMain->executeAction()
#9 /srv/mediawiki/php-1.36.0-wmf.28/includes/api/ApiMain.php(563): ApiMain->executeActionWithErrorHandling()
#10 /srv/mediawiki/php-1.36.0-wmf.28/api.php(90): ApiMain->execute()
#11 /srv/mediawiki/php-1.36.0-wmf.28/api.php(45): wfApiMain()
#12 /srv/mediawiki/w/api.php(3): require(string)
#13 {main}

Impact

Unknown. 4 of these seen in 1.36.0-wmf.28 (T271342) so far.

Tentatively tagging Platform Engineering.

Details

Request ID: YBNB5gpAEJ8AAUoLP8MAAADT
Request URL: https://commons.wikimedia.org/w/api.php

	Subject	Repo	Branch	Lines /-
	Document and fix UploadBase::$mFinalExtension being null	mediawiki/core	master	10 -10
	MWFileProps: Hotfix getPropsFromPath() to treat null as empty string	mediawiki/core	master	4 -1

Customize query in gerrit

Related Objects

Mentioned Here: T271342: 1.36.0-wmf.28 deployment blockers

Event Timeline

brennen created this task.Jan 28 2021, 11:29 PM

Restricted Application added a project: Performance-Team. · View Herald TranscriptJan 28 2021, 11:29 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

brennen moved this task from Backlog to Logs/Train on the User-brennen board.Jan 28 2021, 11:30 PM

Reedy added a project: MediaWiki-File-management.Jan 29 2021, 2:22 AM

Restricted Application added a project: Commons. · View Herald TranscriptJan 29 2021, 2:22 AM

Caused by strict typing in https://gerrit.wikimedia.org/r/c/mediawiki/core/ /656981

• Gilles assigned this task to Krinkle.Feb 1 2021, 7:53 PM

• Gilles moved this task from Inbox, needs triage to Doing (old) on the Performance-Team board.

dancy added a parent task: T271342: 1.36.0-wmf.28 deployment blockers.Feb 1 2021, 8:53 PM

Trace

#0 /srv/mediawiki/php-1.36.0-wmf.28/includes/utils/MWFileProps.php(79): MimeAnalyzer->improveTypeFromExtension(string, NULL)
#1 /srv/mediawiki/php-1.36.0-wmf.28/includes/upload/UploadBase.php(547): MWFileProps->getPropsFromPath(string, NULL)

Looks like the bug might not be in MWFIleProps either, but even higher since the call is invalid there as well:

MWFileProps

	 * @param string $path Filesystem path to a file
	 * @param string|bool $ext The file extension, or true to extract it from the filename.
	 *             Set it to false to ignore the extension.
	 * @return array
	 * @since 1.28
	 */
	public function getPropsFromPath( $path, $ext ) {
		// …
			$info['file-mime'] = $this->magic->guessMimeType( $path, false );
			$ext = ( $ext === true ) ? FileBackend::extensionFromPath( $path ) : $ext;
			// …
			$info['mime'] = $this->magic->improveTypeFromExtension( $info['file-mime'], $ext );

This is expected to change $ext from string|bool to string, where true means FileBackend::extensionFromPath is used and otherwise $ext (already a string) used as-is. I've confirmed that FileBackend::extensionFromPath always returns a string, with the default being an empty string if the file path has no extension, e.g. /foo or /.foo.

But, the trace above shows that getPropsFromPath( string, null ) is called, which is invalid/unexpected.

dancy added a parent task: T271343: 1.36.0-wmf.29 deployment blockers.Feb 1 2021, 9:00 PM

taavi triaged this task as Unbreak Now! priority.Feb 1 2021, 9:16 PM

RhinosF1 subscribed.Feb 1 2021, 9:41 PM

As a quick patch to unblock things, can we cast $ext to a string so that null becomes ''?

In the getPropsFromPath method? Yeah, that should do.

Change 660965 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] MWFileProps: Hotfix getPropsFromPath() to treat null as empty string

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

gerritbot added a project: Patch-For-Review.Feb 2 2021, 12:52 AM

Change 660965 merged by jenkins-bot:
[mediawiki/core@master] MWFileProps: Hotfix getPropsFromPath() to treat null as empty string

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.30; 2021-02-09).Feb 2 2021, 2:00 AM

Maintenance_bot removed a project: Patch-For-Review.Feb 2 2021, 2:10 AM

Change 661044 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/core@master] Fix incomplete getTitle call in UploadBase::verifyPartialFile

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

gerritbot added a project: Patch-For-Review.Feb 2 2021, 7:48 AM

• holger.knust edited projects, added Platform Team Workboards (Clinic Duty Team); removed Platform Engineering.Feb 2 2021, 3:09 PM

• holger.knust moved this task from Inbox to Ready (WIP:5) on the Platform Team Workboards (Clinic Duty Team) board.

• holger.knust added a subscriber: daniel.

Change 661142 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Add regression test for T273249

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

If the issue is caused by the introduction of strict type ( https://gerrit.wikimedia.org/r/c/mediawiki/core/ /656981 ), can we revert that change in the interest of unblocking the train? That would gives time to polish up follow up changes to stabilize master before next week train.

In T273249#6797249, @hashar wrote:

If the issue is caused by the introduction of strict type ( https://gerrit.wikimedia.org/r/c/mediawiki/core/ /656981 ), can we revert that change in the interest of unblocking the train? That would gives time to polish up follow up changes to stabilize master before next week train.

The train blocker is fixed. It remains open for a 'proper' fix, but that doesn't need to block the train.

I missed:

[mediawiki/core@master] MWFileProps: Hotfix getPropsFromPath() to treat null as empty string
https://gerrit.wikimedia.org/r/660965

Thanks @Jdforrester-WMF I am removing the task from the list of blockers.

hashar lowered the priority of this task from Unbreak Now! to Medium.Feb 2 2021, 8:20 PM

hashar removed parent tasks: T271343: 1.36.0-wmf.29 deployment blockers, T271342: 1.36.0-wmf.28 deployment blockers.

Krinkle reassigned this task from Krinkle to thiemowmde.Feb 3 2021, 1:49 AM

Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team, Wikimedia-production-error.

Krinkle subscribed.

Change 661044 merged by jenkins-bot:
[mediawiki/core@master] Document and fix UploadBase::$mFinalExtension being null