Allow Carbon v3

[lloricode]

No description provided.

[freekmurze]

Seems like the tests are failing for Carbon v3. Could you have a look?

[lloricode]

checking

[lloricode]

Hi @freekmurze, done on fixing test, also added matrix for canbon v2 and v3 to make sure all works,

thank you

[freekmurze]

Perfect, thanks!

[PhilETaylor]

This is a breaking change.

With spatie/ssl-certificate v2.6.4 (which installs Carbon 2.72.3) $certificate->expirationDate()->diffInDays() returns a POSITIVE integer E.g 75 ✅

With spatie/ssl-certificate v2.6.5 (which installs Carbon 3.2.1) $certificate->expirationDate()->diffInDays() returns a NEGATIVE double E.g -75.307162384132 ❌

Which, within a real world app, is interpreted as the SSL being expired and has caused me to email 1000s of users - doh...

With spatie/ssl-certificate v2.6.5 (with FORCED Carbon 2.72.3) $certificate->expirationDate()->diffInDays() returns a POSITIVE integer E.g 75 ✅

Reproducer:

<?php
use Spatie\SslCertificate\SslCertificate;
require 'vendor/autoload.php';
$certificate = SslCertificate::createForHostName(parse_url('https://mySites.guru/', \PHP_URL_HOST));
var_dump($certificate->expirationDate()->diffInDays());

My workaround is to not use $certificate->expirationDate()->diffInDays() but to move to $certificate->daysUntilExpirationDate() which returns the POSITIVE integer regardless of Carbon version

[PhilETaylor]

related comments briannesbitt/Carbon#2971 (comment)

[kylekatarnls]

In the case of a certificate expiration, I think it's good not to hide the fact that it's actually already expired. I think the Carbon v2 behavior was actually dangerous because using daysUntilExpirationDate() and getting 2 you might think you still have 2 days before expiration, while it actually can mean it may be expired for 2 days already.

So I think having a negative number when expired is a safe move.

Depending on the feature need some other possible behaviors for when value < 0 could be:

• Throw an exception such as throw new \LogicException('Already expired')
• Simply return 0
• Return a non-integer value (null or false)

[kylekatarnls]

(I misread the code, so I edited my message above, sorry for the noise)

[turbo124]

Just to note on the implementation here casting using (int) will still return a negative integer? Is this expected? I would consider intval(abs($x));

[PhilETaylor]

I dont mind progress, but this was and is a backward compatibility change that broke implementations and was released in a minor release, not a major release.

[kylekatarnls]

I didn't check when was tagged the previous minor but:

        return (int) Carbon::now()->diffInDays($endDate);

using Carbon 3 is equivalent to what was apparently the previous code:

        $interval = Carbon::now()->diff($endDate);

        return (int) $interval->format('%r%a');

I.E. negative number when expired, positive before expiration.

[kylekatarnls]

(int) Carbon::now()->diffInDays($endDate) is absolute by default in Carbon v2 only, so if you have constraint locking Carbon to v2, yes, it might be wrong:

(int) Carbon::now()->diffInDays($endDate, false) would ensure value is negative when expired for both v2 and v3.

[PhilETaylor]

For me personally I just accepted the change and modified my code for future proofing as stated above 3 months ago. Others were also discussing this change in other places at the same time briannesbitt/Carbon#2971 (comment) -

[kylekatarnls]

BTW, lifespanInDays would suffer the same inconsistency. But having a certificate not yet valid is a bit more of an edge-case.