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

odbc_pconnect fails with DSN and username/password with special characters #12251

Closed
kevinreniers opened this issue Sep 20, 2023 · 7 comments
Closed

Comments

@kevinreniers
Copy link

Description

Hello, we're trying to upgrade from PHP 8.1 to PHP 8.2.10. We integrate with an MSSQL server using FreeTDS/unixODBC and ext/odbc. After upgrading, the same code that worked on PHP 8.1 now fails on PHP 8.2.

I assume that Quote when appending username and password to the ODBC connection string is the cause of my issue.

The following code:

<?php

namespace App\Connections;

use RuntimeException;

class OdbcConnection implements DatabaseConnectionInterface
{
    private $connection;

    public function __construct(private readonly string $dsn, private readonly string $username, private readonly string $password)
    {
    }

    /**
     * @throws RuntimeException
     */
    public function connect(): void
    {
        $this->connection = odbc_pconnect($this->dsn, $this->username, $this->password);
    }
}

$conn = new OdbcConnection('Driver=FreeTDS;Server=myserver.local;Port=1433;Database=MyDatabase', 'user', 'foo(bar(');
$conn->connect();

Resulted in this output:

In OdbcConnection.php line 20:

  [ErrorException]
  Warning: odbc_pconnect(): SQL error: [FreeTDS][SQL Server]Unable to connect to data source, SQL state S1000 in SQLConnect

But I expected this output instead:

No error, works fine in PHP 8.1.

I can workaround the issue in my local development setup by creating an entry in /etc/odbc.ini and then only passing the section name to the $dsn parrameter of odbc_pconnect like so:

# /etc/odbc.ini
[sv1]
Driver=FreeTDS
Server=myserver.local
Port=1433
Database=MyDatabase
<?php

odbc_pconnect('sv1', 'user', 'foo(bar(');

Ideally, it would help me the most if I can make it work as-is without any workaround. It's not practical (but not impossible) to apply the /etc/odbc.ini workaround in my infrastructure. It would delay our upgrade significantly though.

Thank you in advance!

PHP Version

PHP 8.2.10

Operating System

RHEL 8.8, Debian Bullseye

@kevinreniers kevinreniers changed the title odbc_pconnect fails with special characters odbc_pconnect fails with DSN and username/password with special characters Sep 20, 2023
@SakiTakamachi
Copy link
Member

Hi.

I was able to set up the environment with docker and reproduce it.

This problem occurs not only in 8.2 but also in 8.3 and master.

It will take a little more time to determine the full cause, but it's very likely a side effect of the changes you mentioned.

I'll write again if there is any progress.

@SakiTakamachi
Copy link
Member

Code like the following will not generate any warnings and will connect:

// my env
odbc_pconnect('Driver=FreeTDS;Server=mssql-server;Port=1433;Database=test;uid=test_user;pwd=p@ssw0rd', 'hoge', 'huga');

I can't understand why it's designed like this.
The second and third arguments are ignored only if $dsn contains uid and pwd. They are not an option though.

And even though the password I gave in the example doesn't require quotes, they are added and it appears that the authentication fails.

@SakiTakamachi
Copy link
Member

Setting aside the fundamental spec issues for the moment, I made a pull request for a really small change for PHP-8.2 to fix the connection issue.

@SakiTakamachi
Copy link
Member

It turns out that this phenomenon is most likely caused by FreeTDS, not php.

@SakiTakamachi
Copy link
Member

FreeTDS fixed it.

FreeTDS/freetds#504

@Girgias
Copy link
Member

Girgias commented Sep 27, 2023

Going to close this as it seems to be a third party issue. Please open a new issue if neeeded.

Thanks for looking into this @SakiTakamachi !

@Girgias Girgias closed this as completed Sep 27, 2023
@kevinreniers
Copy link
Author

Thanks for the investigation @SakiTakamachi. 😄

I'll try to get around to testing it for our environment later this week or the next.

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

No branches or pull requests

3 participants