Skip to content

ext/intl: Fix argument position for uninitialized calendar arguments#22079

Open
LamentXU123 wants to merge 1 commit into
php:PHP-8.4from
LamentXU123:bug-fix-17
Open

ext/intl: Fix argument position for uninitialized calendar arguments#22079
LamentXU123 wants to merge 1 commit into
php:PHP-8.4from
LamentXU123:bug-fix-17

Conversation

@LamentXU123
Copy link
Copy Markdown
Contributor

@LamentXU123 LamentXU123 commented May 18, 2026

Found when looking at the code base. IntlCalendar::equals(), IntlCalendar::before(), IntlCalendar::after(), and IntlCalendar::isEquivalentTo() currently report the second parameter position used by the procedural API, even when called as methods. That is, the argument position will be error by -1.

  <?php

  $cal = IntlGregorianCalendar::createFromDate(2024, 0, 1);
  $uninit = (new ReflectionClass(IntlGregorianCalendar::class))
      ->newInstanceWithoutConstructor();

  try {
      $cal->equals($uninit);
  } catch (Error $e) {
      echo $e->getMessage(), PHP_EOL;
  }

  try {
      intlcal_equals($cal, $uninit);
  } catch (Error $e) {
      echo $e->getMessage(), PHP_EOL;
  }

Resulted in:

  IntlCalendar::equals(): Argument #2 ($other) is uninitialized
  intlcal_equals(): Argument #2 ($other) is uninitialized

But I expeected:

  IntlCalendar::equals(): Argument #1 ($other) is uninitialized
  intlcal_equals(): Argument #2 ($other) is uninitialized

See https://3v4l.org/tQdYo#v8.4.18

Ah, ideally in the future maybe we can refactor this whole parameter logic.

@devnexen
Copy link
Copy Markdown
Member

can you chewck if there no other cases like this ?

@LamentXU123
Copy link
Copy Markdown
Contributor Author

LamentXU123 commented May 18, 2026

can you chewck if there no other cases like this ?

I am quite sure there are no other similar thing near. But as the whole code base is using the same practice in argument position there may be more similar bugs in different areas...

Ideally, we shouldn't use those logic at the beginning /_ \

@devnexen
Copy link
Copy Markdown
Member

ok ... give it a bit of time ... a bug but not an awful one, I ll come it in the following days.

@LamentXU123
Copy link
Copy Markdown
Contributor Author

ok ... give it a bit of time ... a bug but not an awful one, I ll come it in the following days.

I am wondering if I can merge those little fixes myself so it might make thing easier to you. But I don't sure how to get commit right of this repo.

@devnexen
Copy link
Copy Markdown
Member

not entirely up to me (and I would say it might be a bit early for you to get access IMO).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants