createFromTime() fails when seconds parameter > 99

[cjcox17]

Hello,

Anytime that the seconds parameter is greater than 99 the function fails. However, 99 and below works properly.

I encountered an issue with the following code:

Carbon::createFromTime(0,0,100)->toTimeString()

Carbon version: nesbot/carbon v2.17.1

PHP version: v7.1.23

I expected to get:

00:01:40

But I actually get:

Error: The separation symbol could not be found
Trailing data

Try here

https://try-carbon.herokuapp.com/?embed&theme=xcode&border=silver&options-color=rgba(120,120,120,0.5)&input=Carbon%3A%3AcreateFromTime(0%2C0%2C100)-%3EtoTimeString()%3B

Thanks!

[cjcox17]

Test:

>>> Carbon\Carbon::createFromTime(0,0,1)->toTimeString();
=> "00:00:01"
>>> Carbon\Carbon::createFromTime(0,0,99)->toTimeString();
=> "00:01:39"
>>> Carbon\Carbon::createFromTime(0,0,100)->toTimeString();
InvalidArgumentException with message 'The separation symbol could not be found
Trailing data'

[cjcox17]

The above function may not have been designed to provide that kind of functionality and if that's the case I think a more detailed exception should be thrown.

In retrospect,

Carbon\Carbon::createFromTimeStamp(100)->toTimeString();

Works just as well when needing only the time string.

[kylekatarnls]

Hello,

This is in fact due to the DateTime behavior:

echo DateTime::createFromFormat('H:i:s', '00:00:01')->format('H:i:s'); // "00:00:01"
echo DateTime::createFromFormat('H:i:s', '00:00:99')->format('H:i:s'); // "00:01:39"
echo DateTime::createFromFormat('H:i:s', '00:00:100')->format('H:i:s'); // fails as "s" format accepts only 2 digits.

That means, if we change something here, it would be for a more strict validation (error if $seconds > 59) but we won't accept 100, as createFromTime is supposed to take a "time" meaning a valid set of hour/minute/second.

I was about to suggest what you commented meanwhile. Using createFromTimeStamp or some Carbon::today()->addSeconds(100) is really better because it explicitly seconds unit from a start point, not a time.

[kylekatarnls]

I kept the limits to [0; 99] but added specific exceptions messages for the next release.