[Git][ghc/ghc][master] 5 commits: base: Add test for #13660
Marge Bot (@marge-bot)
gitlab at gitlab.haskell.org
Thu May 18 19:19:20 UTC 2023
Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC
Commits:
87626ef0 by Ben Gamari at 2023-05-18T15:18:53-04:00
base: Add test for #13660
- - - - -
9eef53b1 by Ben Gamari at 2023-05-18T15:18:53-04:00
base: Move implementation of GHC.Foreign to GHC.Internal
- - - - -
174ea2fa by Ben Gamari at 2023-05-18T15:18:53-04:00
base: Introduce {new,with}CStringLen0
These are useful helpers for implementing the internal-NUL code unit
check needed to fix #13660.
- - - - -
a46ced16 by Ben Gamari at 2023-05-18T15:18:53-04:00
base: Clean up documentation
- - - - -
b98d99cc by Ben Gamari at 2023-05-18T15:18:53-04:00
base: Ensure that FilePaths don't contain NULs
POSIX filepaths may not contain the NUL octet but previously we did not
reject such paths. This could be exploited by untrusted input to cause
discrepancies between various `FilePath` queries and the opened
filename. For instance, `readFile "hello.so\x00.txt"` would open the
file `"hello.so"` yet `takeFileExtension` would return `".txt"`.
The same argument applies to Windows FilePaths
Fixes #13660.
- - - - -
7 changed files:
- libraries/base/GHC/Foreign.hs
- + libraries/base/GHC/Foreign/Internal.hs
- libraries/base/System/Posix/Internals.hs
- libraries/base/base.cabal
- + libraries/base/tests/T13660.hs
- + libraries/base/tests/T13660.stdout
- libraries/base/tests/all.T
Changes:
=====================================
libraries/base/GHC/Foreign.hs
=====================================
@@ -21,312 +21,22 @@ module GHC.Foreign (
-- * C strings with a configurable encoding
CString, CStringLen,
- -- conversion of C strings into Haskell strings
- --
+ -- * Conversion of C strings into Haskell strings
peekCString,
peekCStringLen,
- -- conversion of Haskell strings into C strings
- --
+ -- * Conversion of Haskell strings into C strings
newCString,
newCStringLen,
+ newCStringLen0,
- -- conversion of Haskell strings into C strings using temporary storage
- --
+ -- * Conversion of Haskell strings into C strings using temporary storage
withCString,
withCStringLen,
+ withCStringLen0,
withCStringsLen,
charIsRepresentable,
) where
-import Foreign.Marshal.Array
-import Foreign.C.Types
-import Foreign.Ptr
-import Foreign.Storable
-
-import Data.Word
-
--- Imports for the locale-encoding version of marshallers
-
-import Data.Tuple (fst)
-
-import GHC.Show ( show )
-
-import Foreign.Marshal.Alloc
-import Foreign.ForeignPtr
-
-import GHC.Debug
-import GHC.List
-import GHC.Num
-import GHC.Base
-
-import GHC.IO
-import GHC.IO.Exception
-import GHC.IO.Buffer
-import GHC.IO.Encoding.Types
-
-
-c_DEBUG_DUMP :: Bool
-c_DEBUG_DUMP = False
-
-putDebugMsg :: String -> IO ()
-putDebugMsg | c_DEBUG_DUMP = debugLn
- | otherwise = const (return ())
-
-
--- | A C string is a reference to an array of C characters terminated by NUL.
-type CString = Ptr CChar
-
--- | A string with explicit length information in bytes instead of a
--- terminating NUL (allowing NUL characters in the middle of the string).
-type CStringLen = (Ptr CChar, Int)
-
--- exported functions
--- ------------------
-
--- | Marshal a NUL terminated C string into a Haskell string.
---
-peekCString :: TextEncoding -> CString -> IO String
-peekCString enc cp = do
- sz <- lengthArray0 nUL cp
- peekEncodedCString enc (cp, sz * cCharSize)
-
--- | Marshal a C string with explicit length into a Haskell string.
---
-peekCStringLen :: TextEncoding -> CStringLen -> IO String
-peekCStringLen = peekEncodedCString
-
--- | Marshal a Haskell string into a NUL terminated C string.
---
--- * the Haskell string may /not/ contain any NUL characters
---
--- * new storage is allocated for the C string and must be
--- explicitly freed using 'Foreign.Marshal.Alloc.free' or
--- 'Foreign.Marshal.Alloc.finalizerFree'.
---
-newCString :: TextEncoding -> String -> IO CString
-newCString enc = liftM fst . newEncodedCString enc True
-
--- | Marshal a Haskell string into a C string (ie, character array) with
--- explicit length information.
---
--- * new storage is allocated for the C string and must be
--- explicitly freed using 'Foreign.Marshal.Alloc.free' or
--- 'Foreign.Marshal.Alloc.finalizerFree'.
---
-newCStringLen :: TextEncoding -> String -> IO CStringLen
-newCStringLen enc = newEncodedCString enc False
-
--- | Marshal a Haskell string into a NUL terminated C string using temporary
--- storage.
---
--- * the Haskell string may /not/ contain any NUL characters
---
--- * the memory is freed when the subcomputation terminates (either
--- normally or via an exception), so the pointer to the temporary
--- storage must /not/ be used after this.
---
-withCString :: TextEncoding -> String -> (CString -> IO a) -> IO a
-withCString enc s act = withEncodedCString enc True s $ \(cp, _sz) -> act cp
-
--- | Marshal a Haskell string into a C string (ie, character array)
--- in temporary storage, with explicit length information.
---
--- * the memory is freed when the subcomputation terminates (either
--- normally or via an exception), so the pointer to the temporary
--- storage must /not/ be used after this.
---
-withCStringLen :: TextEncoding -> String -> (CStringLen -> IO a) -> IO a
-withCStringLen enc = withEncodedCString enc False
-
--- | Marshal a list of Haskell strings into an array of NUL terminated C strings
--- using temporary storage.
---
--- * the Haskell strings may /not/ contain any NUL characters
---
--- * the memory is freed when the subcomputation terminates (either
--- normally or via an exception), so the pointer to the temporary
--- storage must /not/ be used after this.
---
-withCStringsLen :: TextEncoding
- -> [String]
- -> (Int -> Ptr CString -> IO a)
- -> IO a
-withCStringsLen enc strs f = go [] strs
- where
- go cs (s:ss) = withCString enc s $ \c -> go (c:cs) ss
- go cs [] = withArrayLen (reverse cs) f
-
--- | Determines whether a character can be accurately encoded in a
--- 'Foreign.C.String.CString'.
---
--- Pretty much anyone who uses this function is in a state of sin because
--- whether or not a character is encodable will, in general, depend on the
--- context in which it occurs.
-charIsRepresentable :: TextEncoding -> Char -> IO Bool
--- We force enc explicitly because `catch` is lazy in its
--- first argument. We would probably like to force c as well,
--- but unfortunately worker/wrapper produces very bad code for
--- that.
---
--- TODO If this function is performance-critical, it would probably
--- pay to use a single-character specialization of withCString. That
--- would allow worker/wrapper to actually eliminate Char boxes, and
--- would also get rid of the completely unnecessary cons allocation.
-charIsRepresentable !enc c =
- withCString enc [c]
- (\cstr -> do str <- peekCString enc cstr
- case str of
- [ch] | ch == c -> pure True
- _ -> pure False)
- `catch`
- \(_ :: IOException) -> pure False
-
--- auxiliary definitions
--- ----------------------
-
--- C's end of string character
-nUL :: CChar
-nUL = 0
-
--- Size of a CChar in bytes
-cCharSize :: Int
-cCharSize = sizeOf (undefined :: CChar)
-
-
-{-# INLINE peekEncodedCString #-}
-peekEncodedCString :: TextEncoding -- ^ Encoding of CString
- -> CStringLen
- -> IO String -- ^ String in Haskell terms
-peekEncodedCString (TextEncoding { mkTextDecoder = mk_decoder }) (p, sz_bytes)
- = bracket mk_decoder close $ \decoder -> do
- let chunk_size = sz_bytes `max` 1 -- Decode buffer chunk size in characters: one iteration only for ASCII
- !from0 <- fmap (\fp -> bufferAdd sz_bytes (emptyBuffer fp sz_bytes ReadBuffer)) $ newForeignPtr_ (castPtr p)
- !to <- newCharBuffer chunk_size WriteBuffer
-
- let go !iteration !from = do
- (why, from', !to') <- encode decoder from to
- if isEmptyBuffer from'
- then
- -- No input remaining: @why@ will be InputUnderflow, but we don't care
- withBuffer to' $ peekArray (bufferElems to')
- else do
- -- Input remaining: what went wrong?
- putDebugMsg ("peekEncodedCString: " ++ show iteration ++ " " ++ show why)
- (from'', to'') <- case why of InvalidSequence -> recover decoder from' to' -- These conditions are equally bad because
- InputUnderflow -> recover decoder from' to' -- they indicate malformed/truncated input
- OutputUnderflow -> return (from', to') -- We will have more space next time round
- putDebugMsg ("peekEncodedCString: from " ++ summaryBuffer from ++ " " ++ summaryBuffer from' ++ " " ++ summaryBuffer from'')
- putDebugMsg ("peekEncodedCString: to " ++ summaryBuffer to ++ " " ++ summaryBuffer to' ++ " " ++ summaryBuffer to'')
- to_chars <- withBuffer to'' $ peekArray (bufferElems to'')
- fmap (to_chars++) $ go (iteration + 1) from''
-
- go (0 :: Int) from0
-
-{-# INLINE withEncodedCString #-}
-withEncodedCString :: TextEncoding -- ^ Encoding of CString to create
- -> Bool -- ^ Null-terminate?
- -> String -- ^ String to encode
- -> (CStringLen -> IO a) -- ^ Worker that can safely use the allocated memory
- -> IO a
-withEncodedCString (TextEncoding { mkTextEncoder = mk_encoder }) null_terminate s act
- = bracket mk_encoder close $ \encoder -> withArrayLen s $ \sz p -> do
- from <- fmap (\fp -> bufferAdd sz (emptyBuffer fp sz ReadBuffer)) $ newForeignPtr_ p
-
- let go !iteration to_sz_bytes = do
- putDebugMsg ("withEncodedCString: " ++ show iteration)
- allocaBytes to_sz_bytes $ \to_p -> do
- -- See Note [Check *before* fill in withEncodedCString] about why
- -- this is subtle.
- mb_res <- tryFillBuffer encoder null_terminate from to_p to_sz_bytes
- case mb_res of
- Nothing -> go (iteration + 1) (to_sz_bytes * 2)
- Just to_buf -> withCStringBuffer to_buf null_terminate act
-
- -- If the input string is ASCII, this value will ensure we only allocate once
- go (0 :: Int) (cCharSize * (sz + 1))
-
-withCStringBuffer :: Buffer Word8 -> Bool -> (CStringLen -> IO r) -> IO r
-withCStringBuffer to_buf null_terminate act = do
- let bytes = bufferElems to_buf
- withBuffer to_buf $ \to_ptr -> do
- when null_terminate $ pokeElemOff to_ptr (bufR to_buf) 0
- act (castPtr to_ptr, bytes) -- NB: the length information is specified as being in *bytes*
-
-{-# INLINE newEncodedCString #-}
-newEncodedCString :: TextEncoding -- ^ Encoding of CString to create
- -> Bool -- ^ Null-terminate?
- -> String -- ^ String to encode
- -> IO CStringLen
-newEncodedCString (TextEncoding { mkTextEncoder = mk_encoder }) null_terminate s
- = bracket mk_encoder close $ \encoder -> withArrayLen s $ \sz p -> do
- from <- fmap (\fp -> bufferAdd sz (emptyBuffer fp sz ReadBuffer)) $ newForeignPtr_ p
-
- let go !iteration to_p to_sz_bytes = do
- putDebugMsg ("newEncodedCString: " ++ show iteration)
- mb_res <- tryFillBuffer encoder null_terminate from to_p to_sz_bytes
- case mb_res of
- Nothing -> do
- let to_sz_bytes' = to_sz_bytes * 2
- to_p' <- reallocBytes to_p to_sz_bytes'
- go (iteration + 1) to_p' to_sz_bytes'
- Just to_buf -> withCStringBuffer to_buf null_terminate return
-
- -- If the input string is ASCII, this value will ensure we only allocate once
- let to_sz_bytes = cCharSize * (sz + 1)
- to_p <- mallocBytes to_sz_bytes
- go (0 :: Int) to_p to_sz_bytes
-
-
-tryFillBuffer :: TextEncoder dstate -> Bool -> Buffer Char -> Ptr Word8 -> Int
- -> IO (Maybe (Buffer Word8))
-tryFillBuffer encoder null_terminate from0 to_p !to_sz_bytes = do
- !to_fp <- newForeignPtr_ to_p
- go (0 :: Int) from0 (emptyBuffer to_fp to_sz_bytes WriteBuffer)
- where
- go !iteration !from !to = do
- (why, from', to') <- encode encoder from to
- putDebugMsg ("tryFillBufferAndCall: " ++ show iteration ++ " " ++ show why ++ " " ++ summaryBuffer from ++ " " ++ summaryBuffer from')
- if isEmptyBuffer from'
- then if null_terminate && bufferAvailable to' == 0
- then return Nothing -- We had enough for the string but not the terminator: ask the caller for more buffer
- else return (Just to')
- else case why of -- We didn't consume all of the input
- InputUnderflow -> recover encoder from' to' >>= \(a,b) -> go (iteration + 1) a b -- These conditions are equally bad
- InvalidSequence -> recover encoder from' to' >>= \(a,b) -> go (iteration + 1) a b -- since the input was truncated/invalid
- OutputUnderflow -> return Nothing -- Oops, out of buffer during decoding: ask the caller for more
-{-
-Note [Check *before* fill in withEncodedCString]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-It's very important that the size check and readjustment peformed by tryFillBuffer
-happens before the continuation is called. The size check is the part which can
-fail, the call to the continuation never fails and so the caller should respond
-first to the size check failing and *then* call the continuation. Making this evident
-to the compiler avoids historic space leaks.
-
-In a previous iteration of this code we had a pattern that, somewhat simplified,
-looked like this:
-
-go :: State -> (State -> IO a) -> IO a
-go state action =
- case tryFillBufferAndCall state action of
- Left state' -> go state' action
- Right result -> result
-
-`tryFillBufferAndCall` performed some checks, and then we either called action,
-or we modified the state and tried again.
-This went wrong because `action` can be a function closure containing a reference to
-a lazy data structure. If we call action directly, without retaining any references
-to action, that is fine. The data structure is consumed as it is produced and we operate
-in constant space.
-
-However the failure branch `go state' action` *does* capture a reference to action.
-This went wrong because the reference to action in the failure branch only becomes
-unreachable *after* action returns. This means we keep alive the function closure
-for `action` until `action` returns. Which in turn keeps alive the *whole* lazy list
-via `action` until the action has fully run.
-This went wrong in #20107, where the continuation kept an entire lazy bytestring alive
-rather than allowing it to be incrementally consumed and collected.
--}
-
+import GHC.Foreign.Internal
=====================================
libraries/base/GHC/Foreign/Internal.hs
=====================================
@@ -0,0 +1,357 @@
+{-# LANGUAGE Trustworthy #-}
+{-# LANGUAGE NoImplicitPrelude #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+{-# LANGUAGE BangPatterns #-}
+
+-----------------------------------------------------------------------------
+-- |
+-- Module : GHC.Foreign.Internal
+-- Copyright : (c) The University of Glasgow, 2008-2011
+-- License : see libraries/base/LICENSE
+--
+-- Maintainer : libraries at haskell.org
+-- Stability : internal
+-- Portability : non-portable
+--
+-- Foreign marshalling support for CStrings with configurable encodings
+--
+-----------------------------------------------------------------------------
+
+module GHC.Foreign.Internal (
+ -- * C strings with a configurable encoding
+ CString, CStringLen,
+
+ -- * Conversion of C strings into Haskell strings
+ peekCString,
+ peekCStringLen,
+
+ -- * Conversion of Haskell strings into C strings
+ newCString,
+ newCStringLen,
+ newCStringLen0,
+
+ -- * Conversion of Haskell strings into C strings using temporary storage
+ withCString,
+ withCStringLen,
+ withCStringLen0,
+ withCStringsLen,
+
+ charIsRepresentable,
+ ) where
+
+import Foreign.Marshal.Array
+import Foreign.C.Types
+import Foreign.Ptr
+import Foreign.Storable
+
+import Data.Word
+
+-- Imports for the locale-encoding version of marshallers
+
+import Data.Tuple (fst)
+
+import GHC.Show ( show )
+
+import Foreign.Marshal.Alloc
+import Foreign.ForeignPtr
+
+import GHC.Debug
+import GHC.List
+import GHC.Num
+import GHC.Base
+
+import GHC.IO
+import GHC.IO.Exception
+import GHC.IO.Buffer
+import GHC.IO.Encoding.Types
+
+
+c_DEBUG_DUMP :: Bool
+c_DEBUG_DUMP = False
+
+putDebugMsg :: String -> IO ()
+putDebugMsg | c_DEBUG_DUMP = debugLn
+ | otherwise = const (return ())
+
+
+-- | A C string is a reference to an array of C characters terminated by NUL.
+type CString = Ptr CChar
+
+-- | A string with explicit length information in bytes instead of a
+-- terminating NUL (allowing NUL characters in the middle of the string).
+type CStringLen = (Ptr CChar, Int)
+
+-- exported functions
+-- ------------------
+
+-- | Marshal a NUL terminated C string into a Haskell string.
+--
+peekCString :: TextEncoding -> CString -> IO String
+peekCString enc cp = do
+ sz <- lengthArray0 nUL cp
+ peekEncodedCString enc (cp, sz * cCharSize)
+
+-- | Marshal a C string with explicit length into a Haskell string.
+--
+peekCStringLen :: TextEncoding -> CStringLen -> IO String
+peekCStringLen = peekEncodedCString
+
+-- | Marshal a Haskell string into a NUL terminated C string.
+--
+-- * the Haskell string may /not/ contain any NUL characters
+--
+-- * new storage is allocated for the C string and must be
+-- explicitly freed using 'Foreign.Marshal.Alloc.free' or
+-- 'Foreign.Marshal.Alloc.finalizerFree'.
+--
+newCString :: TextEncoding -> String -> IO CString
+newCString enc = liftM fst . newEncodedCString enc True
+
+-- | Marshal a Haskell string into a C string (ie, character array) with
+-- explicit length information.
+--
+-- Note that this does not NUL terminate the resulting string.
+--
+-- * new storage is allocated for the C string and must be
+-- explicitly freed using 'Foreign.Marshal.Alloc.free' or
+-- 'Foreign.Marshal.Alloc.finalizerFree'.
+--
+newCStringLen :: TextEncoding -> String -> IO CStringLen
+newCStringLen enc = newEncodedCString enc False
+
+-- | Marshal a Haskell string into a NUL terminated C string using temporary
+-- storage.
+--
+-- * the Haskell string may /not/ contain any NUL characters
+--
+-- * the memory is freed when the subcomputation terminates (either
+-- normally or via an exception), so the pointer to the temporary
+-- storage must /not/ be used after this.
+--
+withCString :: TextEncoding -> String -> (CString -> IO a) -> IO a
+withCString enc s act = withEncodedCString enc True s $ \(cp, _sz) -> act cp
+
+-- | Marshal a Haskell string into a C string (ie, character array)
+-- in temporary storage, with explicit length information.
+--
+-- Note that this does not NUL terminate the resulting string.
+--
+-- * the memory is freed when the subcomputation terminates (either
+-- normally or via an exception), so the pointer to the temporary
+-- storage must /not/ be used after this.
+--
+withCStringLen :: TextEncoding -> String -> (CStringLen -> IO a) -> IO a
+withCStringLen enc = withEncodedCString enc False
+
+-- | Marshal a Haskell string into a NUL-terminated C string (ie, character array)
+-- with explicit length information.
+--
+-- * new storage is allocated for the C string and must be
+-- explicitly freed using 'Foreign.Marshal.Alloc.free' or
+-- 'Foreign.Marshal.Alloc.finalizerFree'.
+--
+-- @since 4.19.0.0
+newCStringLen0 :: TextEncoding -> String -> IO CStringLen
+newCStringLen0 enc = newEncodedCString enc True
+
+-- | Marshal a Haskell string into a NUL-terminated C string (ie, character array)
+-- in temporary storage, with explicit length information.
+--
+-- * the memory is freed when the subcomputation terminates (either
+-- normally or via an exception), so the pointer to the temporary
+-- storage must /not/ be used after this.
+--
+-- @since 4.19.0.0
+withCStringLen0 :: TextEncoding -> String -> (CStringLen -> IO a) -> IO a
+withCStringLen0 enc = withEncodedCString enc True
+
+-- | Marshal a list of Haskell strings into an array of NUL terminated C strings
+-- using temporary storage.
+--
+-- * the Haskell strings may /not/ contain any NUL characters
+--
+-- * the memory is freed when the subcomputation terminates (either
+-- normally or via an exception), so the pointer to the temporary
+-- storage must /not/ be used after this.
+--
+withCStringsLen :: TextEncoding
+ -> [String]
+ -> (Int -> Ptr CString -> IO a)
+ -> IO a
+withCStringsLen enc strs f = go [] strs
+ where
+ go cs (s:ss) = withCString enc s $ \c -> go (c:cs) ss
+ go cs [] = withArrayLen (reverse cs) f
+
+-- | Determines whether a character can be accurately encoded in a
+-- 'Foreign.C.String.CString'.
+--
+-- Pretty much anyone who uses this function is in a state of sin because
+-- whether or not a character is encodable will, in general, depend on the
+-- context in which it occurs.
+charIsRepresentable :: TextEncoding -> Char -> IO Bool
+-- We force enc explicitly because `catch` is lazy in its
+-- first argument. We would probably like to force c as well,
+-- but unfortunately worker/wrapper produces very bad code for
+-- that.
+--
+-- TODO If this function is performance-critical, it would probably
+-- pay to use a single-character specialization of withCString. That
+-- would allow worker/wrapper to actually eliminate Char boxes, and
+-- would also get rid of the completely unnecessary cons allocation.
+charIsRepresentable !enc c =
+ withCString enc [c]
+ (\cstr -> do str <- peekCString enc cstr
+ case str of
+ [ch] | ch == c -> pure True
+ _ -> pure False)
+ `catch`
+ \(_ :: IOException) -> pure False
+
+-- auxiliary definitions
+-- ----------------------
+
+-- C's end of string character
+nUL :: CChar
+nUL = 0
+
+-- Size of a CChar in bytes
+cCharSize :: Int
+cCharSize = sizeOf (undefined :: CChar)
+
+
+{-# INLINE peekEncodedCString #-}
+peekEncodedCString :: TextEncoding -- ^ Encoding of CString
+ -> CStringLen
+ -> IO String -- ^ String in Haskell terms
+peekEncodedCString (TextEncoding { mkTextDecoder = mk_decoder }) (p, sz_bytes)
+ = bracket mk_decoder close $ \decoder -> do
+ let chunk_size = sz_bytes `max` 1 -- Decode buffer chunk size in characters: one iteration only for ASCII
+ !from0 <- fmap (\fp -> bufferAdd sz_bytes (emptyBuffer fp sz_bytes ReadBuffer)) $ newForeignPtr_ (castPtr p)
+ !to <- newCharBuffer chunk_size WriteBuffer
+
+ let go !iteration !from = do
+ (why, from', !to') <- encode decoder from to
+ if isEmptyBuffer from'
+ then
+ -- No input remaining: @why@ will be InputUnderflow, but we don't care
+ withBuffer to' $ peekArray (bufferElems to')
+ else do
+ -- Input remaining: what went wrong?
+ putDebugMsg ("peekEncodedCString: " ++ show iteration ++ " " ++ show why)
+ (from'', to'') <- case why of InvalidSequence -> recover decoder from' to' -- These conditions are equally bad because
+ InputUnderflow -> recover decoder from' to' -- they indicate malformed/truncated input
+ OutputUnderflow -> return (from', to') -- We will have more space next time round
+ putDebugMsg ("peekEncodedCString: from " ++ summaryBuffer from ++ " " ++ summaryBuffer from' ++ " " ++ summaryBuffer from'')
+ putDebugMsg ("peekEncodedCString: to " ++ summaryBuffer to ++ " " ++ summaryBuffer to' ++ " " ++ summaryBuffer to'')
+ to_chars <- withBuffer to'' $ peekArray (bufferElems to'')
+ fmap (to_chars++) $ go (iteration + 1) from''
+
+ go (0 :: Int) from0
+
+{-# INLINE withEncodedCString #-}
+withEncodedCString :: TextEncoding -- ^ Encoding of CString to create
+ -> Bool -- ^ Null-terminate?
+ -> String -- ^ String to encode
+ -> (CStringLen -> IO a) -- ^ Worker that can safely use the allocated memory
+ -> IO a
+withEncodedCString (TextEncoding { mkTextEncoder = mk_encoder }) null_terminate s act
+ = bracket mk_encoder close $ \encoder -> withArrayLen s $ \sz p -> do
+ from <- fmap (\fp -> bufferAdd sz (emptyBuffer fp sz ReadBuffer)) $ newForeignPtr_ p
+
+ let go !iteration to_sz_bytes = do
+ putDebugMsg ("withEncodedCString: " ++ show iteration)
+ allocaBytes to_sz_bytes $ \to_p -> do
+ -- See Note [Check *before* fill in withEncodedCString] about why
+ -- this is subtle.
+ mb_res <- tryFillBuffer encoder null_terminate from to_p to_sz_bytes
+ case mb_res of
+ Nothing -> go (iteration + 1) (to_sz_bytes * 2)
+ Just to_buf -> withCStringBuffer to_buf null_terminate act
+
+ -- If the input string is ASCII, this value will ensure we only allocate once
+ go (0 :: Int) (cCharSize * (sz + 1))
+
+withCStringBuffer :: Buffer Word8 -> Bool -> (CStringLen -> IO r) -> IO r
+withCStringBuffer to_buf null_terminate act = do
+ let bytes = bufferElems to_buf
+ withBuffer to_buf $ \to_ptr -> do
+ when null_terminate $ pokeElemOff to_ptr (bufR to_buf) 0
+ act (castPtr to_ptr, bytes) -- NB: the length information is specified as being in *bytes*
+
+{-# INLINE newEncodedCString #-}
+newEncodedCString :: TextEncoding -- ^ Encoding of CString to create
+ -> Bool -- ^ Null-terminate?
+ -> String -- ^ String to encode
+ -> IO CStringLen
+newEncodedCString (TextEncoding { mkTextEncoder = mk_encoder }) null_terminate s
+ = bracket mk_encoder close $ \encoder -> withArrayLen s $ \sz p -> do
+ from <- fmap (\fp -> bufferAdd sz (emptyBuffer fp sz ReadBuffer)) $ newForeignPtr_ p
+
+ let go !iteration to_p to_sz_bytes = do
+ putDebugMsg ("newEncodedCString: " ++ show iteration)
+ mb_res <- tryFillBuffer encoder null_terminate from to_p to_sz_bytes
+ case mb_res of
+ Nothing -> do
+ let to_sz_bytes' = to_sz_bytes * 2
+ to_p' <- reallocBytes to_p to_sz_bytes'
+ go (iteration + 1) to_p' to_sz_bytes'
+ Just to_buf -> withCStringBuffer to_buf null_terminate return
+
+ -- If the input string is ASCII, this value will ensure we only allocate once
+ let to_sz_bytes = cCharSize * (sz + 1)
+ to_p <- mallocBytes to_sz_bytes
+ go (0 :: Int) to_p to_sz_bytes
+
+
+tryFillBuffer :: TextEncoder dstate -> Bool -> Buffer Char -> Ptr Word8 -> Int
+ -> IO (Maybe (Buffer Word8))
+tryFillBuffer encoder null_terminate from0 to_p !to_sz_bytes = do
+ !to_fp <- newForeignPtr_ to_p
+ go (0 :: Int) from0 (emptyBuffer to_fp to_sz_bytes WriteBuffer)
+ where
+ go !iteration !from !to = do
+ (why, from', to') <- encode encoder from to
+ putDebugMsg ("tryFillBufferAndCall: " ++ show iteration ++ " " ++ show why ++ " " ++ summaryBuffer from ++ " " ++ summaryBuffer from')
+ if isEmptyBuffer from'
+ then if null_terminate && bufferAvailable to' == 0
+ then return Nothing -- We had enough for the string but not the terminator: ask the caller for more buffer
+ else return (Just to')
+ else case why of -- We didn't consume all of the input
+ InputUnderflow -> recover encoder from' to' >>= \(a,b) -> go (iteration + 1) a b -- These conditions are equally bad
+ InvalidSequence -> recover encoder from' to' >>= \(a,b) -> go (iteration + 1) a b -- since the input was truncated/invalid
+ OutputUnderflow -> return Nothing -- Oops, out of buffer during decoding: ask the caller for more
+{-
+Note [Check *before* fill in withEncodedCString]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+It's very important that the size check and readjustment peformed by tryFillBuffer
+happens before the continuation is called. The size check is the part which can
+fail, the call to the continuation never fails and so the caller should respond
+first to the size check failing and *then* call the continuation. Making this evident
+to the compiler avoids historic space leaks.
+
+In a previous iteration of this code we had a pattern that, somewhat simplified,
+looked like this:
+
+go :: State -> (State -> IO a) -> IO a
+go state action =
+ case tryFillBufferAndCall state action of
+ Left state' -> go state' action
+ Right result -> result
+
+`tryFillBufferAndCall` performed some checks, and then we either called action,
+or we modified the state and tried again.
+This went wrong because `action` can be a function closure containing a reference to
+a lazy data structure. If we call action directly, without retaining any references
+to action, that is fine. The data structure is consumed as it is produced and we operate
+in constant space.
+
+However the failure branch `go state' action` *does* capture a reference to action.
+This went wrong because the reference to action in the failure branch only becomes
+unreachable *after* action returns. This means we keep alive the function closure
+for `action` until `action` returns. Which in turn keeps alive the *whole* lazy list
+via `action` until the action has fully run.
+This went wrong in #20107, where the continuation kept an entire lazy bytestring alive
+rather than allowing it to be incrementally consumed and collected.
+-}
+
=====================================
libraries/base/System/Posix/Internals.hs
=====================================
@@ -34,7 +34,6 @@ import System.Posix.Types
import Foreign
import Foreign.C
--- import Data.Bits
import Data.Maybe
#if !defined(HTYPE_TCFLAG_T)
@@ -51,6 +50,9 @@ import GHC.IO.Device
#if !defined(mingw32_HOST_OS)
import {-# SOURCE #-} GHC.IO.Encoding (getFileSystemEncoding)
import qualified GHC.Foreign as GHC
+import GHC.Ptr
+#else
+import Data.OldList (elem)
#endif
-- ---------------------------------------------------------------------------
@@ -164,13 +166,23 @@ fdGetMode fd = do
#if defined(mingw32_HOST_OS)
withFilePath :: FilePath -> (CWString -> IO a) -> IO a
-withFilePath = withCWString
+withFilePath fp f = do
+ checkForInteriorNuls fp
+ withCWString fp f
newFilePath :: FilePath -> IO CWString
-newFilePath = newCWString
+newFilePath fp = do
+ checkForInteriorNuls fp
+ newCWString fp
peekFilePath :: CWString -> IO FilePath
peekFilePath = peekCWString
+
+-- | Check a 'FilePath' for internal NUL codepoints as these are
+-- disallowed in Windows filepaths. See #13660.
+checkForInteriorNuls :: FilePath -> IO ()
+checkForInteriorNuls fp = when ('\0' `elem` fp) (throwInternalNulError fp)
+
#else
withFilePath :: FilePath -> (CString -> IO a) -> IO a
@@ -178,13 +190,43 @@ newFilePath :: FilePath -> IO CString
peekFilePath :: CString -> IO FilePath
peekFilePathLen :: CStringLen -> IO FilePath
-withFilePath fp f = getFileSystemEncoding >>= \enc -> GHC.withCString enc fp f
-newFilePath fp = getFileSystemEncoding >>= \enc -> GHC.newCString enc fp
+withFilePath fp f = do
+ enc <- getFileSystemEncoding
+ GHC.withCStringLen0 enc fp $ \(str, len) -> do
+ checkForInteriorNuls fp (str, len)
+ f str
+newFilePath fp = do
+ enc <- getFileSystemEncoding
+ (str, len) <- GHC.newCStringLen0 enc fp
+ checkForInteriorNuls fp (str, len)
+ return str
peekFilePath fp = getFileSystemEncoding >>= \enc -> GHC.peekCString enc fp
peekFilePathLen fp = getFileSystemEncoding >>= \enc -> GHC.peekCStringLen enc fp
+-- | Check an encoded 'FilePath' for internal NUL octets as these are
+-- disallowed in POSIX filepaths. See #13660.
+checkForInteriorNuls :: FilePath -> CStringLen -> IO ()
+checkForInteriorNuls fp (str, len) =
+ when (len' /= len) (throwInternalNulError fp)
+ -- N.B. If the string contains internal NUL codeunits then the strlen will
+ -- indicate a size smaller than that returned by withCStringLen.
+ where
+ len' = case str of Ptr ptr -> I# (cstringLength# ptr)
#endif
+throwInternalNulError :: FilePath -> IO a
+throwInternalNulError fp = ioError err
+ where
+ err =
+ IOError
+ { ioe_handle = Nothing
+ , ioe_type = InvalidArgument
+ , ioe_location = "checkForInteriorNuls"
+ , ioe_description = "FilePaths must not contain internal NUL code units."
+ , ioe_errno = Nothing
+ , ioe_filename = Just fp
+ }
+
-- ---------------------------------------------------------------------------
-- Terminal-related stuff
=====================================
libraries/base/base.cabal
=====================================
@@ -351,6 +351,7 @@ Library
GHC.Event.IntVar
GHC.Event.PSQ
GHC.Event.Unique
+ GHC.Foreign.Internal
-- GHC.IOPort -- TODO: hide again after debug
GHC.Unicode.Internal.Bits
GHC.Unicode.Internal.Char.DerivedCoreProperties
=====================================
libraries/base/tests/T13660.hs
=====================================
@@ -0,0 +1,11 @@
+-- | This should print an InvalidArgument error complaining that
+-- the file path contains a NUL octet.
+module Main where
+
+import System.IO.Error
+
+main :: IO ()
+main = do
+ catchIOError
+ (writeFile "hello\x00world" "hello")
+ print
=====================================
libraries/base/tests/T13660.stdout
=====================================
Binary files /dev/null and b/libraries/base/tests/T13660.stdout differ
=====================================
libraries/base/tests/all.T
=====================================
@@ -256,6 +256,7 @@ test('T13191',
['-O'])
test('T13525', [when(opsys('mingw32'), skip), js_broken(22374), req_process], compile_and_run, [''])
test('T13097', normal, compile_and_run, [''])
+test('T13660', when(opsys('mingw32'), skip), compile_and_run, [''])
test('functorOperators', normal, compile_and_run, [''])
test('T3474',
[collect_stats('max_bytes_used',5),
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/385edb65878d9963ea0406887649f7312c188c57...b98d99cc1642e3bba7968e7c9993098538d9491d
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/385edb65878d9963ea0406887649f7312c188c57...b98d99cc1642e3bba7968e7c9993098538d9491d
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230518/d64ab879/attachment-0001.html>
More information about the ghc-commits
mailing list