Page MenuHomePhabricator

Language case-conversion methods (ucfirst, lcfirst, uc, lc, ucwords, ucwordbreaks, caseFold) emit PHP deprecations when passed null
Open, Needs TriagePublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Using PHP 8.2 and 8.3, I get an error when using Page Forms.
  • Edit the page with form and a error message is displayed at the top of the page
  • After inserting multiple instance templates, when saving I get the error message after using the same template either four or five times.

What happens?:

I get the error:

Deprecated: ord(): Passing null to parameter #1 ($character) of type string is deprecated in /mnt/data/html/wiki/includes/language/Language.php on line 2588

This is displayed at the top of the page when you first edit a page with a page form and when you save it if you have included more than four or five template instances in the page. Sometimes the content is saved, sometimes it isn't

What should have happened instead?:

I would expect the page to save and display the page.

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

  • MediaWiki 1.43.3
  • PHP 8.3.6 and 8.2.29

Other information (browser name/version, screenshots, etc.):

I have fixed this by updating:

public function ucfirst( $str ) {
        $octetCode = ord( $str );
        // See https://en.wikipedia.org/wiki/ASCII#Printable_characters
        if ( $octetCode < 96 ) {
                // Assume this is an uppercase/uncased ASCII character
                return (string)$str;
        } elseif ( $octetCode < 128 ) {
                // Assume this is a lowercase/uncased ASCII character
                return ucfirst( $str );
        }
        $first = mb_substr( $str, 0, 1 );
        if ( strlen( $first ) === 1 ) {
                // Broken UTF-8?
                return ucfirst( $str );
        }

The issue is surrounding:

$octetCode = ord( $str );

I have modified the code above to:

public function ucfirst( $str ) {
        // Temp fix for the ord() issue
         if ( !is_string( $str ) || $str === '' ) {
           return '';
        }

        $octetCode = ord( $str );
        // See https://en.wikipedia.org/wiki/ASCII#Printable_characters
        if ( $octetCode < 96 ) {
                // Assume this is an uppercase/uncased ASCII character
                return (string)$str;
        } elseif ( $octetCode < 128 ) {
                // Assume this is a lowercase/uncased ASCII character
                return ucfirst( $str );
        }
        $first = mb_substr( $str, 0, 1 );
        if ( strlen( $first ) === 1 ) {
                // Broken UTF-8?
                return ucfirst( $str );
        }

Event Timeline

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

What version of MW are you using?

And while MW doesn't enforce it in one way or another... The function is declared to only accept and return a string.

	/**
	 * @param string $str
	 * @return string The string with uppercase conversion applied to the first character
	 */
	public function ucfirst( $str ) {

It feels like it's this call specifically: https://github.com/wikimedia/mediawiki-extensions-PageForms/blob/edf00488e43ad5c3f0248bf2024ed0b940638217/includes/PF_Template.php#L197

An example of empty() sucking?

Do you have some missing field names in your template?

		// Finally, get any non-semantic fields defined.
		if ( preg_match_all( '/{{{([^|}]*)/mis', $this->mTemplateText, $matches ) ) {
			foreach ( $matches[1] as $fieldName ) {
				$fieldName = trim( $fieldName );
				if ( !empty( $fieldName ) && ( !in_array( $fieldName, $fieldNamesArray ) ) ) {
					$cur_pos = stripos( $this->mTemplateText, $fieldName );
					$this->mTemplateFields[$cur_pos] = PFTemplateField::create( $fieldName, PFUtils::getContLang()->ucfirst( $fieldName ) );
					$fieldNamesArray[] = $fieldName;
				}
			}
		}

I can fix this in Page Forms - unless the plan is to modify ucfirst() in MW core. Please let me know either way.

Apologies for the delay. This was on a MW 1.43.1 and 1.43.1

Due to the code changes I made, when I tried to run update.php it returned:

  • Invalid MediaWiki configuration parameter "OverrideUcfirstCharacters": Boolean value found, but an object is required

Please correct the issue before running update.php again.
If you know what you are doing, you can bypass this check
using --skip-config-validation.

Change #1178077 had a related patch set uploaded (by Yaron Koren; author: Yaron Koren):

[mediawiki/extensions/PageForms@master] Simplify and improve capitalization of autocomplete sources

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

Change #1178077 merged by jenkins-bot:

[mediawiki/extensions/PageForms@master] Simplify and improve capitalization of autocomplete sources

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

@Reedy, @Squeak24 - I looked into the relevant code in Page Forms. Reedy - I don't think that code you found is the issue, since, as you noticed, there's an empty() check above that would prevent that ucfirst() call from happening. Rather, I think it was a line in PF_ValuesUtils.php that was the issue. I just checked in a change - which I think is an improvement anyway - that may make this particular ucfirst()/ord() warning go away. Maybe a fix to core MediaWiki is still a good idea, although that seems to bring in problems of its own!

I have the same problem in 1.43.6 and PF. Is there a solution now? I am on PF master and I still see the warning message.

ah, wait, my message is a little different:

Warning
: Trying to access array offset on null in
/includes/language/Language.php
on line
2593

the wiki I am seeing this is set to langauge de-formal (happens in "de" as well)

That's unexpected. Are you sure it's due to Page Forms?

Arendpieter renamed this task from Deprecated: ord(): Passing null to parameter #1 ($character) of type string is deprecated in /mnt/data/html/wiki/includes/language/Language.php on line 2588 to Language case-conversion methods (ucfirst, lcfirst, uc, lc, ucwords, ucwordbreaks, caseFold) emit PHP deprecations when passed null.Tue, Sep 15, 3:36 PM

Change #1341929 had a related patch set uploaded (by Arendpieter; author: Arendpieter):

[mediawiki/core@master] Language: Deprecate passing null to the case-conversion methods

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

Retitled: the problem is broader than ucfirst(), and broader than ord().

To correct one thing first — this is not stale. On current master ucfirst( null ) still emits the message in the original title, along with two others:

Warning: Trying to access array offset on null
Deprecated: ord(): Passing null to parameter #1 ($character) of type string is deprecated
Deprecated: ord(): Providing an empty string is deprecated

T410920 added an early return for '' and changed ord( $str ) to ord( $str[0] ), but null takes a different path: $str[0] evaluates to null and goes straight into ord(). So the reported symptom survived that fix, on master and on REL1_43.

It is also not confined to ucfirst(). All seven case-conversion methods document @param string and enforce nothing, each failing differently:

MethodReachesDiagnostic
ucfirst, lcfirst$str[0], then ord()array offset on null + 2 ord() deprecations
uc, lc, caseFoldstrtoupper()Passing null to parameter #1
ucwords, ucwordbreakspreg_replace_callback()Passing null to parameter #3
(all of the above)strlen() in isMultibyte()Passing null to parameter #1

This has now been patched three times in the same two lines (0dec11bc1e4, then T410920 twice), each fix addressing the symptom reported at the time. A fourth null-specific guard in ucfirst() alone would be the same move again.

Change 1341929 takes a different approach. It guards all seven entry points and emits a single wfDeprecatedMsg(). Behaviour is deliberately unchanged — every one of these already returned '' for null — but the caller now gets one MediaWiki deprecation with a backtrace naming the call site, instead of a bare PHP warning pointing at Language.php.

That is the piece this task has been missing. Two people have reported this and neither could be told where the null originated; @Yaron_Koren asked directly whether Page Forms was even responsible and never got an answer. Worth noting: with the guards in place the full phpunit:unit (18839 tests) and --testsuite=includes (17965 tests) suites are clean, which they would not be if any core caller passed null — convertDeprecationsToExceptions is on. So the null is coming from an extension, and the backtrace will say which one.

Intended as the first half of a two-step change: once the deprecation has been out for a release and the callers it surfaces are fixed, these parameters can be declared string and the guards dropped. There is already precedent for native scalar types in this class (checkTitleEncoding( string $s ) and four others).