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

Combining hPutBuilder + primMapByteStringBounded unsafe with reuse? #203

Closed
vdukhovni opened this issue Feb 22, 2020 · 5 comments
Closed

Comments

@vdukhovni
Copy link
Contributor

vdukhovni commented Feb 22, 2020

The program below will hang if the iteration count (command-line argument) is sufficiently high, 253 loops or more (the default of 10 shows it working initially):

{-# LANGUAGE BlockArguments #-}
{-# LANGUAGE TupleSections  #-}
module Main (main) where 

import Data.ByteString.Char8 as C8 (pack)
import Data.ByteString.Builder as B
import Data.ByteString.Builder.Prim as P
import Data.ByteString.Lazy as B
import Data.ByteString.Lazy.Char8 as L8
import Data.List as L
import System.IO as S
import System.Environment as S

main = do
    count >>= B.hPutBuilder S.stdout . loop
  where
    count :: IO Int
    count = maybe 10 read . fmap fst . L.uncons <$> S.getArgs

    loop n
        | n > 0 =
            B.string8 "iteration "
            <> B.intDec n
            <> B.char8 ' '
            <> B.word8 0x22
            <> P.primMapByteStringBounded enc
               do C8.pack "\\Escape me\192\"please\""
            <> B.word8 0x22
            <> B.char8 '\n'
            <> loop (n-1)
        | otherwise = mempty

    enc =
        P.condB
          do (\w -> w >= 0x20 && w < 0x7f)
          do P.condB
               do (\w -> w /= 0x22 && w /= 0x5c)
               do toB P.word8
               do fstUnit >$< esc >*< toB P.word8
          do P.condB
               do (> 99)
               do fstUnit >$< esc >*< P.word8Dec
               do P.condB
                    do (> 9)
                    do fstUnit . fstUnit
                           >$< esc >*< zero >*< P.word8Dec
                    do fstUnit . fstUnit . fstUnit
                           >$< esc >*< zero >*< zero >*< P.word8Dec
    toB = P.liftFixedToBounded
    esc = toB $ const 0x5c >$< P.word8
    fstUnit = ((), )
    zero = toB $ const 0x30 >$< P.word8
@vdukhovni
Copy link
Contributor Author

vdukhovni commented Feb 23, 2020

I found the problem, it is an off-by-one error, causing an infinite loop between the builder and BufferFull, when the available space is exactly equal to bounded. Pull request coming up.

diff --git a/Data/ByteString/Builder/Prim.hs b/Data/ByteString/Builder/Prim.hs
index 777b309..4dcc150 100644
--- a/Data/ByteString/Builder/Prim.hs
+++ b/Data/ByteString/Builder/Prim.hs
@@ -652,7 +652,7 @@ primMapByteStringBounded w =
               touchForeignPtr ifp -- input buffer consumed
               k br
 
-          | op0 `plusPtr` bound < ope =
+          | op0 `plusPtr` bound <= ope =
               goPartial (ip0 `plusPtr` min outRemaining inpRemaining)
 
           | otherwise  = return $ bufferFull bound op0 (goBS ip0)

@sjakobi
Copy link
Member

sjakobi commented Feb 23, 2020

Oh, that's a know bug! See #117 (comment). I hadn't known how it would manifest though!

A PR just for the bug fix would be much appreciated!

@vdukhovni
Copy link
Contributor Author

Here's the PR: #204

@vdukhovni
Copy link
Contributor Author

vdukhovni commented Feb 23, 2020

Wow, previously found and patched elsewhere almost three years ago! :-( I hope the fix makes it in this time...

@vdukhovni
Copy link
Contributor Author

Fixed with #204

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

No branches or pull requests

2 participants