briannesbitt / Carbon

A simple PHP API extension for DateTime.
https://carbon.nesbot.com/
MIT License
16.55k stars 1.28k forks source link

InvalidArgumentException : Unknown setter 'date’ #1988

Closed stefro closed 4 years ago

stefro commented 4 years ago

Hello,

I have an issue with Carbon dates that are serialised through GitHub - opis/closure: Serialize closures (anonymous functions)

Laravel version: 6.11.0 Carbon version: 2.28.0 PHP version: 7.3.10 Opis/Closure version: 3.5.1

The problem seems to occur after upgrading to Laravel 6 and therefor upgraded from Carbon 1 to Carbon 2.

I wasn’t sure if this belongs to the Carbon or Opis issues. So I’ve posted this question on both. I will provide links later.

When a closure is serialized:

return serialize(new SerializableClosure($closure, true));

I get this error: InvalidArgumentException : Unknown setter 'date’.

This boils down to /nesbot/carbon/src/Carbon/Traits/Date.php:1166.

I’ve googled this issue for some time now and I believe it has got a lot to do with the depreciation of serializeUsing on Carbon 2. As found in issue Provide an alternative to serializeUsing to Laravel users · Issue #1870 · briannesbitt/Carbon · GitHub. @driesvints

However after reading multiple threats I’m not able to find a solution. I really hope someone can help me steering me to the right direction 🙏.

Thanks!

kylekatarnls commented 4 years ago

Hello,

The magic behind opis/closure is using Reflection to mimic the code of the closure. While it's pretty smart and there is not so much alternative for such purpose, it's something with what we can't provide any guarantee of interoperability. Mostly as it access private properties (https://github.com/opis/closure/blob/master/src/SerializableClosure.php#L484) and we of course assumed our private scope not to be used as a public API.

Then, the code chunk you gave gives no clue as I have no idea what $closure contains, so I can't reproduce your bug.

About the error, it means some code created a Carbon instance and tried to do $date->date = something; which is not allowed. And even if Carbon would allow it, it would not work as date is a private property of the DateTime class, not the Carbon class:

$date = Carbon::parse('2020-01-16 09:52:05');
$date->date = '2000-01-06 00:00:00';

echo $date; // 2020-01-16 09:52:05

The deprecation of serializeUsing has no probable link here if you don't explicitly use this method.

Thanks.

kylekatarnls commented 4 years ago

Details needed:

stefro commented 4 years ago

Hi @kylekatarnls Thank you so much for the fast reply. I'm trying to create a simplified code chunk to reproduce but it's harder than I thought because there are a lot of classes involved. The application is a Botman (https://github.com/botman/botman) application and it involved a closure of an BotMan\BotMan\Messages\Incoming\Answer instance.

You've already pushed me in the right direction! I know this is not a code chunk you can reproduce, but I hope this provides you more details. I think it all comes down to this line of code in the Opis/Closure package:

$property->setValue($data, $value)

When I do a dump on these variables, I get this:


// $property
ReflectionProperty {#6198
  +name: "date"
  +class: "Carbon\Carbon"
  modifiers: "public"
}

// $data
Carbon\Carbon^ {#6189
  ⚠: Symfony\Component\VarDumper\Exception\ThrowingCasterException^ {#6978
    #message: "Unexpected ErrorException thrown from a caster: DateTime::getTimezone(): The DateTime object has not been correctly initialized by its constructor"
    trace: {
      ./vendor/nesbot/carbon/src/Carbon/Traits/Date.php:627 { …}
      ./vendor/symfony/var-dumper/Cloner/AbstractCloner.php:252 { …}
      Symfony\Component\VarDumper\Cloner\AbstractCloner->Symfony\Component\VarDumper\Cloner\{closure}() {}
      ./vendor/nesbot/carbon/src/Carbon/Traits/Date.php:627 { …}
      ./vendor/symfony/var-dumper/Caster/DateCaster.php:30 { …}
    }
  }
  #localMonthsOverflow: null
  #localYearsOverflow: null
  #localStrictModeEnabled: null
  #localHumanDiffOptions: null
  #localToStringFormat: null
  #localSerializer: null
  #localMacros: null
  #localGenericMacros: null
  #localFormatFunction: null
  #localTranslator: null
  #dumpLocale: null
}

// $value
"2020-01-21 00:00:00.000000"

I'm not that experienced with what should be the expected outcome, but I don't think the Carbon Instance in $data is correct.

I really hope this info is enough for you to push me in the right direction!

kylekatarnls commented 4 years ago

I'm sorry, as for as it concerns Carbon, modifying ->date publically will not be not supported. Carbon instances support serialization: unserialize(serialize($date)) output a value equal to $date. And it seems opis/closure try to recreate the Carbon instance using its serialization but without using the PHP unserialize function (magic method __wakeup should be used). This is not the purpose of the function and it's not something we should support.

You could override Carbon::__set extending the Carbon class then swap Laravel to use it: Date::swap(YourCarbonClass::class) to support the modification of ->date for instance it can use ->modify() to allow ->date = "2020-01-06 12:00:00" to properly set the date but it would just be a work-around to handle the problem Carbon/Laravel-side.

The fact is Carbon has a safe guard: the exception you get InvalidArgumentException : Unknown setter 'date' making you believe there's something wrong in Carbon, but you probably have many other classes that just don't have those kind of safety guard and this can just produce erratic and unpredictable behaviors because it modify properties based on the serialization string which will be wrong in many cases.

stefro commented 4 years ago

Hi @kylekatarnls, Thank you so much for this reply. I can completely understand now what is happening, why and where the responsibility lies for a solution. I love the Carbon package and use it every day. You're awesome for providing this support even though it is not your responsibility in this case.

Thanks!

kylekatarnls commented 4 years ago

Hello @stefro please try to require:

"nesbot/carbon": "dev-feature/issue-1988-reflection-class-support"

It should allow the serialization opis/closure needs.

stefro commented 4 years ago

Hi @kylekatarnls Thank you so much for the PR. I've required the dev-feature/issue-1988-reflection-class-support.

It now throws a different kind of error:

ErrorException : DateTime::modify(): The DateTime object has not been correctly initialized by its constructor

How can I help?

stefro commented 4 years ago

I've just pulled the dev-master in my project and it all works brilliantly! 🎉 Thank you all for the effort on this issue!

AcriCAA commented 4 years ago

@stefro I am experiencing this same issue in Laravel/Botman 2.0 on Laravel 5.6. Would you mind explaining how you addressed it? I see you pulled in the "dev-master", does that mean you incorporated Carbon into your botman project instead of through composer?

stefro commented 4 years ago

@AcriCAA no, I think that was only for some testing. The issue was solved with great help from @kylekatarnls I think the problem you’re experiencing is that you’re on Laravel 5.6 and therefor use Carbon 1. The issue was solved in Carbon 2.

kylekatarnls commented 4 years ago

"dev-master" can be selected as the version via composer. But it's not needed. It was a long time ago. What was on the master branch then has been released since.

It sounds like you have different versions and so likely a different issue.

AcriCAA commented 4 years ago

Thank you @stefro @kylekatarnls. I will have to figure this out because I have to stay with Laravel 5.6 because I believe botman is not compatible with later versions of Laravel. Perhaps I will try the Laravel swap @kylekatarnls mentioned earlier in this thread.

haikajymada commented 3 years ago

Hello @stefro please try to require:

"nesbot/carbon": "dev-feature/issue-1988-reflection-class-support"

It should allow the serialization opis/closure needs.

Botman + Carbon + Opis/Closure seems to be a bad mix.

I always have this following error when using Carbon. So I chose to manage with PHP Standard Date instead of this wonderful library ! => Invalid serialization data for DateTime object. HandlesConversations.php Line 136

Botman-Ask-Listen-Invalid-Serialization-PHP-7-4

kylekatarnls commented 3 years ago

Those 3 can work properly together if you install Carbon 2. If you have a constraint on Carbon 1, you can use an alias to enforce using the 2.x (See the doc). Version 1 is no longer maintained for a long time and will not receive such features updates.

If you are using Botman Studio, it uses Laravel 5.7 which is no longer supported neither and has security advisories: https://packagist.org/packages/laravel/framework/advisories?version=3782796

You will have more serious issues than the Carbon compatibility if you stay with Laravel < 7.23.2. As working on the compatibility issue from the Carbon side won't be enough anyway, we won't spend more time in it.

If you still have an issue with the last version of Carbon (and the rest of the stack pretty up to date), please open a proper issue following the pattern to detail precisely "when using Carbon", dump the stack as text and provide a minimum code chunk that can allow me to reproduce the issue.

Thanks.

haikajymada commented 3 years ago

It's better for me to put the question in Botman project. Thank you.