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

adds pre tds 7.1 compatibility #83

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

eshaker
Copy link

@eshaker eshaker commented Jan 23, 2023

This PR adds rough compatibility to older sql-servers (TDS <= 7.1)

Tested against our internal legacy mssql-server

@eshaker
Copy link
Author

eshaker commented Jan 23, 2023

@microsoft-github-policy-service agree

@@ -273,6 +273,10 @@ func readPreloginOption(buffer []byte, offset int) (*preloginOption, error) {
rec_type := buffer[offset]
if rec_type == preloginTERMINATOR {
return &preloginOption{token: rec_type}, nil
} else if rec_type == preloginTHREADID {
Copy link
Collaborator

Choose a reason for hiding this comment

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

rec_type == preloginTHREADID {

does this check need to be protected by the TDS version check too?

Copy link
Author

Choose a reason for hiding this comment

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

As per https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/60f56408-0188-4cd5-8b90-25c6f2423868 there seems to be no constraint on the TDS-Version.

As I understand the documentation, the parameter is used for the client to communicate its thread-id for debugging-purposes. If sent by the server, it should be empty in any case.

@shueybubbles
Copy link
Collaborator

thx for submitting a PR!
It's blowing up the tests

-- FAIL: TestReturnStatusWithQuery (0.07s)
tds_test.go:307: 2023-01-23 08:09:32.3043182 +0000 GMT m=+0.661839001 [tds.go:927]: Dialing with protocol tcp
tds_test.go:307: 2023-01-23 08:09:32.306315 +0000 GMT m=+0.663835801 [tds.go:939]: Returning connection from protocol tcp
tds_test.go:307: 2023-01-23 08:09:32.3733294 +0000 GMT m=+0.730849901 [token.go:749]: got token tokenEnvChange
tds_test.go:307: 2023-01-23 08:09:32.3743187 +0000 GMT m=+0.731839201 [token.go:749]: got token tokenInfo
tds_test.go:307: 2023-01-23 08:09:32.3743187 +0000 GMT m=+0.731839201 [token.go:894]: got INFO 5701 Changed database context to 'test'.
tds_test.go:307: 2023-01-23 08:09:32.3743187 +0000 GMT m=+0.731839201 [token.go:897]: Changed database context to 'test'.
tds_test.go:307: 2023-01-23 08:09:32.3743187 +0000 GMT m=+0.731839201 [token.go:749]: got token token(0)
tds_test.go:307: 2023-01-23 08:09:32.3743187 +0000 GMT m=+0.731839201 [token.go:719]: ERROR: Intercepted panic Invalid TDS stream: unknown token type returned: token(0)

@@ -119,7 +119,7 @@ type xmlInfo struct {
XmlSchemaCollection string
}

func readTypeInfo(r *tdsBuffer) (res typeInfo) {
func readTypeInfo(sess *tdsSession, r *tdsBuffer) (res typeInfo) {
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure about the code-style. I did not want to copy the whole readTypeInfo func for v7.1 compatibility as the version-info is needed in readVarLen

@eshaker
Copy link
Author

eshaker commented Jan 24, 2023

There still seems to be a problem with the transactions.
I currently have a very limited testing-setup; I will take a look at those as soon as possible

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

Successfully merging this pull request may close these issues.

2 participants