On 3/31/22 21:05, Florian Manschwetus wrote:
> From: manschwetusCS <30724946+manschwetusCS@users.noreply.github.com>
>
> In Windows ODBC it seems to be legal to use "host/port", which is not legal with Linux ODBC.
> This patch aims to find and replace such in the ConnectionString

Hi, Florian.

Thanks for the patch. I have a few comments.

> @@ -1779,10 +1779,21 @@ SQLRETURN WINAPI SQLDriverConnect(SQLHDBC hdbc, SQLHWND hwnd, SQLCHAR *Connectio
>                                     SQLCHAR *conn_str_out, SQLSMALLINT conn_str_out_max,
>                                     SQLSMALLINT *ptr_conn_str_out, SQLUSMALLINT driver_completion)
>   {
> +    static const char *serverStr="SERVER=";
>       struct SQLDriverConnect_params params = { hdbc, hwnd, ConnectionString, Length, conn_str_out,
>                                                 conn_str_out_max, ptr_conn_str_out, driver_completion };
>       SQLRETURN ret;
>
> +    SQLCHAR *serverPos;
> +    for (serverPos = strstr(ConnectionString,serverStr);(*serverPos != ';') && (*serverPos != '\0'); ++serverPos)
> +    {
> +        if (*serverPos == '/')
> +        {
> +           *serverPos = ':';
> +           break;
> +        }
> +    }
> +

My understanding is that connection string attributes have to be passed
as is to underlying driver/unixodbc proxy. If server port syntax differs
between implementations, does that mean different drivers have different
syntax? If that's the case I don't think you can swap one for another
like that. For example I see some examples for SQL Server that are using
"host,port" syntax instead.

So this needs to be investigated across drivers on Windows and unixodbc
to see what needs to happen exactly. Depending on syntax requirements,
it's possible we need to change it only for some drivers, or maybe
drivers need to be fixed.

If it turns out we do need to modify connection string, cleaner way is
to tokenize it fully first, then tweak parts that need tweaking, and
assemble it back. In your fix you're changing original connection
string, which sounds incorrect. Another thing is that attribute names
are case insensitive afaik, so comparison you're using is too strict.