Skip to content

Remove 1024 character limit on ODBC connection strings #73149

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.

Sign up for GitHub

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

Merged
roji merged 1 commit into from
Aug 1, 2022
Merged

Remove 1024 character limit on ODBC connection strings #73149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/libraries/System.Data.Odbc/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,6 @@
<data name="ADP_OdbcNoTypesFromProvider" xml:space="preserve">
<value>The ODBC provider did not return results from SQLGETTYPEINFO.</value>
</data>
<data name="OdbcConnection_ConnectionStringTooLong" xml:space="preserve">
<value>Connection string exceeds maximum allowed length of {0}.</value>
</data>
<data name="Odbc_UnknownSQLType" xml:space="preserve">
<value>Unknown SQL type - {0}.</value>
</data>
Expand Down
13 changes: 3 additions & 10 deletions src/libraries/System.Data.Odbc/src/System/Data/Odbc/Odbc32.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,17 @@ internal static Exception UnknownSQLType(ODBC32.SQL_TYPE sqltype)
{
return ADP.Argument(SR.GetString(SR.Odbc_UnknownSQLType, sqltype.ToString()));
}
internal static Exception ConnectionStringTooLong()
{
return ADP.Argument(SR.GetString(SR.OdbcConnection_ConnectionStringTooLong, ODBC32.MAX_CONNECTION_STRING_LENGTH));
}

internal static ArgumentException GetSchemaRestrictionRequired()
{
return ADP.Argument(SR.GetString(SR.ODBC_GetSchemaRestrictionRequired));
}

internal static ArgumentOutOfRangeException NotSupportedEnumerationValue(Type type, int value)
{
return ADP.ArgumentOutOfRange(SR.GetString(SR.ODBC_NotSupportedEnumerationValue, type.Name, value.ToString(System.Globalization.CultureInfo.InvariantCulture)), type.Name);
}

internal static ArgumentOutOfRangeException NotSupportedCommandType(CommandType value)
{
#if DEBUG
Expand Down Expand Up @@ -664,12 +663,6 @@ internal enum SQL_DRIVER
COMPLETE_REQUIRED = 3,
}

// todo:move
// internal const. not odbc specific
//
// Connection string max length
internal const int MAX_CONNECTION_STRING_LENGTH = 1024;

// Column set for SQLPrimaryKeys
internal enum SQL_PRIMARYKEYS : short
{
Expand Down
8 changes: 0 additions & 8 deletions src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcConnectionString.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ internal OdbcConnectionString(string connectionString, bool validate) : base(con
int position = 0;
_expandedConnectionString = ExpandDataDirectories(ref filename, ref position);
}
if (validate || (null == _expandedConnectionString))
{
// do not check string length if it was expanded because the final result may be shorter than the original
if ((null != connectionString) && (ODBC32.MAX_CONNECTION_STRING_LENGTH < connectionString.Length))
{ // MDAC 83536
Copy link
Member

@stephentoub stephentoub Aug 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even know what this was? :)

Copy link
Member Author

@roji roji Aug 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tried finding out - for months (I can send you some email threads if you're really interested). One theory @GrabYourPitchforks had (based on other modifications in the changeset) was that this was a localization thing, i.e. that the P/Invoke call failed for long connection strings with some cryptic/non-localized error message, and that this check was added to surface a better error message. But we didn't manage to find a definitive answer.

throw ODBC.ConnectionStringTooLong();
}
}
}
}
}