search

prisma / tiberius

TDS 7.2+ (Microsoft SQL Server) driver for Rust
Apache License 2.0
327 stars 119 forks source link

Fix buffer overrun on finalize #266

Closed leons727 closed 1 year ago

leons727 commented 1 year ago

This fixes the issue when appending TokenDone to the buffer in finalize() method causes buffer length exceed packet size.

pimeys commented 1 year ago

Very nice! Is there a way to get a test for this? I understand if it's too tricky to write, and I doubt anybody will break this feature in the future...

pimeys commented 1 year ago

Also, you might want to rebase main, so you'll get rid of the clippy/chrono warning-errors.

leons727 commented 1 year ago

There is probably a way to test this by generating bulk inserts with various sizes, for a table with a single BIT column each row takes 3 bytes, so it'd be possible to construct an insert that trips buffer after appending TokenDone. But that would be a bit fragile setup, and I am not sure if it's worth it...

Regarding rebasing main -- not sure what you mean -- my fork is based on main... I did miss running format, updated PR with format ran.

pimeys commented 1 year ago

There is probably a way to test this by generating bulk inserts with various sizes, for a table with a single BIT column each row takes 3 bytes, so it'd be possible to construct an insert that trips buffer after appending TokenDone. But that would be a bit fragile setup, and I am not sure if it's worth it...

Regarding rebasing main -- not sure what you mean -- my fork is based on main... I did miss running format, updated PR with format ran.

I mean there are some CI errors from the latest rust/clippy and chrono deprecations. Fetch our main branch, then on yours do git rebase main so the tests on this PR are green and we can merge.

leons727 commented 1 year ago

Ah, I thought I've done that, should be up to date now.

pimeys commented 1 year ago

The new version is out. Thank you.