Use strict mode for "in_array()" functions in "\Drupal\serialization\Normalizer\NormalizerBase" for verifying supported types for (de-)normalization

I think we could just add the third parameter to the in_array() call in supportsDenormalization(), not sure what other tests we need there? I guess we can just add a couple of tests to \Drupal\Tests\serialization\Unit\Normalizer\NormalizerBaseTest::testSupportsNormalization.

The !isset() vs empty() for the $format check, I'm not sure of the benefits. If it's something empty like '' or 0 it's unlikely that the format property will match that. Seems like too much baby sitting, potentially?

#4 sounds sensible. Removed point 2 from the IS per #4.

Everything except the tests should be doable by a novice, so adding tags.

+++ Thanks, Wim!

Here is the patch which adds third argument TRUE to in_array functions.

Thanks! Now task 1 is done. Updated issue summary.

Now task 2 remains.

Thanks Wim for reviewing the patch.
Could you please help me how to create Tests as I am creating it for the first time.

Please see comment #4, I think the test I mentioned there can be added to, to cover this edge case.

Thanks damiankloip. I will work on the Tests as mentioned in #4

Test case : \Drupal\Tests\serialization\Unit\Normalizer\NormalizerBaseTest::testSupportsNormalization
if (!isset($format) && empty($format)) {
$this->assertNull($format, 'Null Value');
}

Could you please manually review if it looks good so that I can create the patch.

\Drupal\Tests\serialization\Unit\Normalizer\NormalizerBaseTest::testSupportsNormalization() is testing supportsNormalization(). The change made in #7 modifies supportsDenormalization().

So it's not quite correct yet, but you're thinking along the right lines.

The point here is: add a test that fails if #7 is not applied, and hence demonstrates the bug. Then apply #7, and the test should pass.

Right. You will want to add a new test method (and data provider maybe) for supportsDenormalization I think.

Pinged @Dinesh18.

I have tried creating one default function testSupportsDenormalization() mentioned below. It is hard-coded which always returns TRUE.
Could you please help me how can I pass values to function supportsDenormalization($data, $type, $format = NULL).

class NormalizerBaseTest extends UnitTestCase {

  /**
   * Tests the supportsDenormalization method.
   *
 public function testSupportsDenormalization() {
    $data = array('1.10', 12.4, 1.13);
    $output = in_array(1.13, $data, true);
    $this->assertSame($output, TRUE);
  }

}

@Dinesh18: you want to create a "data provider" function. See \Drupal\Tests\serialization\Unit\Normalizer\NormalizerBaseTest::testSupportsNormalization()'s docblock + \Drupal\Tests\serialization\Unit\Normalizer\NormalizerBaseTest::providerTestSupportsNormalization() for an example.

I'll take a crack at writing this test at the PNWDS.

Here's my attempt at testSupportsDenormalization. This test should have 3 failures with the current patch because I did not include the solution in#7.

This new patch includes the fix in #7 and should pass all tests.

1. +++ b/core/modules/serialization/tests/src/Unit/Normalizer/NormalizerBaseTest.php
   @@ -61,6 +61,50 @@ public function providerTestSupportsNormalization() {
   +    $normalizer_base = $this->getMockForAbstractClass('Drupal\Tests\serialization\Unit\Normalizer\TestNormalizerBase');
   

   This should be TestNormalizerBase::class, to make refactoring easier.

2. +++ b/core/modules/serialization/tests/src/Unit/Normalizer/NormalizerBaseTest.php
   @@ -61,6 +61,50 @@ public function providerTestSupportsNormalization() {
   +  public function providerTestSupportsDenormalization() {
   

   This data provider should also test positive return values, not just negative return values.

   So at least one of them shuold be

   [TRUE, new \stdClass, '123', $supported_types],
   

Awesome, thanks for fixing those @Saviktor. This looks done.

Did changes as per comment #31, please review it :)

+++ b/core/modules/serialization/tests/src/Unit/Normalizer/NormalizerBaseTest.php
@@ -75,7 +75,7 @@
+  public function testSupportsDenormalization($expected_return, $data, $supported_interface_or_class = NULL, $supported_types = NULL) {
...
     if (!empty($supported_types)) {

then makes sense to change condition to isset()

1. +++ b/core/modules/serialization/tests/src/Unit/Normalizer/NormalizerBaseTest.php
   @@ -61,6 +61,58 @@ public function providerTestSupportsNormalization() {
   +   * @param mixed $data
   +   *   The data passed to supportsNormalization.
   ...
   +      [FALSE, new \stdClass(), '1.10', $supported_types],
   

   This is always new \stdClass, so there's no need to pass it in.

2. +++ b/core/modules/serialization/tests/src/Unit/Normalizer/NormalizerBaseTest.php
   @@ -61,6 +61,58 @@ public function providerTestSupportsNormalization() {
   +      // If string '1.10' type is passed should return FALSE.
   

   These comments should be the keys to the test cases: that's how you label test cases. Then if a test case fails, you get that label instead of a number!

This patch is now changing much more.

1. +++ b/core/modules/serialization/src/Normalizer/NormalizerBase.php
   @@ -42,11 +42,23 @@ public function supportsNormalization($data, $format = NULL) {
   +   * @param string|object $type
   
   @@ -56,11 +68,21 @@ public function supportsDenormalization($data, $type, $format = NULL) {
   +    if (
   +      // Object types should be checked.
   +      is_object($type) ||
   

   When can this ever be an object? That is not actually allowed by \Symfony\Component\Serializer\Normalizer\DenormalizerInterface::supportsDenormalization().

2. +++ b/core/modules/serialization/tests/src/Unit/Normalizer/NormalizerBaseTest.php
   @@ -61,6 +61,64 @@ public function providerTestSupportsNormalization() {
   +   * @param string|int|float $type
   

   This is not actually allowed to contain an int or a float. Again, see \Symfony\Component\Serializer\Normalizer\DenormalizerInterface::supportsDenormalization().

I would really love to see something like META Use strict mode for "in_array()" calls as parent of this to fix this big bug possibility once and for all the core.

RE #42 - read issue description and check the link there.

Here is a patch for 11.2.3 based on #40

https://www.drupal.org/files/issues/2025-09-03/2864495-strict-mode-for-d...