PHP 8.3 support added

[glo71317]

Q	A
Documentation	yes/no
Bugfix	yes/no
BC Break	yes/no
New Feature	yes/no
RFC	yes/no
QA	yes/no

Description

[glo71317]

Hi @Ocramius

Few tests are failing due to some certification issue
Error - [Microsoft][ODBC Driver 18 for SQL Server]SSL Provider: [error:0A000086:SSL routines::certificate verify failed:self-signed certificate]

It seems some issue with CI/CD Any plan to fix this issue OR anything can we do?

[Ocramius]

Yep, the usual MSSQL mess.

@weierophinney can we perhaps drop testing MSSQL? It's such a massive waste of time and oxygen for other better RDBMS...

[glo17680]

@Ocramius / @weierophinney is there any update on this. Can anyone please advise on the same ?

[Ocramius]

Either we rip out MSSQL testing (and mark it as untested somewhere in the docs), or we wait for Microsoft to release MSSQL support for PHP 8.3, which usually happens few weeks/months after release.

[weierophinney]

My feelings on the matter are:

• Not testing against MS SQL Server means we don't support it
• Not supporting it would be a BC break, requiring a new major
• We've marked this component as security-only, so a new major feels like a break with that

As such, my vote is we wait until MS has released a version of the extension for 8.3. If tests still fail at that time, we can regroup and figure out what to do from there; my guess is everything will "just work" as it has the last several releases.

[glo71317]

@Ocramius @weierophinney Any update on this?

[Ocramius]

No.

[driehle]

This is a bit unfortunate, as this blocks laminas-log, which is quite a central component in many applications.

How about extracting the namespace Laminas\Db\Adapter\Driver\Sqlsrv into a separate package? This would, obviously, require a new major release.

[weierophinney]

@driehle Considering this repo is security-only, we're not going to do a new major. It would likely make more sense to split the lamians-db related functionality in laminas-log into a new package.

I recognize folks are excited about the new PHP release but (a) it's been not even two weeks, and (b) not every organization is tracking PHP updates in order to release as soon as it drops. It's far better for us to wait and ensure that the component works as expected (which means running tests!) than it is to rush a release. Please be patient.

[glo71317]

@Ocramius Any update on this when we can expect laminas-db available with php 8.3 support

[Ocramius]

#293 (comment)

[rchrdkn]

Looks like this is the msphpsql issue that we're waiting on - microsoft/msphpsql#1477

Let's hope they decide to release soon 🤞🏻

[glo71317]

@Ocramius @weierophinney https://github.com/microsoft/msphpsql/releases/tag/v5.12.0 this released Can we prioritize this?

[glo71317]

I have fixed all the test failures and other deprecated dynamic properties issues, Only following error is there

Unable to connect laminasdb_test. Array

(
    [0] => Array
        (
            [0] => 08001
            [SQLSTATE] => 08001
            [1] => -1
            [code] => -1
            [2] => [Microsoft][ODBC Driver 18 for SQL Server]SSL Provider: [error:0A000086:SSL routines::certificate verify failed:self-signed certificate]
            [message] => [Microsoft][ODBC Driver 18 for SQL Server]SSL Provider: [error:0A000086:SSL routines::certificate verify failed:self-signed certificate]
        )
    [1] => Array
        (
            [0] => 08001
            [SQLSTATE] => 08001
            [1] => -1
            [code] => -1
            [2] => [Microsoft][ODBC Driver 18 for SQL Server]Client unable to establish connection. For solutions related to encryption errors, see https://go.microsoft.com/fwlink/?linkid=2226722
            [message] => [Microsoft][ODBC Driver 18 for SQL Server]Client unable to establish connection. For solutions related to encryption errors, see https://go.microsoft.com/fwlink/?linkid=2226722
        )
)

it seems it should be fixed from your end or suggest to us we can do anything for this.

[Ocramius]

SSounds like a new requirement by the MSSQL adapter? Can you try disabling SSL validation for MSSQL in the test suite?

[Xerkus]

This has to be done via connection options https://learn.microsoft.com/en-us/sql/connect/php/connection-options by setting TrustServerCertificate to 1

This is introduced by ODBC Driver 18

[glo71317]

SSounds like a new requirement by the MSSQL adapter? Can you try disabling SSL validation for MSSQL in the test suite?

No idea how to disable ssl validation for MSSQL in the suite. so any idea?

[driehle]

laminas-db/test/integration/Adapter/Platform/SqlServerTest.php

Lines 36 to 40 in 12385a4

	[
	'UID' => getenv('TESTS_LAMINAS_DB_ADAPTER_DRIVER_SQLSRV_USERNAME'),
	'PWD' => getenv('TESTS_LAMINAS_DB_ADAPTER_DRIVER_SQLSRV_PASSWORD'),
	'Database' => $database,
	]

As Xerkus said, try adding 'TrustServerCertificate' => 1 there.

[glo71317]

laminas-db/test/integration/Adapter/Platform/SqlServerTest.php

Lines 36 to 40 in 12385a4

	[
	'UID' => getenv('TESTS_LAMINAS_DB_ADAPTER_DRIVER_SQLSRV_USERNAME'),
	'PWD' => getenv('TESTS_LAMINAS_DB_ADAPTER_DRIVER_SQLSRV_PASSWORD'),
	'Database' => $database,
	]

As Xerkus said, try adding 'TrustServerCertificate' => 1 there.

I have made changes - 2abd29d but still having same issue

[Xerkus]

There is more than one place where sqlsrv_connect() is used. You need to find which of those need the option

[glo71317]

There is more than one place where sqlsrv_connect() is used. You need to find which of those need the option

I have updated all the places and fixed that issues, now lot tests are failing, Can you check these need to fixed still having some issue?

[Xerkus]

There is still one certificate validation error:

 1) LaminasIntegrationTest\Db\Adapter\Platform\SqlServerTest::testQuoteValueWithSqlServer
PDOException: SQLSTATE[08001]: [Microsoft][ODBC Driver 18 for SQL Server]SSL Provider: [error:0A000086:SSL routines::certificate verify failed:self-signed certificate]

/github/workspace/test/integration/Adapter/Platform/SqlServerTest.php:54

[glo71317]

@Xerkus it seems tests are not running due to .laminas-ci/install-sqlsrv-extension-via-pecl.sh as you told to removed

[Xerkus]

@Xerkus it seems tests are not running due to .laminas-ci/install-sqlsrv-extension-via-pecl.sh as you told to removed

Yes, it is referenced here:

laminas-db/.laminas-ci/pre-install.sh

Line 19 in 12385a4

${WORKING_DIRECTORY}/.laminas-ci/install-sqlsrv-extension-via-pecl.sh "${PHP_VERSION}" || exit 1

Please also remove that line.

[Xerkus]

I asked you to revert changes in adapter previously. The correct way is to pass connection options as parameters for Connection instance: new Connection([..., 'options' => ['TrustServerCertificate' => '1']]);

I looked at the tests code and best place seems to be here:

--- a/test/unit/Adapter/Driver/Sqlsrv/AbstractIntegrationTest.php
+++ b/test/unit/Adapter/Driver/Sqlsrv/AbstractIntegrationTest.php
@@ -38,6 +38,8 @@ abstract class AbstractIntegrationTest extends TestCase
             $this->variables[$name] = getenv($value);
         }
 
+        $this->variables['options'] = ['TrustServerCertificate' => '1'];
+
         if (! extension_loaded('sqlsrv')) {
             $this->fail('The phpunit group integration-sqlsrv was enabled, but the extension is not loaded.');
         }

[Xerkus]

Also you seem to have missed re-adding changes for some sqlsrv_connect() calls in tests.

[glo71317]

sqlsrv_connect

I don't think as i can see here i removed src/Adapter/Driver/Sqlsrv/Connection.php as u suggested

[Xerkus]

sqlsrv_connect

I don't think as i can see here i removed src/Adapter/Driver/Sqlsrv/Connection.php as u suggested

Nevermind, looking at a wrong commit.

[glo71317]

All issues resolved from my end now and pr is green now thank you so much for your quick support and review

[Xerkus]

You are missing signoff for your commits. See DCO check for details

[glo71317]

You are missing signoff for your commits. See DCO check for details

Shall i use following command
git rebase HEAD~8 --signoff
git push --force-with-lease origin php8.3_support