Fix issue where a float could be passed to gmdate, throwing an exception in PHP 8.1

[27pchrisl]

Fixes #2327

Adds a coercion to int, which would have been the internal behaviour of the function pre-8.1. The test suite expects the code should fail when passed a string like "this text is not a date", therefore the is_numeric test and exception.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[27pchrisl]

@SamRemis there appears to be a problem if the random ints generated at

aws-sdk-php/tests/Crypto/Polyfill/AesGcmTest.php

Line 2621 in 0b632d5

$ptLen = \random_int(0, 1024);

are too small (<4), running the test suite as-is again will likely work.

[SamRemis]

Thanks :) I'll go ahead and restart it

[SamRemis]

Have you been seeing floats passed to GM date? If so, what floats and from what services? This seems like something we should investigate at the source before always casting to int. At first glance, I like what this is doing and am likely to approve, just want to do some due diligence and look up any amazon standards we have for parsing dates.

[27pchrisl]

@driesvints I think this came up in DynamoDB somewhere?

[driesvints]

@27pchrisl @SamRemis so what I'm noticing now in our tests is that the first failure happens on line 96 of our test suite:

Failure: https://github.com/laravel/framework/pull/39034/checks?check_run_id=3905548052#step:8:153
Line: https://github.com/laravel/framework/blob/php8.1-fixes/tests/Integration/Cache/DynamoDbStoreTest.php#L96

So it's failing at the point where it's trying to create the table in DynamoDB to be used in the tests. When I look at the docs in AWS I feel like everything looks okay about this piece of code. This has also always worked in previous PHP versions.

When I look at the three other failing tests after that I see them failing on line 127:

Failure: https://github.com/laravel/framework/pull/39034/checks?check_run_id=3905548052#step:8:187
Line: https://github.com/laravel/framework/blob/php8.1-fixes/tests/Integration/Cache/DynamoDbStoreTest.php#L127

Which is at the point where we're checking if the table already exists. Also here we seem to do exactly as the docs say. Except this fails only for the three tests after the first one. In the first test this command succeeds. The only difference is that with the first test the table doesn't exists yet and thus it proceeds to the part that creates. This means the table exists for the three tests after the first failing one so the command execution to create the table was in fact successful, only something went amis after executing the command.

So this leads me to believe there's something amis within the AWS PHP SDK after a DynamoDB call succeeds and an object gets hydrated (or something). And that results in a timestamp retrieval somewhere which is a float instead of an integer.

[27pchrisl]

Hi @SamRemis,

Here's an example of creating a table with DynamoDB (running locally), returning a float CreationDateTime.

curl --location --request POST 'http://localhost:8000/' \
--header 'x-amz-target: DynamoDB_20120810.CreateTable' \
--header 'content-type: application/x-amz-json-1.0' \
--header 'x-amz-date: 20211016T141853Z' \
--header 'authorization: AWS4-HMAC-SHA256 Credential=foo/20211016/eu-west-1/dynamodb/aws4_request, SignedHeaders=content-type;host;x-amz-date;x-amz-target, Signature=17f434e56d37317d5a06c1c5add12cc6faa9950cc87fb5d49d3ad97824453d54' \
--data-raw '{
    "AttributeDefinitions": [
        {
            "AttributeName": "A",
            "AttributeType": "S"
        }
    ],
    "TableName": "laravel",
    "KeySchema": [
        {
            "AttributeName": "A",
            "KeyType": "HASH"
        }
    ],
    "ProvisionedThroughput": {
        "ReadCapacityUnits": 10,
        "WriteCapacityUnits": 5
    }
}'

{
    "TableDescription": {
        "AttributeDefinitions": [
            {
                "AttributeName": "A",
                "AttributeType": "S"
            }
        ],
        "TableName": "laravel",
        "KeySchema": [
            {
                "AttributeName": "A",
                "KeyType": "HASH"
            }
        ],
        "TableStatus": "ACTIVE",
        "CreationDateTime": 1634394371.040,
        "ProvisionedThroughput": {
            "LastIncreaseDateTime": 0.000,
            "LastDecreaseDateTime": 0.000,
            "NumberOfDecreasesToday": 0,
            "ReadCapacityUnits": 10,
            "WriteCapacityUnits": 5
        },
        "TableSizeBytes": 0,
        "ItemCount": 0,
        "TableArn": "arn:aws:dynamodb:ddblocal:000000000000:table/laravel"
    }
}

[GrahamCampbell]

Ah, so this code seems incorrect. The DateTime API should be used instead, in order to recover the sub-second part of the data.

[GrahamCampbell]

return self::createFromFormat('U.u', sprintf('%0.6f', $unixTimestamp));

[GrahamCampbell]

I have prepared a replacement at #2337 with test coverage.

[driesvints]

@SamRemis is there anything we can do to help to get this PR or @GrahamCampbell's PR merged and tagged? Last thing we need to offer full PHP 8.1 support in Laravel 🤞

[SamRemis]

This is on my list of PRs to merge, so it will be soon. We tend to only merge one per release day in case it has to roll back

[driesvints]

For some reason the build on our end has started to pass again. So this isn't urgent anymore for us 👍

[SamRemis]

replaced by in #2337