Ensure that FilePaths don't contain interior NULs #218
Conversation
System/Win32/Types.hsc
Outdated
| @@ -189,6 +190,9 @@ newTString :: String -> IO LPCTSTR | |||
| -- UTF-16 version: | |||
| type TCHAR = CWchar | |||
| withTString = withCWString | |||
| withFilePath path f = do | |||
| checkForInteriorNuls path | |||
| withCWString path f | |||
There was a problem hiding this comment.
withCWString has the following definition:
withCWString :: String -> (CWString -> IO a) -> IO a
withCWString = withArray0 wNUL . charsToCWchars
charsToCWchars :: [Char] -> [CWchar]
charsToCWchars = foldr utf16Char [] . map ord
where
utf16Char c wcs
| c < 0x10000 = fromIntegral c : wcs
| otherwise = let c' = c - 0x10000 in
fromIntegral (c' `div` 0x400 + 0xd800) :
fromIntegral (c' `mod` 0x400 + 0xdc00) : wcsWhich I think is safe from introducing additional NUL bytes during decoding.
|
Are there any other modules dealing with filepath primitives? |
System/Win32/Types.hsc
Outdated
| @@ -203,6 +207,21 @@ peekTStringLen = peekCStringLen | |||
| newTString = newCString | |||
| -} | |||
|
|
|||
| -- | Check a 'FilePath' for internal NUL codepoints as these are | |||
| -- disallowed in Windows filepaths. See #13660. | |||
There was a problem hiding this comment.
Not that I know off, I do intend to replace the entire library at some point with autogenerated ones so we'll have more then. Thanks for the submission. Could you just update the comment to say it's a GHC #13660 and add a changelog entry. I assume this is urgent? I will make the release later tonight. |
|
Note that |
Not urgent. The CLC still hasn't voted on it. It just needs to be released in lockstep with the base version containing the fix. |
|
Ping |
|
Ah I was waiting until the CLC vote. I assume that's happened now? If so would you mind rebasing? |
|
The CLC vote passed: haskell/core-libraries-committee#144 (comment) I can rebase later, I'm out for dinner. |
hasufell
force-pushed
the
filepath-NUL
branch
from
c188923
to
868b0f4
Compare
hasufell
force-pushed
the
filepath-NUL
branch
from
868b0f4
to
e5d93f2
Compare
| let len = SBS.numWord16 path | ||
| clen <- c_wcslen ptr | ||
| if clen == fromIntegral len | ||
| then f ptr | ||
| else do | ||
| path' <- either (const (_toStr wp)) id <$> (EX.try @IOException) (decodeWithBaseWindows path) | ||
| ioError (err path') |
There was a problem hiding this comment.
This is a little more efficient than the String variant and we do the same in unix package.
hasufell
force-pushed
the
filepath-NUL
branch
from
e5d93f2
to
ddf7287
Compare
hasufell
force-pushed
the
filepath-NUL
branch
from
ddf7287
to
26482a3
Compare
hasufell
force-pushed
the
filepath-NUL
branch
from
26482a3
to
b1a6a0f
Compare
|
All done |
|
Thanks! |
|
Will you make a release? |
|
Yes, when I get home on Sunday evening.
Releases are mostly automated. If the package version is increased in the
cabal file it auto releases. But I want to double check some more.
…Sent from my Mobile
On Sat, Dec 16, 2023, 13:44 Julian Ospald ***@***.***> wrote:
Will you make a release?
—
Reply to this email directly, view it on GitHub
<#218 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAI7OKM64AJZMZT5JNMYJODYJWQVDAVCNFSM6AAAAAAWHMZ3MWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJYHAZDEMRWGE>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Follows:
Another possible implementation is to simply switch out
withTStringand make it fail on any NUL char. But I'm not sure that's what we want to do. I don't have a clear picture what other APIs this can affect, but it's clear we don't want this behavior when it's about filepaths, so we specialize here.