haskell / text

Haskell library for space- and time-efficient operations over Unicode text.
http://hackage.haskell.org/package/text
BSD 2-Clause "Simplified" License
408 stars 159 forks source link

Fixed off by one for writeBlocksRaw #590

Closed BebeSparkelSparkel closed 6 months ago

BebeSparkelSparkel commented 7 months ago

closes #587

Bodigrim commented 7 months ago

Is it possible to add a regression test?

BebeSparkelSparkel commented 7 months ago

It may be possible, but I am unsure how to. This is a case of the last character in the buffer not being used because the bounds check was copy pasted from the above write* functions that have the possibility of writing two characters CRLF.

Bodigrim commented 7 months ago

Would anything in the current test suite break if we put n - 10 instead of n / n + 1? Do we have any tests in this area at all?

BebeSparkelSparkel commented 7 months ago

All tests pass with n - 10

BebeSparkelSparkel commented 7 months ago

I don't know how to test for buffer overflow unless we can add a test buffer type that checks for that.

Lysxia commented 7 months ago

We might not catch this in regular builds but we can test this by inserting a debug assertion (enabled by -DASSERTS under the -fdeveloper cabal flag) to ensure that writeCharBuf is within bounds.

BebeSparkelSparkel commented 7 months ago

@Lysxia Good idea. I have added the bounds assertion to a wrapped writeCharBuf

BebeSparkelSparkel commented 6 months ago

@Bodigrim Anything else that needs to be done?

Bodigrim commented 6 months ago

Thanks!