Undefined index: lower-roman in cite_body.php
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	• mmodell
	Oct 29 2015, 7:34 PM

Description

Notice: Undefined index: lower-roman in /srv/mediawiki/php-1.27.0-wmf.4/extensions/Cite/Cite_body.php on line 654

Details

	Subject	Repo	Branch	Lines +/-
	Add isset() check before accessing $this->mRefs[$group]	mediawiki/extensions/Cite	master	+1 -1

Customize query in gerrit

Revisions and Commits

rECIT extension-Cite
	Audited	rECITc00cf4a20495 Avoid counting arrays if not needed

Related Objects

Mentioned In: T117089: Deadlock found when trying to get lock; try restarting transaction
Mentioned Here: rECITc00cf4a20495: Avoid counting arrays if not needed

Event Timeline

• mmodell created this task.Oct 29 2015, 7:34 PM

• mmodell raised the priority of this task from to Needs Triage.

• mmodell updated the task description. (Show Details)

• mmodell added projects: Cite, Wikimedia-production-error.

• mmodell subscribed.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 29 2015, 7:34 PM

1.27.0-wmf.4 had to be rolled back because this was spamming logs a lot.

greg merged a task: T117087: High increase in fatalmonitor with wmf.4 to wikipedias.Oct 29 2015, 7:42 PM

Fatal logs during wmf.4 being on wikipedias: https://logstash.wikimedia.org/#dashboard/temp/AVC1I9wyptxhN1Xaf0VE

• mmodell mentioned this in T117089: Deadlock found when trying to get lock; try restarting transaction.Oct 29 2015, 7:49 PM

Looks caused by c00cf4a204954acef2f50712c38d33f3741df5f7.

I guess it happens because "!$this->mRefs[$group]" is used instead of "!isset( $this->mRefs[$group] )"?

• mmodell added a commit: rECITc00cf4a20495: Avoid counting arrays if not needed.Oct 29 2015, 8:05 PM

The gerrit code review comments are amusing: https://gerrit.wikimedia.org/r/#/c/223250/

• mmodell added a subscriber: Krinkle.Oct 29 2015, 8:10 PM

In T117084#1766300, @greg wrote:

Fatal logs during wmf.4 being on wikipedias: https://logstash.wikimedia.org/#dashboard/temp/

This task is about a PHP notice though, those shouldn't end up on fatalmonitor?

In T117084#1766416, @mmodell wrote:

The gerrit code review comments are amusing: https://gerrit.wikimedia.org/r/#/c/223250/

They are amusing and related to array counting, not array index existence. In fact, this regression supports the view that empty() has a negative impact on code quality and maintainability due to the inability to understand what code intends to do because empty() does it all.

Fortunately the rest of the Cite extension (before the above commit) already used isset() for this particular purpose. Only one place in the code used empty() instead of isset(). It was an outlier that had to be replaced with one of two things: boolean cast or isset (depending on why empty() is there). We picked the wrong one.

Fixed in https://gerrit.wikimedia.org/r/#/c/249889/.

Change 249889 had a related patch set uploaded (by Krinkle):
Add isset() check before accessing $this->mRefs[$group]

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

gerritbot added a project: Patch-For-Review.Oct 29 2015, 9:57 PM

Change 249889 merged by jenkins-bot:
Add isset() check before accessing $this->mRefs[$group]

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

ReleaseTaggerBot added a project: MW-1.27-release (WMF-deploy-2015-11-03_(1.27.0-wmf.5)).Oct 31 2015, 1:00 PM

Umherirrender closed this task as Resolved.Nov 1 2015, 5:59 PM

Umherirrender assigned this task to Krinkle.

Umherirrender set Security to None.

• demon moved this task from Untriaged to Resolved on the Wikimedia-production-error board.Nov 3 2015, 10:45 PM