Skip to content

SoapClient can't convert BackedEnum to scalar value #15711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MrMeshok opened this issue Sep 2, 2024 · 11 comments
Closed

SoapClient can't convert BackedEnum to scalar value #15711

MrMeshok opened this issue Sep 2, 2024 · 11 comments

Comments

@MrMeshok
Copy link

MrMeshok commented Sep 2, 2024

Description

The following code:

<?php
enum TestBackedEnum: string
{
    case First = 'first';
}

$client = new SoapClient('https://raw.githubusercontent.com/php/php-src/master/ext/soap/tests/classmap.wsdl');

$book = new stdClass();
$book->a = TestBackedEnum::First;
$book->b = '';

$client->dotest($book);

Resulted in this output:

PHP Fatal error:  Uncaught Error: Object of class TestBackedEnum could not be converted to string

But I expected this output instead:

No errors

PHP Version

PHP 8.3.11

Operating System

No response

@cmb69
Copy link
Member

cmb69 commented Sep 2, 2024

Somewhat related to #15650. Quoting from there:

In general, enums are not their backing values, and conversion is explicit. This was a deliberate design decision of enums.

So this is not a bug, but might be a legit feature request.

@cmb69
Copy link
Member

cmb69 commented Sep 6, 2024

cc @Crell and @iluuu1994

@Crell
Copy link
Contributor

Crell commented Sep 6, 2024

I think the main question here would be if we consider SOAPClient to be a serialization context. serialize(), json_encode(), etc. are, and so use the backing value. Would SOAPClient fall into that category? If so, pulling the backing value makes sense. If not, it doesn't.

@cmb69
Copy link
Member

cmb69 commented Sep 6, 2024

I think the main question here would be if we consider SOAPClient to be a serialization context.

I think, we do. @nielsdos can possibly clarify.

However, I'm not sure where to draw the line about what we support. https://pecl.php.net/package/xmlrpc is likely of no concern any longer, although XMLRPC can be regarded as precursor to SOAP. But there are likely more relevant packages, e.g. https://pecl.php.net/package/yaml. Well, not bundled extension probably need to decide for themselves, but what about XMLWriter and FFI (not really serialization, but somewhat related), for instance?

@nielsdos
Copy link
Member

nielsdos commented Sep 6, 2024

What's the definition of "serialization context"? I guess it depends on that.
In any case, this error comes from zval_get_string_func called from to_xml_string when trying to convert the field to a string. So to me, it is casting, not serialization, and therefore, there is nothing to do here.

@MrMeshok
Copy link
Author

MrMeshok commented Sep 8, 2024

Let me show you how I got to this issue.
I have WSDL with this definition of enum

<xsd:simpleType name="AuthMethod">
    <xsd:restriction base="xsd:string">
        <xsd:enumeration value="SessionToken"/>
        <xsd:enumeration value="LoginProductAndPassword"/>
    </xsd:restriction>
</xsd:simpleType>

So I created Enum and DTO which I will pass to SoapClient

enum AuthMethod: string
{
    case SessionToken = 'SessionToken';
    case LoginProductAndPassword = 'LoginProductAndPassword';
}

class AuthRequest
{
    public function __construct(
        public AuthMethod $AuthMethod,
        public ?string $SessionToken = null,
        public ?int $ProductId = null,
        public ?string $Login = null,
        public ?string $Password = null,
    ) {}
}

For me it's seems reasonable, to expect for it to just work.

If instead of SOAP it was some JSON RPC API I would just json_encode my DTO and it will work how I expected it.

{
    "AuthMethod": "SessionToken",
    "SessionToken": "123345"
}

@nielsdos
Copy link
Member

nielsdos commented Sep 8, 2024

Okay I see, it seems reasonable then to use the backing value. I'll take a look soon-ish.

@nielsdos
Copy link
Member

nielsdos commented Sep 8, 2024

I've opened #15803 to fix this. @MrMeshok If possible please test this.

@MrMeshok
Copy link
Author

MrMeshok commented Sep 9, 2024

@nielsdos Works great!
While testing I thought about integer enums, I don't have them in my use case, but I think they should be supported too.
I did a test with definition like this

<xsd:simpleType name="TestIntegerEnum">
  <xsd:restriction base="xsd:integer">
    <xsd:enumeration value="1"/>
    <xsd:enumeration value="2"/>
  </xsd:restriction>
</xsd:simpleType>

Which will result in error: Object of class TestIntegerEnum could not be converted to int

@nielsdos
Copy link
Member

nielsdos commented Sep 9, 2024

Thanks for testing. Supporting this for ints makes sense, I'll give it a shot.

nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 9, 2024
nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 9, 2024
@nielsdos
Copy link
Member

nielsdos commented Sep 9, 2024

@MrMeshok I updated the PR with support for int backed enums being serialized with xsd:integer (and alike) types.

nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 9, 2024
nielsdos added a commit to nielsdos/php-src that referenced this issue Sep 9, 2024
@nielsdos nielsdos added Bug and removed Feature labels Sep 16, 2024
nielsdos added a commit that referenced this issue Sep 16, 2024
* PHP-8.3:
  Fix GH-15711: SoapClient can't convert BackedEnum to scalar value
  Use get_serialization_string_from_zval() in all encoding functions
  Introduce get_serialization_string_from_zval() and use it in to_xml_string()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants