[squeak-dev] Review Request: noUNCPathsEliot Miranda eliot.miranda at gmail.comWed Nov 30 15:16:49 UTC 2022
Oops! I see you did cc. I should stop reading email in my phone. The letterbox is too small. _,,,^..^,,,_ (phone) > On Nov 30, 2022, at 7:16 AM, Eliot Miranda <eliot.miranda at gmail.com> wrote: > > Hi Christoph, > > looks good. Please cc messages like this which are relevant to the vm to vm-dev (aka opensmalltalk-vm). I would love to see some example paths in the comments below rather than just “don’t include ‘..’ ‘.’). Show a path that would be converted to unc path vs a proper windows path. People coming to the vm may not have experience with all platforms. Over commenting is a much better evil than overcommenting :-) > > Sorry for top posting > _,,,^..^,,,_ (phone) > >>> On Nov 30, 2022, at 6:58 AM, Christoph.Thiede at student.hpi.uni-potsdam.de wrote: >>> >> Repairs FileDirectory>>#exists for long paths (> 260 characters) on Windows by well-defining an edge case in the Windows implementation of the FilePlugin primitiveDirectoryEntry, that is, to specify a single dot (.) as the file name. Documents the present limitations to syntactic sugar in long file paths in the platform code, the relevant plugin methods, and on the image side of Squeak Trunk. >> >> An alternative consideration was to rewrite FileDirectory>>#exists to pass an empty string as file name to the primitive instead of modifying the VM, but strictly speaking, even this would have exploited an undefined behavior in the VM plugin, and an empty file name would be less idiomatic than a single dot. >> >> For the original bug report, see: https://github.com/hpi-swa-teaching/Morphic-Testing-Framework/issues/13 Thanks to Marcel (mt) for his support! >> >> Signed-off-by: Christoph Thiede <christoph.thiede at student.hpi.de> >> --- >> Note that this mail contains both a git diff for the changes in the opensmalltalk-vm repository and a changeset for the changes in the Squeak Trunk so that they may be reviewed together. Any feedback on this formatting approach will be appreciated. Excited to receive your review! >> >> platforms/win32/vm/sqWin32Directory.c | 29 +++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/platforms/win32/vm/sqWin32Directory.c b/platforms/win32/vm/sqWin32Directory.c >> index 2b3d090ec..0aa5c2108 100644 >> --- a/platforms/win32/vm/sqWin32Directory.c >> +++ b/platforms/win32/vm/sqWin32Directory.c >> @@ -196,6 +196,15 @@ sqInt dir_Lookup(char *pathString, sqInt pathLength, sqInt index, >> /* Lookup the index-th entry of the directory with the given path, starting >> at the root of the file system. Set the name, name length, creation date, >> creation time, directory flag, and file size (if the entry is a file). >> + >> + Note that, due to restrictions by the operating system, this method only >> + has limited support for paths longer than 260 characters. In the case of >> + longer paths, they must not contain syntactic sugar such as ".", "..", or >> + "a/b".* For more details, see: >> + https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#paths >> + *) Long paths are automatically converted into UNC paths, which only >> + support a subset of the normal path syntax. >> + >> Return: >> 0 if a entry is found at the given index >> 1 if the directory has fewer than index entries >> @@ -354,6 +363,20 @@ sqInt dir_EntryLookup(char *pathString, sqInt pathLength, char* nameString, sqIn >> /* Lookup a given file in a given named directory. >> Set the name, name length, creation date, >> creation time, directory flag, and file size (if the entry is a file). >> + >> + Note that, due to restrictions by the operating system, this method only >> + has limited support for paths longer than 260 characters (for the full >> + path concatenated from the directory path, a backslash, and the file >> + name). In the case of longer paths, neither the path nor the file name >> + must contain syntactic sugar such as ".", "..", or "a/b".* However, this >> + method defines a single (!) convention for passing nameString as ".", >> + which is also supported for long paths. This convention is provided for >> + an efficient existence check of the directory (e.g., >> + FileDirectory>>#exists). For more details, see: >> + https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#paths >> + *) Long paths are automatically converted into UNC paths, which only >> + support a subset of the normal path syntax. >> + >> Return: >> 0 if found (a file or directory 'nameString' exists in directory 'pathString') >> 1 if the directory has no such entry >> @@ -416,6 +439,12 @@ sqInt dir_EntryLookup(char *pathString, sqInt pathLength, char* nameString, sqIn >> fullPath=(char *)calloc(fullPathLength,sizeof(char)); >> memcpy(fullPath,pathString,pathLength); >> if (pathString[pathLength-1] != '\\') fullPath[pathLength]='\\'; >> + if (nameStringLength == 1 && nameString[0] == '.') { >> + /* special syntax: "." is the current directory. Trim it from the full >> + path to avoid generating a UNC path with an unresolved ".", which is >> + not supported there. See comment above. */ >> + nameStringLength = 0; >> + } >> memcpy(fullPath+fullPathLength-nameStringLength,nameString,nameStringLength); >> >> /* convert the path name into a null-terminated C string */ >> -- >> 2.37.3.windows.1 >> >> =============== Summary =============== >> >> Change Set: noUNCPaths >> Date: 30 November 2022 >> Author: Christoph Thiede >> >> Documents the present limitations to syntactic sugar in long file paths in the relevant plugin methods and on the image side. >> >> For the original bug report, see: https://github.com/hpi-swa-teaching/Morphic-Testing-Framework/issues/13 Thanks to Marcel (mt) for his support! >> >> =============== Diff =============== >> >> FileDirectory>>primLookupEntryIn:index: {private} · ct 11/29/2022 20:24 (changed) >> primLookupEntryIn: fullPath index: index >> "Look up the index-th entry of the directory with the given fully-qualified path (i.e., starting from the root of the file hierarchy) and return an array containing: >> >> <name> <creationTime> <modificationTime> <dirFlag> <fileSize> >> >> The empty string enumerates the top-level files or drives. (For example, on Unix, the empty path enumerates the contents of '/'. On Macs and PCs, it enumerates the mounted volumes/drives.) >> + Note that in general, fullPath must not contain syntactic sugar for the current platform (e.g., '.' or '..', or on Windows, forward slashes instead of backslashes). These conventions are only fully supported on Unix platforms; on Windows, they are only supported for short non-UNC file paths containing max 260 characters. See the comment in the primitive implementation for more details. >> >> The creation and modification times are in seconds since the start of the Smalltalk time epoch. DirFlag is true if the entry is a directory. FileSize the file size in bytes or zero for directories. The primitive returns nil when index is past the end of the directory. It fails if the given path is bad." >> >> <primitive: 'primitiveDirectoryLookup' module: 'FilePlugin'> >> ^ #badDirectoryPath >> >> >> >> FileDirectory>>primLookupEntryIn:name: {private} · ct 11/30/2022 15:38 (changed) >> primLookupEntryIn: fullPath name: fName >> - >> "Look up <fName> (a simple file name) in the directory identified by <fullPath> >> and return an array containing: >> >> <fName> <creationTime> <modificationTime> <dirFlag> <fileSize> >> >> On Unix, the empty path denotes '/'. >> On Macs and PCs, it is the container of the system volumes.) >> + Note that in general, neither fullPath nor fName must contain syntactic sugar for the current platform (e.g., '.' or '..', or on Windows, forward slashes instead of backslashes). These conventions are only fully supported on Unix platforms; on Windows, they are only supported for short non-UNC file paths containing max 260 characters (for the full path concatenated from the directory path, a backslash, and the file name). As a single (!) exception, the file name may always be a single dot ('.'), which is supported for an efficient existence test of the directory path (e.g., FileDirectory>>#exists). See the comment in the primitive implementation for more details. >> >> The creation and modification times are in seconds since the start of the Smalltalk time epoch. DirFlag is true if the entry is a directory. FileSize the file size in bytes or zero for directories. The primitive returns nil when index is past the end of the directory. It fails if the given path is bad." >> >> <primitive: 'primitiveDirectoryEntry' module: 'FilePlugin'> >> >> ^ #primFailed "to distinguish from nil" >> >> >> >> FilePlugin>>primitiveDirectoryEntry {directory primitives} · ct 11/30/2022 15:13 (changed) >> primitiveDirectoryEntry >> >> "Two arguments - directory path, and simple file name; >> returns an array (see primitiveDirectoryLookup) describing the file or directory, >> or nil if it does not exist. >> + >> + Note that in general, neither the directory path nor the file name must >> + contain syntactic sugar for the current platform (e.g., '.' or '..', or on >> + Windows, forward slashes instead of backslashes). These conventions are >> + only fully supported on Unix platforms; on Windows, they are only >> + supported for short non-UNC file paths containing max 260 characters (for >> + the full path concatenated from the directory path, a backslash, and the >> + file name). As a single (!) exception, the file name may always be a >> + single dot ('.'), which is supported for an efficient existence test of >> + the directory path (e.g., FileDirectory>>#exists). See the comment in >> + sqWin32Directory.c for more details. >> + >> Primitive fails if the outer path does not identify a readable directory. >> (This is a lookup-by-name variant of primitiveDirectoryLookup.)" >> >> | requestedName pathName pathNameIndex pathNameSize status entryName entryNameSize createDate modifiedDate dirFlag posixPermissions symlinkFlag fileSize okToList reqNameIndex reqNameSize | >> >> <var: 'entryName' declareC: 'char entryName[256]'> >> <var: 'pathNameIndex' type: #'char *'> >> <var: 'reqNameIndex' type: #'char *'> >> <var: 'fileSize' type: #squeakFileOffsetType> >> <export: true> >> >> requestedName := interpreterProxy stackValue: 0. >> pathName := interpreterProxy stackValue: 1. >> (interpreterProxy isBytes: pathName) ifFalse: >> [^interpreterProxy primitiveFail]. >> >> "Outbound string parameters" >> pathNameIndex := interpreterProxy firstIndexableField: pathName. >> pathNameSize := interpreterProxy byteSizeOf: pathName. >> >> reqNameIndex := interpreterProxy firstIndexableField: requestedName. >> reqNameSize := interpreterProxy byteSizeOf: requestedName. >> self cCode: '' inSmalltalk: >> [entryName := ByteString new: 256. >> entryNameSize := createDate := modifiedDate := dirFlag := fileSize := posixPermissions := symlinkFlag := nil]. >> "If the security plugin can be loaded, use it to check for permission. >> If not, assume it's ok" >> okToList := sCLPfn ~= 0 >> ifTrue: [self cCode: '((sqInt (*)(char *, sqInt))sCLPfn)(pathNameIndex, pathNameSize)' inSmalltalk: [true]] >> ifFalse: [true]. >> status := okToList >> ifTrue: >> [self dir_EntryLookup: pathNameIndex _: pathNameSize >> _: reqNameIndex _: reqNameSize >> _: entryName _: (self addressOf: entryNameSize put: [:v| entryNameSize := v]) >> _: (self addressOf: createDate put: [:v| createDate := v]) >> _: (self addressOf: modifiedDate put: [:v| modifiedDate := v]) >> _: (self addressOf: dirFlag put: [:v| dirFlag := v]) >> _: (self addressOf: fileSize put: [:v| fileSize := v]) >> _: (self addressOf: posixPermissions put: [:v| posixPermissions := v]) >> _: (self addressOf: symlinkFlag put: [:v| symlinkFlag := v])] >> ifFalse: >> [DirNoMoreEntries]. >> >> interpreterProxy failed ifTrue: >> [^nil]. >> status = DirNoMoreEntries ifTrue: "no entry; return nil" >> [interpreterProxy "pop pathName, index, rcvr" >> pop: 3 thenPush: interpreterProxy nilObject. >> ^nil]. >> status = DirBadPath ifTrue: >> [^interpreterProxy primitiveFail]."bad path" >> >> interpreterProxy >> pop: 3 "pop pathName, index, rcvr" >> thenPush: >> (self >> cppIf: PharoVM >> ifTrue: >> [self >> makeDirEntryName: entryName >> size: entryNameSize >> createDate: createDate >> modDate: modifiedDate >> isDir: dirFlag >> fileSize: fileSize >> posixPermissions: posixPermissions >> isSymlink: symlinkFlag] >> ifFalse: >> [self >> makeDirEntryName: entryName >> size: entryNameSize >> createDate: createDate >> modDate: modifiedDate >> isDir: dirFlag >> fileSize: fileSize]) >> >> FilePlugin>>primitiveDirectoryLookup {directory primitives} · ct 11/30/2022 15:00 (changed) >> primitiveDirectoryLookup >> + "Two arguments - directory path, and an index to an item; returns an array (see primitiveDirectoryLookup) describing the file or directory, or nil if it does not exist. >> + >> + Note that in general, the directory path must not contain syntactic sugar for the current platform (e.g., '.' or '..', or on Windows, forward slashes instead of backslashes). These conventions are only fully supported on Unix platforms; on Windows, they are only supported for short non-UNC file paths containing max 260 characters. See the comment in sqWin32Directory.c for more details. >> + >> + Primitive fails if the outer path does not identify a readable directory. (For a lookup-by-name variant, see primitiveDirectoryEntry.)" >> >> | index pathName pathNameIndex pathNameSize status entryName entryNameSize createDate modifiedDate dirFlag symlinkFlag posixPermissions fileSize okToList | >> >> <var: 'entryName' declareC: 'char entryName[256]'> >> <var: 'pathNameIndex' type: #'char *'> >> <var: 'fileSize' type: #squeakFileOffsetType> >> <export: true> >> >> index := interpreterProxy stackIntegerValue: 0. >> pathName := interpreterProxy stackValue: 1. >> (interpreterProxy isBytes: pathName) >> ifFalse: [^interpreterProxy primitiveFail]. >> pathNameIndex := interpreterProxy firstIndexableField: pathName. >> pathNameSize := interpreterProxy byteSizeOf: pathName. >> self cCode: '' inSmalltalk: >> [entryName := ByteString new: 256. >> entryNameSize := createDate := modifiedDate := dirFlag := fileSize := posixPermissions := symlinkFlag := nil]. >> "If the security plugin can be loaded, use it to check for permission. >> If not, assume it's ok" >> okToList := sCLPfn ~= 0 >> ifTrue: [self cCode: '((sqInt (*)(char *, sqInt))sCLPfn)(pathNameIndex, pathNameSize)' inSmalltalk: [true]] >> ifFalse: [true]. >> status := okToList >> ifTrue: >> [self dir_Lookup: pathNameIndex _: pathNameSize >> _: index >> _: entryName _: (self addressOf: entryNameSize put: [:v| entryNameSize := v]) >> _: (self addressOf: createDate put: [:v| createDate := v]) >> _: (self addressOf: modifiedDate put: [:v| modifiedDate := v]) >> _: (self addressOf: dirFlag put: [:v| dirFlag := v]) >> _: (self addressOf: fileSize put: [:v| fileSize := v]) >> _: (self addressOf: posixPermissions put: [:v| posixPermissions := v]) >> _: (self addressOf: symlinkFlag put: [:v| symlinkFlag := v])] >> ifFalse: [DirNoMoreEntries]. >> interpreterProxy failed ifTrue: >> [^nil]. >> status = DirNoMoreEntries ifTrue: "no more entries; return nil" >> [interpreterProxy "pop pathName, index, rcvr" >> pop: 3 thenPush: interpreterProxy nilObject. >> ^nil]. >> status = DirBadPath ifTrue: >> [^interpreterProxy primitiveFail]."bad path" >> >> interpreterProxy >> pop: 3 "pop pathName, index, rcvr" >> thenPush: >> (self >> cppIf: PharoVM >> ifTrue: >> [self >> makeDirEntryName: entryName >> size: entryNameSize >> createDate: createDate >> modDate: modifiedDate >> isDir: dirFlag >> fileSize: fileSize >> posixPermissions: posixPermissions >> isSymlink: symlinkFlag] >> ifFalse: >> [self >> makeDirEntryName: entryName >> size: entryNameSize >> createDate: createDate >> modDate: modifiedDate >> isDir: dirFlag >> fileSize: fileSize]) >> >> --- >> Sent from Squeak Inbox Talk >> ["noUNCPaths.3.cs"] > >> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20221130/9172446f/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: noUNCPaths.3.cs Type: application/octet-stream Size: 11726 bytes Desc: not available URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20221130/9172446f/attachment.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20221130/9172446f/attachment-0001.html>
More information about the Squeak-dev mailing list |