[cheenamalhotra]

Since 20.3.0, a Binary parameter with an explicit length: 0 or NaN is rejected by the server (#1812).

Fixes #1812

Cause

#1774 moved length resolution into resolveParameter(), which keeps any explicit length (!= null), including 0. Before, a falsy length fell back to resolveLength(). Binary.resolveLength() ignores parameter.length, so Binary was the one type whose wire bytes changed: TYPE_INFO said length 0 while declaration() (still truthiness-based) said binary(2).

Changes

• Binary parameters with length 0 or NaN are rejected by the server since 20.3.0 #1812 fix (8fe62f40): Binary gets a native resolve() that treats a falsy length as unspecified, matching its declaration() and pre-20.3.0 behavior.
• Related pre-existing bugs in the same code (bb5e34c9, also broken in 20.2.x):
  • An empty buffer sent TYPE_INFO length 0, which the server rejects. It now uses 1, matching its binary(1) declaration.
  • A declared length longer than the value wrote the declared length as the data length, corrupting the stream. writeValue now writes the actual byte count, and the server pads the value. Truncation is unchanged.
• Tests (08d28dd0): unit contract tests plus integration tests for length 0/NaN, padding, truncation, and an empty value.

Notes for review

• writeValue is also used by TVP and bulk load, so a binary(n) cell shorter than n no longer corrupts those requests either. This is why bb5e34c9 is a separate commit; it can be split out if preferred.
• Binary now treats length: 0 as unset, while VarBinary/NVarChar keep an explicit 0. This is deliberate: it matches Binary's own declaration, and NaN currently selects max/PLP for those types, so changing them would be a separate behavior change.
• Char with length: 0 has a similar problem but predates 20.3.0. It is left for a follow-up.

Verification

• Compared against Microsoft.Data.SqlClient on SQL Server 2025 by capturing its RPC bytes and replaying the bytes generated by this branch through a TDS proxy. Tedious's bytes now match SqlClient's for every non-null case: length 0/NaN/2 return 0x4A50, length 4 returns 0x4A500000, length 1 returns 0x4A, and an empty value returns 0x00. Null values still use TYPE_INFO 8000, which the server accepts.
• npm test (609 passing), npm run lint, and npm run build pass.
• The new integration tests could not be run locally (no SQL-auth test server), so CI will be their first run.

cc @saurabh500 @David-Engel @arthurschreiber

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.72%. Comparing base (446c0f9) to head (cb2e53f).

Files with missing lines	Patch %	Lines
src/data-types/binary.ts	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1817      +/-   ##
==========================================
- Coverage   84.76%   83.72%   -1.04%     
==========================================
  Files          96       96              
  Lines        4720     4726       +6     
  Branches      884      886       +2     
==========================================
- Hits         4001     3957      -44     
- Misses        455      514      +59     
+ Partials      264      255       -9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[arthurschreiber]

Thanks @cheenamalhotra, this is a good find. I'm happy with the direction: treating a Binary length of 0/NaN as "not given" restores what every release did up to 20.2.3. It also matches SqlClient, where Size = 0 means "not set". There is one problem that has to be fixed before merging, plus a few smaller points.

For context, I ran the same cases against SQL Server 2025 with tedious 14.7.0 through 20.3.5 and with this branch, covering request parameters, bulk load and TVPs:

Case (Buffer.from('JP') unless noted)	14.7.0 – 20.2.3	20.3.0 – 20.3.5	this PR
param Binary, length 0 / NaN	0x4A50	server: invalid data length	0x4A50
param Binary, length 4	server: stream ended	same	0x4A500000
param Binary, empty value	server: invalid data length	same	0x00
bulk / TVP Binary, length 4	server error	same	0x4A500000
bulk / TVP Binary, length 3, rows A, ABC, ABCDE	server error	same	0x410000, 0x414243, 0x414243

Must fix: writeValue now silently truncates bulk-load and TVP rows

writeValue is also used for every bulk-load and TVP cell. With this branch, a Binary column declared as length 3 stores ABCDE as ABC, with no error. Before, the whole request failed. Truncating a request parameter to its declared length is long-standing behaviour (and SqlClient does it too), but rows shouldn't lose data silently.

A way to keep both behaviours: do the parameter truncation in resolve, and have writeValue reject a value that is too long.

resolve(parameter) {
  let value = this.validate(parameter.value, undefined);
  const length = parameter.length || (value != null ? value.length || 1 : this.maximumLength);
  if (value != null && value.length > length) {
    value = value.subarray(0, length); // a request parameter is truncated to its declared length, as before
  }
  return { value, length };
},

writeValue(buffer, parameter) {
  // ...
  const length = parameter.length != null ? parameter.length : this.maximumLength;
  if (value.length > length) {
    throw new TypeError(`Binary value of ${value.length} bytes exceeds the declared length of ${length}.`);
  }
  buffer.writeUInt16LE(value.length);
  buffer.writeBuffer(value);
}

I tried this on top of your branch against SQL Server 2025:

• Request parameters behave exactly as they do on your branch, including truncation (length 1 still gives 0x4A), and your tests pass.
• Rows that are too long now fail with an InputError: "Column 'c' could not be serialized" for bulk load, "Input parameter 't' could not be validated" for a TVP. The detail is in cause.
• Side effect: a bulk-load Binary column with length: 0 now fails instead of silently storing a single 0x00 byte, which it has done in every version since at least 14.x.

Please add a bulk-load and a TVP integration test for both a shorter and a longer value: the new integration tests only cover RPC parameters, but this change affects the bytes of every row.

Smaller points

• !== undefined should be != null in writeValue. A TVP column with length: null currently becomes Math.min(..., null, ...) === 0, and the value is sent as zero bytes. The rest of the code base uses != null.
• Bulk load without a length. A Binary column with no length is declared binary(8000), so a 2-byte value is now stored padded to 8000 bytes. It used to fail. Char has always padded like this, so I'm fine with it, but please add a test so it's intentional.
• The comment on resolve is inaccurate. It says TYPE_INFO matches the declared binary(n), but a null input value resolves to 8000 while declaration() emits binary(1), as your unit test asserts. Either align the two or adjust the comment.
• Duplicated length logic. resolve now repeats the length rules from declaration() and resolveLength(), and resolveLength() (still used by bulk load) returns 0 for an empty buffer where resolve returns 1. Could resolve call resolveLength, with resolveLength changed to value.length || 1?
• Test helper. selectBinary duplicates most of execSql. It drops the errorMessage logging and the TEDIOUS_DEBUG hook, and doesn't close the connection if connect fails. Could execSql compare Buffers with deepEqual instead?

Out of scope here

Some related problems are older than 20.3.0 and we'll handle them separately: rejecting negative, fractional and oversized lengths with a clear error, a clear error for length: 0/NaN on bulk-load columns (with the change above they no longer store 0x00, but the error doesn't say what's wrong), and the matching problems with Char/NChar and TVP columns.

---

Generated by Claude Code