Strict object type checking doesn't allow backed enums.

[jeroen1602]

The library throws an exception when bStrictObjectTypeChecking is enabled and the object that needs to be mapped has a backed enum inside of it. The error states that $var must be an object, integer given.

I created an example project with a minimum setup to reproduce it.

I'm willing to create a PR for this if you want.

[patrickmaken]

Confirmed, I have the same issue

[SvenRtbg]

I was wondering how that could have escaped testing, but the test case dealing with Enums is explicitly turning off this flag.

This isn't by accident. I believe this to be a sign of a not fully discussed feature that wasn't well understood during implementation, or when making sense of it when used. No offense to anyone, as I believe everybody did the best they could at the time, given the situation and level of knowledge.

Frankly, I feel these collection of flags are confusing. Several features depend on them being set to specifics, and some are mutually exclusive: If you want @required to work, you need bStrintObjectTypeChecking to be on. But if you want to cast datetime strings into DateTimeImmutable objects, the same flag needs to be off. A very generic mentioning of the flag set to true increasing security also doesn't help much understanding the impact.

So what's the solution here? Disabling bStrictObjectTypeChecking disables the feature to be able to create an object from a scalar value. Backed enums are scalar values in the JSON. Getting an object back is disabled - so getting the exception is pretty much expected according to the feature.

As mentioned above: If this feature conflicts with real world use cases too much, we need to find a way to allow certain objects to be returned despite forbidding random objects in general. But as far as the current feature is implemented, I'd say the behavior is pretty much expected.

[SvenRtbg]

See #240 for the same problem regarding DateTime*

[ioigoume]

@SvenRtbg i am following up on the Pull Request 249 and sharing my thoughts on how i understand the whole problem. After looking into the code in more detail my proposal is to refactor the JsonMapper::createInstance strict type checking usage. Here under is a brief explanation of my thinking.

Summary

• The method createInstance is invoked with the second parameter set to true in two places.
• The flag bStrictObjectTypeChecking is also checked in exactly these two places.
• In both spots, the code checks bStrictObjectTypeChecking and then, if not strict, proceeds to create an instance with the provided JSON value as a constructor argument.
• This duplication suggests extracting a helper responsible for the “unsafe” instantiation path and centralizing the strictness check and error messaging.

Current code example (around line 485):

// Use constructor parameter if we have a class but only a flat type (i.e., string, int)
if ($jvalue === null) {
    $array[$key] = null;
} else {
    if ($this->isSimpleType($class)) {
        settype($jvalue, $class);
        $array[$key] = $jvalue;
    } elseif ($this->bStrictObjectTypeChecking) {
        throw new JsonMapper_Exception(
            'JSON property "' . ($parent_key ? $parent_key : '?') . '[' . $key . ']" must be an object, ' . gettype($jvalue) . ' given'
        );
    } else {
        $array[$key] = $this->createInstance($class, true, $jvalue);
    }
}

Proposed refactor for that block:

// Use constructor parameter if we have a class but only a flat type (i.e., string, int)
if ($jvalue === null) {
    $array[$key] = null;
} elseif ($this->isSimpleType($class)) {
    settype($jvalue, $class);
    $array[$key] = $jvalue;
} else {
    if ($this->bStrictObjectTypeChecking) {
        throw new JsonMapper_Exception(sprintf(
            'JSON property "%s[%s]" must be an object, %s given',
            $prop,
            $key,
            gettype($jvalue)
        ));
    }
    $array[$key] = $this->createInstance($class, true, $jvalue);
}

Rationale for extraction
The other occurrence of the bStrictObjectTypeChecking check (around line 333) is structurally identical; only the error message differs:

// Use constructor parameter if we have a class but only a flat type (i.e., string, int)
if ($this->bStrictObjectTypeChecking) {
    throw new JsonMapper_Exception(
        'JSON property "' . $key . '" must be an object, ' . gettype($jvalue) . ' given'
    );
}
$child = $this->createInstance($type, true, $jvalue);

Given the duplication, we can extract the “unsafe instantiation” logic into a dedicated method, for example createUnsafeInstance, and simplify createInstance by removing the $useParameter toggle.

Refactoring steps

1. Move the instantiation logic from createInstance into createUnsafeInstance:

if (PHP_VERSION_ID >= 80100 && is_subclass_of($class, \BackedEnum::class)) {
    return $class::from($jvalue);
}

return new $class($jvalue);

2. Remove the $useParameter argument from createInstance.

3. Implement createUnsafeInstance with explicit handling for enums and clear behavior under strict checking:

public function createUnsafeInstance(string $class, $jvalue): object
{
    // Enumerations
    if (PHP_VERSION_ID >= 80100 && is_subclass_of($class, \BackedEnum::class)) {
        $evalue = $class::tryFrom($jvalue);
        if ($evalue === null) {
            throw new JsonMapper_Exception(
                'Enum value "' . $jvalue . '" does not belong to ' . $class . ' enum'
            );
        }
        return $evalue;
    }

    // TODO: Consider special handling for \DateTimeInterface or other known mappable classes.

    // Unsafe passthrough
    if (!$this->bStrictObjectTypeChecking) {
        return new $class($jvalue);
    }

    // In strict mode, reaching here implies a violation; either throw or return a neutral value.
    throw new JsonMapper_Exception(
        'Strict object type checking is enabled; cannot instantiate ' . $class . ' from non-object JSON value (' . gettype($jvalue) . ')'
    );
}