From c2ceb41e19634d9d5f10f90bd3b9f8871a3db6d6 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 21 Sep 2023 08:51:19 +0000 Subject: Start on various bugs related to zipfs paths. --- generic/tclZipfs.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 194 insertions(+), 23 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 45a2041..340a8d9 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -205,7 +205,7 @@ typedef struct ZipFile { struct ZipEntry *entries; /* List of files in archive */ struct ZipEntry *topEnts; /* List of top-level dirs in archive */ char *mountPoint; /* Mount point name */ - size_t mountPointLen; /* Length of mount point name */ + Tcl_Size mountPointLen; /* Length of mount point name */ #ifdef _WIN32 HANDLE mountHandle; /* Handle used for direct file access. */ #endif /* _WIN32 */ @@ -307,13 +307,14 @@ static const char *zipfs_literal_tcl_library = NULL; static int CopyImageFile(Tcl_Interp *interp, const char *imgName, Tcl_Channel out); -static inline int DescribeMounted(Tcl_Interp *interp, +static int DescribeMounted(Tcl_Interp *interp, const char *mountPoint); static int InitReadableChannel(Tcl_Interp *interp, ZipChannel *info, ZipEntry *z); static int InitWritableChannel(Tcl_Interp *interp, ZipChannel *info, ZipEntry *z, int trunc); -static inline int ListMountPoints(Tcl_Interp *interp); +static int ListMountPoints(Tcl_Interp *interp); +static int ContainsMountPoint(const char *path, int pathLen); static void SerializeCentralDirectoryEntry( const unsigned char *start, const unsigned char *end, unsigned char *buf, @@ -1075,6 +1076,123 @@ ZipFSLookupZip( return zf; } +#ifdef NOTNEEDED +/* + *------------------------------------------------------------------------ + * + * ZipFSDirExists -- + * + * Checks if a given path is a directory in a zipfs mount. A directory + * may be either a real directory in an archive or a prefix of a mount + * path. For example, if an archive is mounted at //zipfs:/a/b/c then + * //zipfs:/a and //zipfs:/a/b are treated as directories even though + * there are no entries for it in the mounted archive. + * + * Caller must hold lock before calling. + * + * Results: + * Returns 1 if a real directory, -1 if a mount prefix, and 0 otherwise. + * + * Side effects: + * None. + * + *------------------------------------------------------------------------ + */ +static int +ZipFSDirExists(const char *path) +{ + Tcl_HashEntry *hPtr; + hPtr = Tcl_FindHashEntry(&ZipFS.fileHash, path); + if (hPtr) { + ZipEntry *z = (ZipEntry *) Tcl_GetHashValue(hPtr); + return z->isDirectory ? 1 : 0; + } + + /* + * We have to do an exhaustive search. No other alternative with the + * current data structures. Still, this should be a rare case + */ + for (hPtr = Tcl_FirstHashEntry(&ZipFS.fileHash, &search); + hPtr; hPtr = Tcl_NextHashEntry(&search)) { + ZipEntry *z = (ZipEntry *) Tcl_GetHashValue(hPtr); + + if ((dirOnly >= 0) && ((dirOnly && !z->isDirectory) + || (!dirOnly && z->isDirectory))) { + continue; + } + if ((z->depth == scnt) && + ((z->flags & ZE_F_VOLUME) == 0) /* Bug 14db54d81e */ + && Tcl_StringCaseMatch(z->name, pat, 0)) { + AppendWithPrefix(result, prefixBuf, z->name + strip, -1); + } + } +} +#endif + +/* + *------------------------------------------------------------------------ + * + * ContainsMountPoint -- + * + * Check if there is a mount point anywhere under the specified path. + * Caller must hold read lock before calling. + * + * Results: + * 1 - there is at least one mount point under the path + * 0 - otherwise + * + * Side effects: + * None. + * + *------------------------------------------------------------------------ + */ +static int +ContainsMountPoint (const char *path, int pathLen) +{ + Tcl_HashEntry *hPtr; + Tcl_HashSearch search; + + if (ZipFS.zipHash.numEntries == 0) { + return 0; + } + if (pathLen < 0) + pathLen = strlen(path); + + /* + * We are looking for the case where the path is //zipfs:/a/b + * and there is a mount point //zipfs:/a/b/c/.. below it + */ + for (hPtr = Tcl_FirstHashEntry(&ZipFS.zipHash, &search); hPtr; + hPtr = Tcl_NextHashEntry(&search)) { + ZipFile *zf = (ZipFile *) Tcl_GetHashValue(hPtr); + + if (zf->mountPointLen == 0) { + /* + * Enumerate the contents of the ZIP; it's mounted on the root. + * TODO - a holdover from androwish? Tcl does not allow mounting + * outside of the //zipfs:/ area. + */ + ZipEntry *z; + + for (z = zf->topEnts; z; z = z->tnext) { + int lenz = (int) strlen(z->name); + if ((lenz >= pathLen) && + (z->name[pathLen] == '/' || z->name[pathLen] == '\0') && + (strncmp(z->name, path, pathLen) == 0)) { + return 1; + } + } + } else if ((zf->mountPointLen >= pathLen) && + (zf->mountPoint[pathLen] == '/' || + zf->mountPoint[pathLen] == '\0') && + (strncmp(zf->mountPoint, path, pathLen) == 0)) { + /* Matched standard mount */ + return 1; + } + } + return 0; +} + /* *------------------------------------------------------------------------- * @@ -1993,7 +2111,7 @@ ZipfsSetup(void) *------------------------------------------------------------------------- */ -static inline int +static int ListMountPoints( Tcl_Interp *interp) { @@ -2043,7 +2161,7 @@ ListMountPoints( *------------------------------------------------------------------------- */ -static inline int +static int DescribeMounted( Tcl_Interp *interp, const char *mountPoint) @@ -4976,17 +5094,23 @@ ZipEntryAccess( char *path, int mode) { - ZipEntry *z; - if (mode & 3) { return -1; } + ReadLock(); - z = ZipFSLookup(path); + int access; + ZipEntry *z = ZipFSLookup(path); + /* Could a real zip entry or an intermediate directory of a mount point */ + if (z || ContainsMountPoint(path, -1)) { + access = 0; + } else { + access = -1; + } Unlock(); - return (z ? 0 : -1); + return access; } - + /* *------------------------------------------------------------------------- * @@ -5321,9 +5445,10 @@ ZipFSMatchMountPoints( { Tcl_HashEntry *hPtr; Tcl_HashSearch search; - int l, normLength; + int l; + Tcl_Size normLength; const char *path = TclGetStringFromObj(normPathPtr, &normLength); - size_t len = (size_t) normLength; + Tcl_Size len = (size_t) normLength; if (len < 1) { /* @@ -5351,10 +5476,12 @@ ZipFSMatchMountPoints( /* * Enumerate the contents of the ZIP; it's mounted on the root. + * TODO - a holdover from androwish? Tcl does not allow mounting + * outside of the //zipfs:/ area. */ for (z = zf->topEnts; z; z = z->tnext) { - size_t lenz = strlen(z->name); + Tcl_Size lenz = strlen(z->name); if ((lenz > len + 1) && (strncmp(z->name, path, len) == 0) && (z->name[len] == '/') @@ -5402,7 +5529,6 @@ ZipFSPathInFilesystemProc( { Tcl_HashEntry *hPtr; Tcl_HashSearch search; - int ret = -1; Tcl_Size len; char *path; @@ -5411,39 +5537,84 @@ ZipFSPathInFilesystemProc( return -1; } path = TclGetStringFromObj(pathPtr, &len); + /* + * TODO - why not make ZIPFS_VOLUME both necessary AND sufficient? + * Currently we only claim ownership if there is an actual matching mount. + */ if (strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) != 0) { return -1; } + int ret = TCL_OK; + ReadLock(); hPtr = Tcl_FindHashEntry(&ZipFS.fileHash, path); if (hPtr) { - ret = TCL_OK; goto endloop; } + /* + * Not in hash table but still could be owned by zipfs in two other cases: + * Assuming there is a mount point //zipfs:/a/b/c, + * 1. The path is under the mount point, e.g. //zipfs:/a/b/c/f but that + * file does not exist. + * 2. The path is an intermediate directory in a mount point, e.g. + * //zipfs:/a/b + */ + for (hPtr = Tcl_FirstHashEntry(&ZipFS.zipHash, &search); hPtr; hPtr = Tcl_NextHashEntry(&search)) { ZipFile *zf = (ZipFile *) Tcl_GetHashValue(hPtr); if (zf->mountPointLen == 0) { + /* + * Mounted on the root (/) + * TODO - a holdover from androwish? Tcl does not allow mounting + * outside of the //zipfs:/ area. + */ ZipEntry *z; for (z = zf->topEnts; z != NULL; z = z->tnext) { - size_t lenz = strlen(z->name); + if (strncmp(path, z->name, len) == 0) { + int lenz = (int)strlen(z->name); + if (len == lenz) { + /* Would have been in hash table? But nm ... */ + goto endloop; + } else if (len > lenz) { + /* Case 1 above */ + if (path[lenz] == '/') { + goto endloop; + } + } else { /* len < lenz */ + /* Case 2 above */ + if (z->name[len] == '/') { + goto endloop; + } + } + } - if (((size_t) len >= lenz) && - (strncmp(path, z->name, lenz) == 0)) { - ret = TCL_OK; + } + } else { + /* Not mounted on root - the norm in Tcl core */ + + /* Lengths are known so check them before strnmp for efficiency*/ + if (len == zf->mountPointLen) { + goto endloop; + } else if (len > zf->mountPointLen) { + /* Case 1 above */ + if (path[zf->mountPointLen] == '/' && + strncmp(path, zf->mountPoint, zf->mountPointLen) == 0) { + goto endloop; + } + } else { /* len < zf->mountPointLen */ + if (zf->mountPoint[len] == '/' && + strncmp(path, zf->mountPoint, len) == 0) { goto endloop; } } - } else if (((size_t) len >= zf->mountPointLen) && - (strncmp(path, zf->mountPoint, zf->mountPointLen) == 0)) { - ret = TCL_OK; - break; } } + ret = -1; /* Not our file */ endloop: Unlock(); -- cgit v0.12 From 44b4ef27b3c9fb917be5ebf388e4e120d430de7b Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 21 Sep 2023 16:18:04 +0000 Subject: Fix file stat,exists for zipfs intermediate paths --- generic/tclZipfs.c | 80 ++++++++++++++---------------------------------------- 1 file changed, 21 insertions(+), 59 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 340a8d9..a7d246d 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -1076,65 +1076,16 @@ ZipFSLookupZip( return zf; } -#ifdef NOTNEEDED -/* - *------------------------------------------------------------------------ - * - * ZipFSDirExists -- - * - * Checks if a given path is a directory in a zipfs mount. A directory - * may be either a real directory in an archive or a prefix of a mount - * path. For example, if an archive is mounted at //zipfs:/a/b/c then - * //zipfs:/a and //zipfs:/a/b are treated as directories even though - * there are no entries for it in the mounted archive. - * - * Caller must hold lock before calling. - * - * Results: - * Returns 1 if a real directory, -1 if a mount prefix, and 0 otherwise. - * - * Side effects: - * None. - * - *------------------------------------------------------------------------ - */ -static int -ZipFSDirExists(const char *path) -{ - Tcl_HashEntry *hPtr; - hPtr = Tcl_FindHashEntry(&ZipFS.fileHash, path); - if (hPtr) { - ZipEntry *z = (ZipEntry *) Tcl_GetHashValue(hPtr); - return z->isDirectory ? 1 : 0; - } - - /* - * We have to do an exhaustive search. No other alternative with the - * current data structures. Still, this should be a rare case - */ - for (hPtr = Tcl_FirstHashEntry(&ZipFS.fileHash, &search); - hPtr; hPtr = Tcl_NextHashEntry(&search)) { - ZipEntry *z = (ZipEntry *) Tcl_GetHashValue(hPtr); - - if ((dirOnly >= 0) && ((dirOnly && !z->isDirectory) - || (!dirOnly && z->isDirectory))) { - continue; - } - if ((z->depth == scnt) && - ((z->flags & ZE_F_VOLUME) == 0) /* Bug 14db54d81e */ - && Tcl_StringCaseMatch(z->name, pat, 0)) { - AppendWithPrefix(result, prefixBuf, z->name + strip, -1); - } - } -} -#endif - /* *------------------------------------------------------------------------ * * ContainsMountPoint -- * * Check if there is a mount point anywhere under the specified path. + * Although the function will work for any path, for efficiency reasons + * it should be called only after checking ZipFSLookup does not find + * the path. + * * Caller must hold read lock before calling. * * Results: @@ -1184,7 +1135,8 @@ ContainsMountPoint (const char *path, int pathLen) } } else if ((zf->mountPointLen >= pathLen) && (zf->mountPoint[pathLen] == '/' || - zf->mountPoint[pathLen] == '\0') && + zf->mountPoint[pathLen] == '\0' || + pathLen == ZIPFS_VOLUME_LEN) && (strncmp(zf->mountPoint, path, pathLen) == 0)) { /* Matched standard mount */ return 1; @@ -5067,6 +5019,11 @@ ZipEntryStat( buf->st_ctime = z->timestamp; buf->st_atime = z->timestamp; ret = 0; + } else if (ContainsMountPoint(path, -1)) { + /* An intermediate dir under which a mount exists */ + memset(buf, 0, sizeof(Tcl_StatBuf)); + buf->st_mode = S_IFDIR | 0555; + ret = 0; } Unlock(); return ret; @@ -5365,10 +5322,10 @@ ZipFSMatchInDirectoryProc( if (!pattern || (pattern[0] == '\0')) { ZipEntry *z = ZipFSLookup(path); - - if (z && ((dirOnly < 0) || (!dirOnly && !z->isDirectory) - || (dirOnly && z->isDirectory))) { - AppendWithPrefix(result, prefixBuf, z->name, -1); + __debugbreak(); + if (z && ((dirOnly < 0) || (!dirOnly && !z->isDirectory) || + (dirOnly && z->isDirectory))) { + AppendWithPrefix(result, prefixBuf, z->name + strip, -1); } goto end; } @@ -5539,10 +5496,13 @@ ZipFSPathInFilesystemProc( path = TclGetStringFromObj(pathPtr, &len); /* * TODO - why not make ZIPFS_VOLUME both necessary AND sufficient? - * Currently we only claim ownership if there is an actual matching mount. + * Currently we only claim ownership if there is a matching mount. */ if (strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) != 0) { return -1; + } else if (len == ZIPFS_VOLUME_LEN && ZipFS.zipHash.numEntries != 0) { + /* zipfs root and at least one entry */ + return TCL_OK; } int ret = TCL_OK; @@ -5598,7 +5558,9 @@ ZipFSPathInFilesystemProc( /* Not mounted on root - the norm in Tcl core */ /* Lengths are known so check them before strnmp for efficiency*/ + assert(len != ZIPFS_VOLUME_LEN); /* Else already handled at top */ if (len == zf->mountPointLen) { + /* A non-root or root mount. */ goto endloop; } else if (len > zf->mountPointLen) { /* Case 1 above */ -- cgit v0.12 From 814c7d5faa8f01750383a3ded4aec02c22b50dd2 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Fri, 22 Sep 2023 02:51:58 +0000 Subject: Minor refactor for globbing zipfs --- generic/tclZipfs.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index a7d246d..92a39e4 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -5315,19 +5315,31 @@ ZipFSMatchInDirectoryProc( goto end; } - /* - * Can we skip the complexity of actual globbing? Without a pattern, yes; - * it's a directory existence test. - */ - - if (!pattern || (pattern[0] == '\0')) { - ZipEntry *z = ZipFSLookup(path); - __debugbreak(); - if (z && ((dirOnly < 0) || (!dirOnly && !z->isDirectory) || - (dirOnly && z->isDirectory))) { - AppendWithPrefix(result, prefixBuf, z->name + strip, -1); + /* Does the path exist in the hash table? */ + ZipEntry *z = ZipFSLookup(path); + if (z) { + /* + * Can we skip the complexity of actual globbing? Without a pattern, + * yes; it's a directory existence test. + */ + if (!pattern || (pattern[0] == '\0')) { + /* TODO - can't seem to get to this code from script for tests. */ + /* Follow logic of what tclUnixFile.c does */ + if ((dirOnly < 0) || (!dirOnly && !z->isDirectory) || + (dirOnly && z->isDirectory)) { + Tcl_ListObjAppendElement(NULL, result, pathPtr); + } + goto end; + } + } else { + /* Not in the hash table but could be an intermediate dir in a mount */ + if (!pattern || (pattern[0] == '\0')) { + /* TODO - can't seem to get to this code from script for tests. */ + if (dirOnly && ContainsMountPoint(path, len)) { + Tcl_ListObjAppendElement(NULL, result, pathPtr); + } + goto end; } - goto end; } /* -- cgit v0.12 From 5a3fed566177b092e6c88e0cf43e202697ac9f8a Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Mon, 25 Sep 2023 15:37:09 +0000 Subject: Fix file exist for zipfs --- doc/zipfs.n | 3 ++- generic/tclZipfs.c | 5 +++++ tests/zipfs.test | 36 +++++++++++++++--------------------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/doc/zipfs.n b/doc/zipfs.n index 247cac2..f9cbdc5 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -77,8 +77,9 @@ the compressed size of the file, and .IP (4) the offset of the compressed data in the ZIP archive file. .PP -Note: querying the mount point gives the start of the zip data as the offset +As a special case, querying the mount point gives the start of the zip data as the offset in (4), which can be used to truncate the zip information from an executable. +Querying an ancestor of a mount point will raise an error. .RE .TP \fBzipfs list\fR ?(\fB\-glob\fR|\fB\-regexp\fR)? ?\fIpattern\fR? diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 673c9e6..fec7586 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -3864,6 +3864,11 @@ ZipFSExistsObjCmd( ReadLock(); exists = ZipFSLookup(filename) != NULL; + if (!exists) { + /* An ancestor directory of a file ? */ + exists = ContainsMountPoint(filename, -1); + } + Unlock(); Tcl_SetObjResult(interp, Tcl_NewBooleanObj(exists)); diff --git a/tests/zipfs.test b/tests/zipfs.test index 200a8a1..d519cb7 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -706,7 +706,7 @@ namespace eval test_ns_zipfs { testzipfslist no-pattern-mount-on-empty "" {test.zip {}} {{} test testdir testdir/test2} -constraints !zipfslib testzipfslist no-pattern-mount-on-root "" [list test.zip [zipfs root]] {{} test testdir testdir/test2} -constraints !zipfslib testzipfslist no-pattern-mount-on-slash "" [list test.zip /] {{} test testdir testdir/test2} -constraints !zipfslib - testzipfslist no-pattern-mount-on-level3 "" [list test.zip testmt/a/b] {{} testmt testmt/a testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2} -constraints {bug-02acab5aea !zipfslib} + testzipfslist no-pattern-mount-on-level3 "" [list test.zip testmt/a/b] {{} testmt testmt/a testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2} -constraints {!zipfslib} testzipfslist no-pattern-multiple "" {test.zip testmountA test.zip testmountB/subdir} { testmountA testmountA/test testmountA/testdir testmountA/testdir/test2 testmountB/subdir testmountB/subdir/test testmountB/subdir/testdir testmountB/subdir/testdir/test2 @@ -739,15 +739,14 @@ namespace eval test_ns_zipfs { cleanup } -result $result {*}$args } - testzipfsexists native-file [info nameofexecutable] 0 - testzipfsexists nonexistent-file [file join $defaultMountPoint nosuchfile] 0 - testzipfsexists file [file join $defaultMountPoint test] 1 - testzipfsexists dir [file join $defaultMountPoint testdir] 1 - testzipfsexists mountpoint $defaultMountPoint 1 - testzipfsexists root [zipfs root] 1 \ - $defaultMountPoint -constraints bug-02acab5aea - testzipfsexists level3 [file join $defaultMountPoint a b] 1 \ - [file join $defaultMountPoint a b c] -constraints bug-02acab5aea + testzipfsexists native-file [info nameofexecutable] 0 + testzipfsexists enoent [file join $defaultMountPoint nosuchfile] 0 + testzipfsexists file [file join $defaultMountPoint test] 1 + testzipfsexists dir [file join $defaultMountPoint testdir] 1 + testzipfsexists mountpoint $defaultMountPoint 1 + testzipfsexists root [zipfs root] 1 $defaultMountPoint + testzipfsexists level3 [file join $defaultMountPoint a b] 1 [file join $defaultMountPoint a b c] + testzipfsexists level3-enoent [file join $defaultMountPoint a c] 0 [file join $defaultMountPoint a b c] # # zipfs find @@ -809,13 +808,12 @@ namespace eval test_ns_zipfs { testzipfsfind level3 [file join [zipfs root] testmt a] { test.zip testmt/a/b - } [zipfspaths testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2] \ - -constraints bug-02acab5aea + } [zipfspaths testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2] testzipfsfind level3-root [zipfs root] { test.zip testmt/a/b } [zipfspaths testmt testmt/a testmt/a/b testmt/a/b/test testmt/a/b/testdir testmt/a/b/testdir/test2] \ - -constraints bug-02acab5aea + -constraints bug-9e039ee0b9 test zipfs-find-native-absolute "zipfs find on native file system" -setup { set dir [makeDirectory zipfs-native-absolute] @@ -897,7 +895,7 @@ namespace eval test_ns_zipfs { cleanup } -body { zipfs info [file join [zipfs root] testmt a] - } -result {{} 0 0 0} -constraints bug-02acab5aea + } -result {path "//zipfs:/testmt/a" not found in any zipfs volume} -returnCodes error # # zipfs canonical - @@ -1276,21 +1274,17 @@ namespace eval test_ns_zipfs { lsort -stride 2 [file stat [zipfs root]] } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] - test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -constraints { - bug-02acab5aea - } -setup { + test zipfs-file-stat-root-subdir-mount "Read stat of root when mount is subdir" -setup { mount [zippath test.zip] } -cleanup cleanup -body { lsort -stride 2 [file stat [zipfs root]] } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] - test zipfs-file-stat-level3 "Stat on a directory that is intermediary in a mount point" -constraints { - bug-02acab5aea - } -setup { + test zipfs-file-stat-level3 "Stat on a directory that is intermediary in a mount point" -setup { mount [zippath test.zip] [file join $defaultMountPoint mt2] } -cleanup cleanup -body { lsort -stride 2 [file stat $defaultMountPoint] - } + } -result [fixupstat {atime 0 ctime 0 dev 0 gid 0 ino 0 mode 16749 mtime 0 nlink 0 size 0 type directory uid 0}] # # glob of zipfs file -- cgit v0.12