Repository navigation
fix: send the PLP terminator when an NVarChar or VarBinary length is missing - #1811
Merged
Merged
Conversation
…missing NVarChar and VarBinary wrote a value's PLP header unless its length was `<= maximumLength`, but its PLP terminator only if the length was `> maximumLength`. A length that is neither, such as `undefined` for a TVP column whose length was left out, or `NaN`, got the `max` TYPE_INFO and PLP header but no terminator, and the server rejected the request with "The stream ended unexpectedly". Decide the header and the terminator from the same test, so these values are sent as terminated `max` values again, as `writeTypeInfo` declares them and as before 20.3.0. Numeric lengths serialize exactly as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
|
Thanks for catching this! ❤️ |
Contributor
|
🎉 This PR is included in version 20.3.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1810.
Since 20.3.0 (#1774),
NVarCharandVarBinarywrite the PLP header unlesslength <= maximumLength, but the PLP terminator only iflength > maximumLength. A length that is neither, such asundefinedfor a TVP column whoselengthwas left out, orNaN, gets amaxTYPE_INFO and PLP header but no terminator, and the server fails the request with "The stream ended unexpectedly".VarChardecides both in one if/else and isn't affected. (node-mssql'slength: 'max'took the same path; node-mssql now converts that on its side.)Changes
maxvalue again: whatwriteTypeInfodeclares, and what 20.2.3 sent.callProcedure(input and output),execSql,execute, TVPs and bulk load, across lengthsundefined,null,0,1, the maximum, maximum + 1,65535,Infinity,NaN,'max'and numeric strings, with null, empty, short, surrogate-pair and over-maximum values: every case matches 20.2.3 byte for byte.This is deliberately minimal so it can ship as a 20.3.x patch. #1780 replaces
writeValuewithcompileWriter, whose single if/else already handles this, so #1780 can supersede it: when rebasing #1780, take its side of the twowriteValuehunks, and the new tests pass unchanged on top of it.Tests
test/unit/data-type.ts:NVarChar(string) andVarBinary(Buffer) values withlength: undefinedandlength: NaNserialize as PLP values ending with the terminator. These four fail on master. ThewriteTypeInfotests also pin that both lengths are declared asmax; those assertions already pass on master.test/integration/tvp-test.ts: returns the same data when string column lengths are left out sends a TVP whoseVarCharandNVarCharcolumns have nolength. On master it fails with anAggregateErrorwrapping "The stream ended unexpectedly". The shared test type has no binary column, soVarBinaryis covered by the unit tests; I also checked a TVPVarBinarycolumn without a length andexecSqlwithlength: NaNby hand against SQL Edge (both fail on master, both work with this change and on 20.2.3).Checks
npm run lint(eslint andtsc)npm run builddatatypes-in-results-test.ts, which needs CLR, which SQL Edge doesn't have🤖 Generated with Claude Code