php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79521 `__set_state` structure not been checked
Submitted: 2020-04-26 02:59 UTC Modified: 2020-04-28 15:05 UTC
From: carusogabriel@php.net Assigned: carusogabriel (profile)
Status: Closed Package: Class/Object related
PHP Version: Next Major Version OS: *
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: carusogabriel@php.net
New email:
PHP Version: OS:

 

 [2020-04-26 02:59 UTC] carusogabriel@php.net
Description:
------------
The documentation (https://php.net/manual/en/language.oop5.magic.php#object.set-state) of the magic method `__set_state` says:

- it MUST take an array argument
- it MUST return an object

Ok, two problems:

- it CAN take an argument, is not mandatory, e.g.: https://3v4l.org/gTlmV
- it CAN return, is not mandatory: https://3v4l.org/SG8Hs

Should we fix its documentation? Something like

-static __set_state ( array $properties ) : object
+static __set_state ( [array $properties] ) : mixed


Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-04-26 03:06 UTC] requinix@php.net
-Package: Documentation problem +Package: Class/Object related
 [2020-04-26 03:06 UTC] requinix@php.net
The fact that you can get away without arguments is for the usual reasons: the signature of __set_state is not enforced by the engine, and it is not an error to pass more arguments than are defined in a function signature.

The return value is simply being ignored. Also not an error.

Related: the magic-methods-signature RFC
https://wiki.php.net/rfc/magic-methods-signature
It doesn't include __set_state, but this is likely just an oversight.

See also bug #69718
 [2020-04-26 03:13 UTC] carusogabriel@php.net
> It doesn't include __set_state, but this is likely just an oversight.

Yeah, I'm working on that right now, that's why I come up with this bug report.

> The return value is simply being ignored. Also not an error.

It isn't: it does change the result based on the type:

- https://3v4l.org/jt3jL (string)
- https://3v4l.org/tIHi3 (object)

This is supported by the engine: https://github.com/php/php-src/blob/d21d23afef54f152d9ca661909c6d042d6bfcf48/ext/standard/var.c#L472-L593

> and it is not an error to pass more arguments than are defined in a function signature.

It is not an error, this is why the documentation needs to be updated, right?
 [2020-04-26 03:26 UTC] requinix@php.net
Sorry, by ignored I meant that the type doesn't matter.
It really should return an object, though. It's what the method is for.

And I totally didn't notice the RFC was yours. I'll shut up now :)
 [2020-04-26 03:39 UTC] carusogabriel@php.net
> Sorry, by ignored I meant that the type doesn't matter.

:+1:


> It really should return an object, though. It's what the method is for.


In this case, we'd need an RFC removing that functionality, right?

> And I totally didn't notice the RFC was yours. I'll shut up now :)

Comments are always welcome, don't shut up ;)
 [2020-04-26 13:08 UTC] carusogabriel@php.net
-Summary: Fix `__set_state` documentation +Summary: `__set_state` structure not been checked -Type: Documentation Problem +Type: Bug
 [2020-04-26 13:08 UTC] carusogabriel@php.net
I've opened https://github.com/php/php-src/pull/5462 to fix this. Indeed, not a documentation problem, we need to fix it.
 [2020-04-27 15:08 UTC] carusogabriel@php.net
-Assigned To: +Assigned To: carusogabriel
 [2020-04-27 15:08 UTC] carusogabriel@php.net
The following pull request has been associated:

Patch Name: [#79521] Check `__set_state` structure
On GitHub:  https://github.com/php/php-src/pull/5462
Patch:      https://github.com/php/php-src/pull/5462.patch
 [2020-04-28 15:05 UTC] carusogabriel@php.net
-Status: Assigned +Status: Closed -Operating System: +Operating System: * -PHP Version: Irrelevant +PHP Version: Next Major Version
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Dec 03 17:01:29 2024 UTC