Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

laravel PDO invalid attribute exception with PHP 8.2.9/8.1.22 #47937

Closed
ssswang opened this issue Aug 2, 2023 · 61 comments
Closed

laravel PDO invalid attribute exception with PHP 8.2.9/8.1.22 #47937

ssswang opened this issue Aug 2, 2023 · 61 comments
Labels

Comments

@ssswang
Copy link

ssswang commented Aug 2, 2023

Laravel Version

10.17

PHP Version

8.2.9

Database Driver & Version

php_pdo_sqlsrv_82_ts_x64 5.11, Microsoft sql server 2019

Description

PHP just released a new minor version 8.1.22 / 8.2.9.
There are some changes related to PDO.

My applications work fine with php 8.1/8.2 previous minor versions.

With 8.1.22 (laravel 9.* ) or 8.2.9 (laravel 10.* ), have a connection to a Microsoft sql server 2019, with official Microsoft database php pdo driver,

I got
Illuminate \ Database \ QueryException SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object.

for any queries in laravel.

I have also tested with raw php, pdo seems working fine.

It seems related to this line
https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Connectors/SqlServerConnector.php#L19

Steps To Reproduce

  • laravel application with php 8.1.22/8.2.9 and Microsoft pdo driver, either windows or linux,
  • connecting to a Microsoft sql server
  • do a query in laravel.
@khairahmanplus
Copy link

I have same problem with this. Already format a laptop and make a fresh installation with PHP and Laravel. I got same error message Illuminate \ Database \ QueryException SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object.

@khairahmanplus
Copy link

khairahmanplus commented Aug 3, 2023

Okay i already check. This is from PHP side. When you use PHP 8.2.9 and 8.1.22, you will get this kind of problem. Downgrade PHP version to 8.2.8, the problem is solved.

@crynobone
Copy link
Member

This could be related to php/php-src#11622

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Aug 3, 2023

Is someone able to open an issue on the php github repo with a small reproducer (no laravel dependency), please? We don't stand any chance of getting a resolution here, on the PHP side, without this.

@SakiTakamachi
Copy link
Contributor

SakiTakamachi commented Aug 3, 2023

Hi. This is my fix.

PDO SQLSRV is not a module managed by php-src.

As you can see here, SQLSRV throws exceptions for unsupported options, unlike other standard PDO modules that just return false.

https://github.com/microsoft/msphpsql/blob/f6f76d4ac116411b6c53ce482c16da73a0652292/source/pdo_sqlsrv/pdo_dbh.cpp#L1367

If you don't mind, I'll put out a fix PR for SQLSRV, what do you think?

@SakiTakamachi
Copy link
Contributor

SakiTakamachi commented Aug 3, 2023

Opps, when I looked at it again, I found that the exception was caught.

I'll check it out properly again.

@SakiTakamachi
Copy link
Contributor

I took a closer look at the code and it looks like it's throwing a different exception after all.
I will prepare a fix PR.

@ssswang
Copy link
Author

ssswang commented Aug 3, 2023

$host = 'localhost';
echo "pdo connection test ".$host.":1433<br />\n";

$conn = new \PDO("sqlsrv:Server=".$host.",1433;Database=table", "user", "password");
$conn->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);

$conn->setAttribute(\PDO::ATTR_STRINGIFY_FETCHES, false); //throw exception

$stmt = $conn->prepare("SELECT * FROM table");
$stmt->execute();
$tables = $stmt->fetch(\PDO::FETCH_ASSOC);
echo gettype($tables["id"]);
var_dump($tables);
echo "<br />\n";

if we don't set PDO::ATTR_STRINGIFY_FETCHES to false, all columns return will be string, default is true.
it looks like \PDO::ATTR_STRINGIFY_FETCHES was not supported by msphpsql

https://github.com/microsoft/msphpsql/blob/master/source/pdo_sqlsrv/pdo_dbh.cpp
ATTR_STRINGIFY_FETCHESwas only dealt with in pdo_sqlsrv_dbh_get_attr() but not in pdo_sqlsrv_dbh_set_attr()

There is a sqlsrv attribute called SQLSRV_ATTR_FETCHES_NUMERIC_TYPE

So actually this laravel line was not working before @SakiTakamachi's fix on php pdo class:
https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Connectors/SqlServerConnector.php#L19

To get the intended result, it looks like we should set SQLSRV_ATTR_FETCHES_NUMERIC_TYPE to true, and remove L19 from SqlServerConnector class.

https://github.com/microsoft/msphpsql/blob/master/CHANGELOG.md#linux-408---2016-12-19

@o15a3d4l11s2
Copy link

Can anyone suggest a workaround solution for this? I have also systems with Laravel 8.83.27, the same issue appears.
For me reverting to the previous minor version is not an easy solution.

@GrahamCampbell
Copy link
Member

It is the only solution available, unless you want to re-build the extension yourself with the patch from microsoft/msphpsql#1468 included.

@o15a3d4l11s2
Copy link

It is the only solution available, unless you want to re-build the extension yourself with the patch from microsoft/msphpsql#1468 included.

Once the extension is fixed, it would be possible to update it also under 8.1.22, right?

@GrahamCampbell
Copy link
Member

I would not expect Microsoft to move fast there on reviewing/merging/releasing your PR. ;)

@HoudaAbdellaoui1
Copy link

Having the same issue with my app usign Laravel and pdo_sqlsrv, but the problem only occurs in production, on my local machine app is working fine . Tried to downgrade PHP version to 8.2.8 like @khairahmanplus said, but then my api troubleshoots and doesn't work at all .

@khairahmanplus Any advice to downgrade my php version in dockerfile without side effects ?

@SakiTakamachi
Copy link
Contributor

I have prepared a repository that will be somewhat better than a steady build.
https://github.com/SakiTakamachi/pdo-sqlsrv-extend

However, this should only be used when "there is really no other option but to use this".

We should revert back to the prev php version and wait for an official driver to be released.

@luisedr98
Copy link

I had the same problem. I tried to connect laravel 9 with SQL Server 2012. I had to downgrade php version. You can visit php website for getting it

Okay i already check. This is from PHP side. When you use PHP 8.2.9 and 8.1.22, you will get this kind of problem. Downgrade PHP version to 8.2.8, the problem is solved.

@HoudaAbdellaoui1
Copy link

Same for me, worked for me when downgrading PHP version to 8.2.8

@Rikj000
Copy link

Rikj000 commented Aug 8, 2023

@khairahmanplus Any advice to downgrade my php version in dockerfile without side effects ?

Simply specify the minor version of PHP in the tag:

# FROM php:8.1-fpm-alpine
FROM php:8.1.21-fpm-alpine

@driesvints
Copy link
Member

Unfortunately there's nothing we can do about this on our side. Please either report this to php-src or encourage microsoft/msphpsql#1468 to be merged and released.

I'll close this now but keep this pinned on our issue tracker.

@SakiTakamachi
Copy link
Contributor

There is a very interesting error avoidance method in Drupal.

I haven't tried it yet, so I don't know the effect, but I'll share it for now.

https://www.drupal.org/project/sqlsrv/issues/3379887

https://git.drupalcode.org/project/sqlsrv/-/commit/d298031b858bb597f5478d7e9acca320405db3ee

@HoudaAbdellaoui1
Copy link

@khairahmanplus Any advice to downgrade my php version in dockerfile without side effects ?

Simply specify the minor version of PHP in the tag:

# FROM php:8.1-fpm-alpine

FROM php:8.1.21-fpm-alpine

I believe you should use php:8.1.2-fpm-alpine and then use the same exact version in your composer.json file . Then add RUN composer update to your dockerfile.

These steps worked for me .

@SakiTakamachi
Copy link
Contributor

I tried it.
When the exception was caught and processing continued, it resulted in the expected behavior!

// create table test_table(id int, str varchar(50));
// INSERT INTO test_table(id, str) VALUES (1, 'abcd');

<?php

// my env
$pdo = new PDO('sqlsrv:server=mssql-server;database=test;', 'test_user', 'p@ssw0rd');
try {
    $pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);
} catch (PDOException $e) {
    // do nothing
}
$pdo->setAttribute(PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE, true);

$results = $pdo->query('SELECT * FROM test_table');
foreach ($results as $result) {
    var_dump($result);
}

result

array(4) {
  ["id"]=>
  int(1)
  [0]=>
  int(1)
  ["str"]=>
  string(4) "abcd"
  [1]=>
  string(4) "abcd"
}

if $pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, true);

array(4) {
  ["id"]=>
  string(1) "1"
  [0]=>
  string(1) "1"
  ["str"]=>
  string(4) "abcd"
  [1]=>
  string(4) "abcd"
}

@martinbkaiser
Copy link

We just had to update all our Docker files, 8.1.2 fixes the issue

@rickycheers
Copy link
Contributor

rickycheers commented Aug 14, 2023

An alternative to downgrading to PHP 8.1.21 or 8.2.8, if is not possible or not an option, the following could be of help.

Set a custom connection class to be used by Laravel by registering it in the service container:

Extend SqlServerConnector:

class CustomSqlServerConnector extends \Illuminate\Database\Connectors\SqlServerConnector
{
    protected $options = [
        PDO::ATTR_CASE => PDO::CASE_NATURAL,
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_ORACLE_NULLS => PDO::NULL_NATURAL,
        PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE => true,
    ];
}

Then, from AppServiceProvider, register it in the service container.

$this->app->bind('db.connector.sqlsrv', CustomSqlServerConnector::class);

@SakiTakamachi
Copy link
Contributor

SakiTakamachi commented Aug 14, 2023

@rickycheers

Does this work?

I think it can't be replaced with DI because it's directly new in ConnectionFactory.

Oops, excuse me, you can DI this.

I haven't had time to test it yet, but I think it will probably look something like this.

If you wrap it with try catch, it will work as expected, so I think something like this would work too.


    protected function createPdoConnection($dsn, $username, $password, $options)
    {
        $stringifyFetches = null;
        if (array_key_exists(PDO::ATTR_STRINGIFY_FETCHES, $options)) {
            $stringifyFetches = $options[PDO::ATTR_STRINGIFY_FETCHES];
            unset($options[PDO::ATTR_STRINGIFY_FETCHES]);
        }

        $pdo = new PDO($dsn, $username, $password, $options);

        if (! is_null($stringifyFetches)) {
            try {
                $pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, $stringifyFetches);
            } catch (PDOException $e) {
                // do nothing
            }
        }

        return $pdo;
    }

https://github.com/SakiTakamachi/laravel-sqlsrv-err-avoid

@SakiTakamachi
Copy link
Contributor

SakiTakamachi commented Aug 23, 2023

The lead of PDO SQLSRV seems to have a negative opinion of hotfixes.
I'm still convinced that the problem is on the PDO SQLSRV side, but it's also true that it's getting harder to fix early.

So, I proposed a temporary workaround on the php-src side.
If you are following information on this issue, please check this:
php/php-src#12038

@skylord123
Copy link

The lead of PDO SQLSRV seems to have a negative opinion of hotfixes.

Well no one likes doing hotfixes but when you manage something used by so many people that is broken..

Really hoping a final fix isn't weeks away.

@SakiTakamachi
Copy link
Contributor

SakiTakamachi commented Aug 30, 2023

Since there was no response, I opened an issue.
microsoft/msphpsql#1474

@estin92
Copy link

estin92 commented Sep 4, 2023

Another solution here is to create a custom connector and override the binding. This then also gives protection in the future to fix any object issues before a global patch becomes available, albeit you'd need to manage any updates/new requirements too.

<?php

namespace App\Database\Connectors;

use Illuminate\Database\Connectors\SqlServerConnector;
use PDO;

class CustomSqlServerConnector extends SqlServerConnector
{
    /**
     * Override the default options array to prevent the SQLSRV error:
     * SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object.
     *
     * @var array
     */
    protected $options = [
        PDO::ATTR_CASE => PDO::CASE_NATURAL,
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_ORACLE_NULLS => PDO::NULL_NATURAL,
    ];
}

Then in App\Providers\AppServiceProvider add in the following:

/**
 * Register any application services.
 *
 * @return void
 */
public function register()
{
    $this->app->bind('db.connector.sqlsrv', \App\Database\Connectors\CustomSqlServerConnector::class);
}

@rianhe
Copy link

rianhe commented Sep 5, 2023

My solution...

I have reviewed the PDO documentation, and you can specify the "error mode" with the PDO::ATTR_ERRMODE attribute: https://www.php.net/manual/en/pdo.error-handling.php

Laravel set the PDO::ERRMODE_EXCEPTION value by default, the quick and easy solution is set value to PDO::ERRMODE_SILENT in config/database.php with "options" key`:

'sqlsrv' => [
    'driver' => 'sqlsrv',
    ...
    'options' => [
        PDO::ATTR_ERRMODE => PDO::ERRMODE_SILENT,
    ]
],

@axsweet
Copy link

axsweet commented Sep 5, 2023

@rianhe is a beast. 100% that is the TEMP work around. BUTTTT works!!!!

@gmutinel
Copy link

gmutinel commented Sep 6, 2023

My solution...

I have reviewed the PDO documentation, and you can specify the "error mode" with the PDO::ATTR_ERRMODE attribute: https://www.php.net/manual/en/pdo.error-handling.php

Laravel set the PDO::ERRMODE_EXCEPTION value by default, the quick and easy solution is set value to PDO::ERRMODE_SILENT in config/database.php with "options" key`:

'sqlsrv' => [
    'driver' => 'sqlsrv',
    ...
    'options' => [
        PDO::ATTR_ERRMODE => PDO::ERRMODE_SILENT,
    ]
],

This solution will silence EVERY PDO crash, including wrong queries which now will return empty array. I think it's too dangerous...

@rianhe
Copy link

rianhe commented Sep 6, 2023

This solution will silence EVERY PDO crash, including wrong queries which now will return empty array. I think it's too dangerous...

Are you sure? I have tried to execute wrong queries and exceptions are thrown...

@axsweet
Copy link

axsweet commented Sep 6, 2023 via email

@SakiTakamachi
Copy link
Contributor

SakiTakamachi commented Sep 6, 2023

The behavior of PDO ERR MODE is very esoteric.

Sometimes it's silent as expected, other times it ignores the mode and raises an exception.

If you want to know more about what happens in which cases, you need to understand the behavior written in C.

However, choosing a silent error mode doesn't seem like a wise choice, at least in a production environment.

@gmutinel
Copy link

gmutinel commented Sep 6, 2023

image

These are the results with and without the option

@gmutinel
Copy link

gmutinel commented Sep 6, 2023

True but if you cant roll back this saves you behind..... I rolled back production with in a hour after finding these issues back when .8 was out but now .11 i cant on dev and this was a decent temp fix. Is there a way to suppress just that stringafi issue?

On Wed, Sep 6, 2023 at 9:18 AM rianhe @.> wrote: This solution will silence EVERY PDO crash, including wrong queries which now will return empty array. I think it's too dangerous... Are you sure? I have tried to execute wrong queries and exceptions are thrown... — Reply to this email directly, view it on GitHub <#47937 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALF5SJCKVXRQUYQG442PWMDXZBZ2BANCNFSM6AAAAAA3BXP6AQ . You are receiving this because you commented.Message ID: @.>

This will give back a result instead of an exception, the value will be [] and the logic after this query will treat it as a REAL VALUE, I think it's crazy to use this without a custom error handling for each query...

@rianhe
Copy link

rianhe commented Sep 6, 2023

image

These are the results with and without the option

You are right, in that case it returns false positives.

My solution is really quick and temporary... to be able to unlock the environments where the bug is occurring right now.

The final solution should be done at https://github.com/microsoft/msphpsql/

@wlius-support3
Copy link

wlius-support3 commented Sep 6, 2023

An alternative to downgrading to PHP 8.1.21 or 8.2.8, if is not possible or not an option, the following could be of help.

Set a custom connection class to be used by Laravel by registering it in the service container:

Extend SqlServerConnector:

class CustomSqlServerConnector extends \Illuminate\Database\Connectors\SqlServerConnector
{
    protected $options = [
        PDO::ATTR_CASE => PDO::CASE_NATURAL,
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_ORACLE_NULLS => PDO::NULL_NATURAL,
        PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE => true,
    ];
}

Then, from AppServiceProvider, register it in the service container.

$this->app->bind('db.connector.sqlsrv', CustomSqlServerConnector::class);

This is the solution that worked for me.

In App\Providers, I created a file called CustomSqlServerConnector.php.

I placed the following code in that file:

<?php

namespace App\Providers;

use PDO;

class CustomSqlServerConnector extends \Illuminate\Database\Connectors\SqlServerConnector
{
    protected $options = [
        PDO::ATTR_CASE => PDO::CASE_NATURAL,
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_ORACLE_NULLS => PDO::NULL_NATURAL,
        PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE => true,
    ];
}

Then I edited app/Providers/AppServiceProvider.php and now the file looks like this:

<?php

namespace App\Providers;

use Illuminate\Support\ServiceProvider;

class AppServiceProvider extends ServiceProvider
{
    /**
     * Register any application services.
     *
     * @return void
     */
    public function register()
    {
        $this->app->bind('db.connector.sqlsrv', CustomSqlServerConnector::class);
    }

    /**
     * Bootstrap any application services.
     *
     * @return void
     */
    public function boot()
    {
        //
    }
}

That seems like the cleanest workaround until we see Microsoft update the driver.

@rianhe
Copy link

rianhe commented Sep 6, 2023

This i think a more complete and dynamic solution, not only for PDO::ATTR_STRINGIFY_FETCHES... and not overwrite default connector options.

Then, make the PDO connection as follows:

New file App\Fixes\SqlServerConnector.php

<?php

namespace App\Fixes;

use Illuminate\Database\Connectors\SqlServerConnector as BaseConnector;
use Illuminate\Database\Connectors\ConnectorInterface;

use PDO;
use PDOException;

class SqlServerConnector extends BaseConnector implements ConnectorInterface
{

    /**
     * Create a new PDO connection instance.
     *
     * @param  string  $dsn
     * @param  string  $username
     * @param  string  $password
     * @param  array  $options
     * @return \PDO
     */
    protected function createPdoConnection($dsn, $username, $password, $options)
    {

        $pdo = new PDO($dsn, $username, $password);

        foreach ($options as $attribute => $value) {
            try {
                $pdo->setAttribute($attribute, $value);
            } catch (PDOException $e) {
                //
            }
        }

        return $pdo;

    }

}

Now, in App\Providers\AppServiceProvider.php:

<?php

namespace App\Providers;

use Illuminate\Support\ServiceProvider;

class AppServiceProvider extends ServiceProvider
{
    /**
     * Register any application services.
     */
    public function register(): void
    {

        $this->app->bind('db.connector.sqlsrv', \App\Fixes\SqlServerConnector::class);

    }

    /**
     * Bootstrap any application services.
     */
    public function boot(): void
    {
        //
    }
}

@wlius-support3
Copy link

wlius-support3 commented Sep 6, 2023

This i think a more complete and dynamic solution, not only for PDO::ATTR_STRINGIFY_FETCHES... and not overwrite default connector options

I would be careful as the long term ramifications of silently killing any PDOException related to setting attributes may cause future issues. Suppose we encounter an issue where the driver from Microsoft drops support for an attribute that we expected to have a certain effect on the results of a query.

This situation is an example of that. By killing the exception from trying to set PDO::ATTR_STRINGIFY_FETCHES, values returned from a query may have a different type than your application is expecting.

Edit: As noted by Saki below, she did test that the try/catch solution for this scenario is fine. However, I do think it is worth being careful with the try/catch solution, as it will catch any PDOException related to setting attributes, not just for this specific scenario. Ideally, this would never be an issue, as Saki pointed out in a separate thread, the setAttribute method should only return false on failure (not raise an exception), however we see here that Microsoft's messed this up once, there's no telling if they might do it again in the future in a different way that ends up impacting the results of a query.

@SakiTakamachi
Copy link
Contributor

This situation is an example of that. By killing the exception from trying to set PDO::ATTR_STRINGIFY_FETCHES, values returned from a query may have a different type than your application is expecting.

In this case, you don't have to worry about that.

This is because the exception is raised at the end after the attribute is accepted by the driver.

@wlius-support3
Copy link

On a positive note, it looks like there may be a patch this week from Microsoft.

@SakiTakamachi
Copy link
Contributor

It has been released!

https://github.com/microsoft/msphpsql/releases/tag/v5.11.1

@axsweet
Copy link

axsweet commented Sep 8, 2023 via email

@fernando-moreno
Copy link

Confirm that update 5.11.1 for PHP Driver for SQL Server for Laravel works fine.

@esetnik
Copy link

esetnik commented Sep 18, 2023

Beware v5.11.1 breaks the way some connection attributes are handled. See microsoft/msphpsql#1478

@woodspire
Copy link

@taylorotwell Please unpin this from the "issue" tab since it's been closed for over 3 months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.