getid3: PHP Warning: Division by zero
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error

mwversion: 1.37.0-wmf.18
reqId: 652c0845-f249-494e-aab4-9f33079bd5f8
Find reqId in Logstash
Find normalized_message in Logstash

normalized_message

[{reqId}] {exception_url}   PHP Warning: Division by zero

exception.trace

from /srv/mediawiki/php-1.37.0-wmf.18/vendor/james-heinrich/getid3/getid3/module.audio-video.mpeg.php(619)
#0 /srv/mediawiki/php-1.37.0-wmf.18/vendor/james-heinrich/getid3/getid3/module.audio-video.mpeg.php(619): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.37.0-wmf.18/vendor/james-heinrich/getid3/getid3/module.audio-video.mpeg.php(180): getid3_mpeg::videoAspectRatioLookup(integer, integer, integer, integer)
#2 /srv/mediawiki/php-1.37.0-wmf.18/vendor/james-heinrich/getid3/getid3/getid3.php(636): getid3_mpeg->Analyze()
#3 /srv/mediawiki/php-1.37.0-wmf.18/extensions/TimedMediaHandler/includes/handlers/ID3Handler/ID3Handler.php(32): getID3->analyze(string)
#4 /srv/mediawiki/php-1.37.0-wmf.18/extensions/TimedMediaHandler/includes/handlers/ID3Handler/ID3Handler.php(51): ID3Handler->getID3(string)
#5 /srv/mediawiki/php-1.37.0-wmf.18/includes/media/MediaHandler.php(240): ID3Handler->getMetadata(FSFile, string)
#6 /srv/mediawiki/php-1.37.0-wmf.18/includes/utils/MWFileProps.php(88): MediaHandler->getSizeAndMetadataWithFallback(FSFile, string)
#7 /srv/mediawiki/php-1.37.0-wmf.18/includes/upload/UploadBase.php(552): MWFileProps->getPropsFromPath(string, string)
#8 /srv/mediawiki/php-1.37.0-wmf.18/includes/upload/UploadFromChunks.php(382): UploadBase->verifyPartialFile()
#9 /srv/mediawiki/php-1.37.0-wmf.18/includes/upload/UploadFromChunks.php(241): UploadFromChunks->verifyChunk()
#10 /srv/mediawiki/php-1.37.0-wmf.18/includes/api/ApiUpload.php(274): UploadFromChunks->addChunk(string, integer, integer)
#11 /srv/mediawiki/php-1.37.0-wmf.18/includes/api/ApiUpload.php(153): ApiUpload->getChunkResult(array)
#12 /srv/mediawiki/php-1.37.0-wmf.18/includes/api/ApiUpload.php(124): ApiUpload->getContextResult()
#13 /srv/mediawiki/php-1.37.0-wmf.18/includes/api/ApiMain.php(1842): ApiUpload->execute()
#14 /srv/mediawiki/php-1.37.0-wmf.18/includes/api/ApiMain.php(821): ApiMain->executeAction()
#15 /srv/mediawiki/php-1.37.0-wmf.18/includes/api/ApiMain.php(792): ApiMain->executeActionWithErrorHandling()
#16 /srv/mediawiki/php-1.37.0-wmf.18/api.php(90): ApiMain->execute()
#17 /srv/mediawiki/php-1.37.0-wmf.18/api.php(45): wfApiMain()
#18 /srv/mediawiki/w/api.php(3): require(string)
#19 {main}

Impact

Unclear, but probably some display error. Wrong aspect ratio?

Notes

Noticed 2 of these in 1.37.0-wmf.18 during train log triage.

Details

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

Subject	Repo	Branch	Lines +/-
Replace wikimedia/getid3 v1.9.22.2 with james-heinrich/getid3 v1.9.23	mediawiki/extensions/TimedMediaHandler	master	+1 -1
Replace wikimedia/getid3 v1.9.22.2 with james-heinrich/getid3 v1.9.23	mediawiki/vendor	master	+375 -7 K
Switch james-heinrich/getid3 out for wikimedia/getid3	mediawiki/vendor	master	+7 K -326
Update getid3 from upstream and pin tag	mediawiki/extensions/TimedMediaHandler	master	+1 -1
Replace james-heinrich/getid3 v1.9.22 with wikimedia/getid3 v1.9.22.2	mediawiki/vendor	master	+8 K -1 K
HTTP Live Streaming (HLS for iOS) and WebM transcode cleanup	mediawiki/extensions/TimedMediaHandler	master	+3 K -568

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Stalled		None	T255792 Quibble runs core:unit tests twice!
Open		None	T328919 Upgrade to PHPUnit 10
Open		None	T376276 PHP 8.4: Implicitly nullable parameter declarations deprecated
Open		None	T379587 PHP 8.4: Implicitly nullable parameter declarations deprecated in 3rd party vendor libs
Open		None	T378424 PHP 8.4: Implicitly nullable parameter declarations deprecated in qrcode vendor libs
Open		None	T358666 Drop PHP 8.1 support from MediaWiki
Open		None	T380185 Prepare for newer geoip2/geoip2 library
Open		None	T338103 Micro-optimize ApiResult::isMetadataKey with str_starts_with once we support PHP8+
Open		None	T379930 Use WeakMap instead of an array in RestrictedEntityLookupFactory
Stalled		None	T334726 Use return type `never` in Wikibase
Stalled		None	T373753 Drop symfony/polyfill-php80 and symfony/polyfill-php81 from mediawiki-vendor
Open		None	T328921 Drop PHP 7.4 support from MediaWiki
Open		None	T328922 Drop PHP 8.0 support from MediaWiki
Stalled		None	T319055 Upgrade to psr/container 2.x
Stalled		Krinkle	T319432 Migrate WMF production from PHP 7.4 to PHP 8.1
Resolved		Jdforrester-WMF	T316078 Make PHP 8.1 voting on development (master) branch of MW ecosystem (core, extensions, skins, libraries)
Resolved		Jdforrester-WMF	T319065 All Wikimedia production extensions and skins must support PHP 8.1
Resolved	PRODUCTION ERROR	• brooke	T289189 getid3: PHP Warning: Division by zero

Event Timeline

brennen created this task.Aug 18 2021, 6:45 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 18 2021, 6:45 PM

brennen moved this task from Backlog to Logs/Train on the User-brennen board.Aug 18 2021, 6:46 PM

We've not seen this before, and there were a number of recent patches merged in TMH. Perhaps one of those caused this regression? /cc @brion.

• brooke moved this task from To sort to In progress on the TimedMediaHandler board.Dec 22 2021, 10:36 PM

Looks like it's related to the MPEG file uploads aspect ratio handling, one of the cases may be bad and is failing on specific files being uploaded from time to time. Let me see if I can narrow it down; the logs don't include the source material being uploaded alas so I can't just inspect a held file. :(

The entire data extraction through to the exception is performed within the getid3 library and is specific to files that are being interpreted as MPEG-2 video. It looks sensible enough, unless it contravenes the spec or the input files are corrupt I'm not sure what could cause this failure.

Arguably throwing an exception here is correct behavior, though it might be wise to wrap this low-level arithmetic error in a high-level exception that aids in understanding the error report.

Agh dangit it's not catchable is it. Silly PHP.

Ah well, I'll open a protective patch on the getid3 library and we can apply it later with other updates.

Yeah, this one may be my fault. I wrote the code that's failing in getid3. lol

Upstream pull req with quick workaround to avoid failing (but may result in wrong interpretation of aspect ratio): https://github.com/JamesHeinrich/getID3/pull/359

We can pull it in once it hits, no need to go out of sequence I think unless we can find specific files triggering it first.

Going ahead and checking for any other files that cause problems while I'm at it though. I'll run the tests tomorrow after downloading the data set.

Bumping prio to medium since it may mean rejection of legit files (don't know yet)

upstream => Upstream

Aklapper moved this task from Backlog to Patch merged upstream on the Upstream board.Dec 23 2021, 3:41 PM

I found a previously-uploaded file that triggers the error and am trying to track it down inside getid3. Filed an upstream bug w/ the link & repro: https://github.com/JamesHeinrich/getID3/issues/360

Confirmed with upstream that the workaround is good enough for now, so we either wait until he gets a release out or we patch it locally. I've found about a half dozen files already uploaded that trigger this bug when re-parsed, so those may also trigger it this bug if something causes metadata to be re-read from the file.

They are:

Born_of_a_Dream_-_Extra_Feature.mpg
Fröbel-Stern_basteln_(98MB).mpg
Prometei_–_Marco_Bertarini_teaser.mpg
Ruffed_Lemur_at_Ikeda_zoo_Japan.mpg
Zdravko_Pečar_-_Elephant_Hunt.mpg
لغو_ناگهانی_سفر_مدیر_اجرایی_بنیاد_ویکی?مدیا_به_ایران.mpg

They're all MPEG-2 but there's a lot of difference in resolutions and frame rates. Anyway if we have more trouble with them, that's the ones we got.

TheDJ mentioned this in T305042: Handle mpeg files with an aspect ratio of 0.Apr 4 2022, 6:33 PM

tstarling mentioned this in T303784: InvalidArgumentException: Cannot add non-finite floats to ApiResult.Nov 13 2022, 11:36 PM

Per the stack trace I posted at T303784, this also affects module.audio-video.matroska.php, line 299. I found a few more issues by code review. I'll make a PR.

In PHP 8.1, division by zero will throw a DivisionByZeroError.

https://github.com/JamesHeinrich/getID3/pull/404

Reedy moved this task from Backlog to Wikimedia production on the PHP 8.1 support board.Nov 26 2022, 7:53 AM

Reedy added a parent task: T319065: All Wikimedia production extensions and skins must support PHP 8.1.Jan 6 2023, 12:40 AM

Reedy changed the task status from In Progress to Stalled.Jul 7 2023, 5:03 PM

Change 888279 had a related patch set uploaded (by Brion VIBBER; author: Brion VIBBER):

[mediawiki/extensions/TimedMediaHandler@master] HTTP Live Streaming (HLS for iOS) and WebM transcode cleanup

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

gerritbot added a project: Patch-For-Review.Sep 11 2023, 6:03 PM

Above patch includes an update to our newly forked copy of getid3 and should include all previously merged but unreleased patches, as well as my additional recent patches for MPEG parsing.

brennen unsubscribed.Sep 11 2023, 7:23 PM

Change 888279 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] HTTP Live Streaming (HLS for iOS) and WebM transcode cleanup

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

with the merge of the TMH updates including the forked getid3 library, this fix'll go out with the next train. Yay!

In T289189#9208311, @brion wrote:

with the merge of the TMH updates including the forked getid3 library, this fix'll go out with the next train. Yay!

Not yet. wikimedia/getid3 isn't published yet, and production is still pinned to james-heinrich/getid3. Can you get it published so I can update vendor?

Change 964617 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/vendor@master] [DNM] Switch james-heinrich/getid3 out for wikimedia/getid3

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

In T289189#9237371, @Jdforrester-WMF wrote:

In T289189#9208311, @brion wrote:

with the merge of the TMH updates including the forked getid3 library, this fix'll go out with the next train. Yay!

Not yet. wikimedia/getid3 isn't published yet, and production is still pinned to james-heinrich/getid3. Can you get it published so I can update vendor?

Added to packagist (mediawiki and wikimedia accounts as owner), set the update hook...

https://packagist.org/packages/wikimedia/getid3

In T289189#9237371, @Jdforrester-WMF wrote:

In T289189#9208311, @brion wrote:

with the merge of the TMH updates including the forked getid3 library, this fix'll go out with the next train. Yay!

Not yet. wikimedia/getid3 isn't published yet, and production is still pinned to james-heinrich/getid3. Can you get it published so I can update vendor?

And we should probably tag a release to use in vendor, if we're going to do that.

Is there any reason we can't get upstream to make a release/similar?

https://github.com/wikimedia/getID3/compare/master...JamesHeinrich:getID3:master has a couple of bug fixes too that we don't have...

Not yet. wikimedia/getid3 isn't published yet, and production is still pinned to james-heinrich/getid3. Can you get it published so I can update vendor?

should be a pin in there for the git repo. [ahh i see, it's better for the manual vendoring dir to use the packagist system directly instead of raw git branches? sorry! i'll try and fix this up right :D]

Is there any reason we can't get upstream to make a release/similar?

We could ask, but this would slow the cycle of every future fix even if he agrees to make releases on request. [ideally we will improve this and asymptotically approach a re-merge, we'll see :D]

Change 966572 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/vendor@master] Replace james-heinrich/getid3 v1.9.22 with wikimedia/getid3 v1.9.22.2

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

Change 965780 had a related patch set uploaded (by Jforrester; author: Brion VIBBER):

[mediawiki/extensions/TimedMediaHandler@master] Update getid3 from upstream and pin tag

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

Change 966572 merged by jenkins-bot:

[mediawiki/vendor@master] Replace james-heinrich/getid3 v1.9.22 with wikimedia/getid3 v1.9.22.2

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

Change 965780 merged by jenkins-bot:

[mediawiki/extensions/TimedMediaHandler@master] Update getid3 from upstream and pin tag

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

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.2; 2023-10-24).Oct 17 2023, 7:00 PM

Jdforrester-WMF closed this task as Resolved.Oct 18 2023, 6:07 PM

Change 964617 abandoned by Jforrester:

[mediawiki/vendor@master] Switch james-heinrich/getid3 out for wikimedia/getid3

Reason:

Done in I34c572636ad9bede46350e83667ef6e0c4ff2097 instead.

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

Change 967217 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/vendor@master] Replace wikimedia/getid3 v1.9.22.2 with james-heinrich/getid3 v1.9.23

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

Change 967218 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/TimedMediaHandler@master] Replace wikimedia/getid3 v1.9.22.2 with james-heinrich/getid3 v1.9.23

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

Change 967217 merged by jenkins-bot: