-- cgit v0.12 From d64ced490d84fc3d9882daae3dad861769ee41da Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 14 Oct 2023 15:51:20 +0000 Subject: Redo zipfs path handling --- doc/filename.n | 4 +- doc/zipfs.n | 2 + generic/tclZipfs.c | 409 ++++++++++++++++++++++++++++++++++++++--------------- tests/zipfs.test | 17 ++- 4 files changed, 311 insertions(+), 121 deletions(-) diff --git a/doc/filename.n b/doc/filename.n index d236b7f..801e346 100644 --- a/doc/filename.n +++ b/doc/filename.n @@ -122,8 +122,8 @@ extra backslashes are superfluous. .TP \fBZipfs\fR On all platforms where \fBzipfs\fR support is enabled, paths within mounted -ZIP archives begin with the string returned by the \fBzipfs root\fR command -and otherwise follow the rules for the Unix paths. +ZIP archives begin with the string returned by the \fBzipfs root\fR command. +Zipfs paths are case-sensitive on all platforms. .RE .SH "TILDE SUBSTITUTION" .PP diff --git a/doc/zipfs.n b/doc/zipfs.n index 7cb050e..fa361ef 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -47,6 +47,8 @@ zip64 formats and other compression methods like bzip2 are not supported. Files within mounted archives can be written to but new files or directories cannot be created. Further, modifications to files are limited to the mounted archive in memory and are not persisted to disk. +.PP +Paths in mounted archives are case-sensitive on all platforms. .TP \fBzipfs canonical\fR ?\fImountpoint\fR? \fIfilename\fR ?\fIinZipfs\fR? . diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index 27e002e..a7c96c4 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -491,7 +491,18 @@ static Tcl_ChannelType ZipChannelType = { */ int TclIsZipfsPath (const char *path) { +#ifdef _WIN32 return strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) ? 0 : ZIPFS_VOLUME_LEN; +#else + int i; + for (i = 0; i < ZIPFS_VOLUME_LEN; ++i) { + if (path[i] != ZIPFS_VOLUME[i] && + (path[i] != '\\' || ZIPFS_VOLUME[i] != '/')) { + return 0; + } + } + return ZIPFS_VOLUME_LEN; +#endif } /* @@ -959,6 +970,166 @@ DecodeZipEntryText( } /* + *------------------------------------------------------------------------ + * + * NormalizeMountPoint -- + * + * Converts the passed path into a normalized zipfs mount point + * of the form //zipfs:/some/path. On Windows any \ path separators + * are converted to /. + * + * Mount points with a volume will raise an error unless the volume is + * zipfs root. Thus D:/foo is not a valid mount point. + * + * Relative paths and absolute paths without a volume are mapped under + * the zipfs root. + * + * The empty string is mapped to the zipfs root. + * + * dsPtr is initialized by the function and must be cleared by caller + * on a successful return. + * + * Results: + * TCL_OK on success with normalized mount path in dsPtr + * TCL_ERROR on fail with error message in interp if not NULL + * + *------------------------------------------------------------------------ + */ +static int +NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPtr) +{ + const char *joiner[2]; + char *joinedPath; + Tcl_Obj *unnormalizedObj; + Tcl_Obj *normalizedObj; + const char *normalizedPath; + Tcl_Size normalizedLen; + + Tcl_DStringInit(dsPtr); + + /* + * Several things need to happen here + * - Absolute paths containing volumes (drive letter or UNC) raise error + * except of course if the volume is zipfs root + * - \ need to be converted to / + * - \ -> / and // -> / conversions (except if UNC which is error) + * - . and .. have to be dealt with + * The first is explicitly checked, the others are dealt with a + * combination file join and normalize. Easier than doing it ourselves + * and not performance sensitive anyways. + */ + + joiner[0] = ZIPFS_VOLUME; + joiner[1] = mountPath; + joinedPath = Tcl_JoinPath(2, joiner, dsPtr); + + /* Now joinedPath has all \ -> / and // -> / (except UNC) converted. */ + + if (!strncmp(ZIPFS_VOLUME, joinedPath, ZIPFS_VOLUME_LEN)) { + unnormalizedObj = Tcl_DStringToObj(dsPtr); + } else { + if (joinedPath[0] != '/' || joinedPath[1] == '/') { + /* D:/x, D:x or //unc */ + goto invalidMountPath; + } + unnormalizedObj = Tcl_ObjPrintf(ZIPFS_VOLUME "%s", joinedPath + 1); + } + Tcl_IncrRefCount(unnormalizedObj); + normalizedObj = Tcl_FSGetNormalizedPath(interp, unnormalizedObj); + if (normalizedObj == NULL) { + Tcl_DecrRefCount(unnormalizedObj); + goto errorReturn; + } + Tcl_IncrRefCount(normalizedObj); /* BEFORE DecrRefCount on unnormalizedObj */ + Tcl_DecrRefCount(unnormalizedObj); + + /* normalizedObj owned by Tcl!! Do NOT DecrRef without an IncrRef */ + normalizedPath = Tcl_GetStringFromObj(normalizedObj, &normalizedLen); + Tcl_DStringFree(dsPtr); /* Reset */ + Tcl_DStringAppend(dsPtr, normalizedPath, normalizedLen); + Tcl_DecrRefCount(normalizedObj); + return TCL_OK; + +invalidMountPath: + if (interp) { + Tcl_SetObjResult(interp, + Tcl_ObjPrintf("Invalid mount path \"%s\"", mountPath)); + ZIPFS_ERROR_CODE(interp, "MOUNT_PATH"); + } + +errorReturn: + Tcl_DStringFree(dsPtr); + return TCL_ERROR; +} + +/* + *------------------------------------------------------------------------ + * + * MapPathToZipfs -- + * + * Maps a path as stored in a zip archive to its normalized location + * under a given zipfs mount point. Relative paths and Unix style + * absolute paths go directly under the mount point. Volume relative + * paths and absolute paths that have a volume (drive or UNC) are + * stripped of the volume before joining the mount point. + * + * Results: + * Pointer to normalized path. + * + * Side effects: + * Stores mapped path in dsPtr. + * + *------------------------------------------------------------------------ + */ +static char * +MapPathToZipfs(Tcl_Interp *interp, + const char *mountPath, /* Must be fully normalized */ + const char *path, /* Archive content path to map */ + Tcl_DString *dsPtr) /* Must be cleared on success return */ +{ + const char *joiner[2]; + char *joinedPath; + Tcl_Obj *unnormalizedObj; + Tcl_Obj *normalizedObj; + const char *normalizedPath; + Tcl_Size normalizedLen; + + assert(TclIsZipfsPath(mountPath)); + Tcl_DStringInit(dsPtr); + + joiner[0] = mountPath; + joiner[1] = path; + joinedPath = Tcl_JoinPath(2, joiner, dsPtr); + + if (strncmp(ZIPFS_VOLUME, joinedPath, ZIPFS_VOLUME_LEN)) { + /* path was not relative. Strip off the volume */ + Tcl_Size numParts; + const char **partsPtr; + Tcl_SplitPath(path, &numParts, &partsPtr); + Tcl_DStringFree(dsPtr); + partsPtr[0] = ZIPFS_VOLUME; + (void)Tcl_JoinPath(numParts, partsPtr, dsPtr); + ckfree(partsPtr); + } + unnormalizedObj = Tcl_DStringToObj(dsPtr); /* Also resets dsPtr */ + Tcl_IncrRefCount(unnormalizedObj); + normalizedObj = Tcl_FSGetNormalizedPath(interp, unnormalizedObj); + if (normalizedObj == NULL) { + /* Should not happen but continue... */ + normalizedObj = unnormalizedObj; + } + Tcl_IncrRefCount(normalizedObj); /* BEFORE DecrRefCount on unnormalizedObj */ + Tcl_DecrRefCount(unnormalizedObj); + + /* normalizedObj owned by Tcl!! Do NOT DecrRef without an IncrRef */ + Tcl_DStringFree(dsPtr); /* Reset */ + normalizedPath = Tcl_GetStringFromObj(normalizedObj, &normalizedLen); + Tcl_DStringAppend(dsPtr, normalizedPath, normalizedLen); + Tcl_DecrRefCount(normalizedObj); + return Tcl_DStringValue(dsPtr); +} + +/* *------------------------------------------------------------------------- * * CanonicalPath -- @@ -1850,7 +2021,7 @@ static int ZipFSCatalogFilesystem( Tcl_Interp *interp, /* Current interpreter. NULLable. */ ZipFile *zf, /* Temporary buffer hold archive descriptors */ - const char *mountPoint, /* Mount point path. */ + const char *mountPoint, /* Mount point path. Must be fully normalized */ const char *passwd, /* Password for opening the ZIP, or NULL if * the ZIP is unprotected. */ const char *zipname) /* Path to ZIP file to build a catalog of. */ @@ -1860,9 +2031,13 @@ ZipFSCatalogFilesystem( ZipFile *zf0; ZipEntry *z; Tcl_HashEntry *hPtr; - Tcl_DString ds, dsm, fpBuf; + Tcl_DString ds, fpBuf; unsigned char *q; + assert(TclIsZipfsPath(mountPoint)); /* Caller should have normalized */ + + Tcl_DStringInit(&ds); + /* * Basic verification of the password for sanity. */ @@ -1892,17 +2067,6 @@ ZipFSCatalogFilesystem( WriteLock(); - /* - * Mount point sometimes is a relative or otherwise denormalized path. - * But an absolute name is needed as mount point here. - */ - - Tcl_DStringInit(&ds); - Tcl_DStringInit(&dsm); - if (strcmp(mountPoint, "/") == 0) { - mountPoint = ""; - } - mountPoint = CanonicalPath("", mountPoint, &dsm, 1); hPtr = Tcl_CreateHashEntry(&ZipFS.zipHash, mountPoint, &isNew); if (!isNew) { if (interp) { @@ -1913,6 +2077,7 @@ ZipFSCatalogFilesystem( } Unlock(); ZipFSCloseArchive(interp, zf); + Tcl_DStringFree(&ds); ckfree(zf); return TCL_ERROR; } @@ -1939,6 +2104,7 @@ ZipFSCatalogFilesystem( } zf->passBuf[k] = '\0'; } + /* TODO - is this test necessary? WHen will mountPoint[0] be \0 ? */ if (mountPoint[0] != '\0') { hPtr = Tcl_CreateHashEntry(&ZipFS.fileHash, mountPoint, &isNew); if (isNew) { @@ -2038,7 +2204,7 @@ ZipFSCatalogFilesystem( } Tcl_DStringSetLength(&fpBuf, 0); - fullpath = CanonicalPath(mountPoint, path, &fpBuf, 1); + fullpath = MapPathToZipfs(interp, mountPoint, path, &fpBuf); z = AllocateZipEntry(); z->depth = CountSlashes(fullpath); assert(z->depth >= ZIPFS_ROOTDIR_DEPTH); @@ -2129,9 +2295,9 @@ ZipFSCatalogFilesystem( nextent: q += pathlen + comlen + extra + ZIP_CENTRAL_HEADER_LEN; } + Unlock(); Tcl_DStringFree(&fpBuf); Tcl_DStringFree(&ds); - Unlock(); Tcl_FSMountsChanged(NULL); return TCL_OK; } @@ -2316,13 +2482,13 @@ DescribeMounted( int TclZipfs_Mount( Tcl_Interp *interp, /* Current interpreter. NULLable. */ - const char *zipname, /* Path to ZIP file to mount; should be - * normalized. */ + const char *zipname, /* Path to ZIP file to mount */ const char *mountPoint, /* Mount point path. */ const char *passwd) /* Password for opening the ZIP, or NULL if * the ZIP is unprotected. */ { ZipFile *zf; + int ret; ReadLock(); if (!ZipFS.initialized) { @@ -2333,45 +2499,51 @@ TclZipfs_Mount( * No mount point, so list all mount points and what is mounted there. */ - if (!mountPoint) { - int ret = ListMountPoints(interp); + if (mountPoint == NULL) { + ret = ListMountPoints(interp); Unlock(); return ret; } - /* - * Mount point but no file, so describe what is mounted at that mount - * point. - */ - - if (!zipname) { - DescribeMounted(interp, mountPoint); + Tcl_DString ds; + ret = NormalizeMountPoint(interp, mountPoint, &ds); + if (ret != TCL_OK) { Unlock(); - return TCL_OK; + return ret; } - Unlock(); + mountPoint = Tcl_DStringValue(&ds); - /* - * Have both a mount point and a file (name) to mount there. - */ + if (!zipname) { + /* + * Mount point but no file, so describe what is mounted at that mount + * point. + */ - if (passwd && IsPasswordValid(interp, passwd, strlen(passwd)) != TCL_OK) { - return TCL_ERROR; - } - zf = AllocateZipFile(interp, strlen(mountPoint)); - if (!zf) { - return TCL_ERROR; - } - if (ZipFSOpenArchive(interp, zipname, 1, zf) != TCL_OK) { - ckfree(zf); - return TCL_ERROR; - } - if (ZipFSCatalogFilesystem(interp, zf, mountPoint, passwd, zipname) - != TCL_OK) { - /* zf would have been freed! */ - return TCL_ERROR; + ret = DescribeMounted(interp, mountPoint); + Unlock(); + } else { + Unlock(); + + /* Have both a mount point and a file (name) to mount there. */ + if (passwd == NULL || + (ret = IsPasswordValid(interp, passwd, strlen(passwd))) == TCL_OK) { + zf = AllocateZipFile(interp, strlen(mountPoint)); + if (zf == NULL) { + ret = TCL_ERROR; + } else { + ret = ZipFSOpenArchive(interp, zipname, 1, zf); + if (ret != TCL_OK) { + ckfree(zf); + } else { + ret = ZipFSCatalogFilesystem( + interp, zf, mountPoint, passwd, zipname); + /* Note zf is already freed on error! */ + } + } + } } - return TCL_OK; + Tcl_DStringFree(&ds); + return ret; } /* @@ -2401,6 +2573,7 @@ TclZipfs_MountBuffer( int copy) { ZipFile *zf; + int ret; /* TODO - how come a *read* lock suffices for initialzing ? */ ReadLock(); @@ -2413,62 +2586,72 @@ TclZipfs_MountBuffer( */ if (!mountPoint) { - int ret = ListMountPoints(interp); + ret = ListMountPoints(interp); Unlock(); return ret; } - /* - * Mount point but no data, so describe what is mounted at that mount - * point. - */ - - if (!data) { - DescribeMounted(interp, mountPoint); + Tcl_DString ds; + ret = NormalizeMountPoint(interp, mountPoint, &ds); + if (ret != TCL_OK) { Unlock(); - return TCL_OK; + return ret; } - Unlock(); + mountPoint = Tcl_DStringValue(&ds); - /* - * Have both a mount point and data to mount there. - * What's the magic about 64 * 1024 * 1024 ? - */ - if ((datalen <= ZIP_CENTRAL_END_LEN) || - (datalen - ZIP_CENTRAL_END_LEN) > - (64 * 1024 * 1024 - ZIP_CENTRAL_END_LEN)) { - ZIPFS_ERROR(interp, "illegal file size"); - ZIPFS_ERROR_CODE(interp, "FILE_SIZE"); - return TCL_ERROR; - } + if (data == NULL) { + /* Mount point but no data, so describe what is mounted at there */ + ret = DescribeMounted(interp, mountPoint); + Unlock(); + } else { + Unlock(); - zf = AllocateZipFile(interp, strlen(mountPoint)); - if (!zf) { - return TCL_ERROR; - } - zf->isMemBuffer = 1; - zf->length = datalen; - if (copy) { - zf->data = (unsigned char *) attemptckalloc(datalen); - if (!zf->data) { - ZipFSCloseArchive(interp, zf); + /* + * Have both a mount point and data to mount there. + * What's the magic about 64 * 1024 * 1024 ? + */ + ret = TCL_ERROR; + if ((datalen <= ZIP_CENTRAL_END_LEN) || + (datalen - ZIP_CENTRAL_END_LEN) > + (64 * 1024 * 1024 - ZIP_CENTRAL_END_LEN)) { + ZIPFS_ERROR(interp, "illegal file size"); + ZIPFS_ERROR_CODE(interp, "FILE_SIZE"); + goto done; + } + zf = AllocateZipFile(interp, strlen(mountPoint)); + if (zf == NULL) { + goto done; + } + zf->isMemBuffer = 1; + zf->length = datalen; + + if (copy) { + zf->data = (unsigned char *)attemptckalloc(datalen); + if (zf->data == NULL) { + ZipFSCloseArchive(interp, zf); + ckfree(zf); + ZIPFS_MEM_ERROR(interp); + goto done; + } + memcpy(zf->data, data, datalen); + zf->ptrToFree = zf->data; + } else { + zf->data = (unsigned char *)data; + zf->ptrToFree = NULL; + } + ret = ZipFSFindTOC(interp, 1, zf); + if (ret != TCL_OK) { ckfree(zf); - ZIPFS_MEM_ERROR(interp); - return TCL_ERROR; + } else { + /* Note ZipFSCatalogFilesystem will free zf on error */ + ret = ZipFSCatalogFilesystem( + interp, zf, mountPoint, NULL, "Memory Buffer"); } - memcpy(zf->data, data, datalen); - zf->ptrToFree = zf->data; - } else { - zf->data = (unsigned char *) data; - zf->ptrToFree = NULL; - } - if (ZipFSFindTOC(interp, 1, zf) != TCL_OK) { - ckfree(zf); - return TCL_ERROR; } - /* Note ZipFSCatalogFilesystem will free zf on error */ - return ZipFSCatalogFilesystem( - interp, zf, mountPoint, NULL, "Memory Buffer"); + +done: + Tcl_DStringFree(&ds); + return ret; } /* @@ -2497,6 +2680,8 @@ TclZipfs_Unmount( Tcl_DString dsm; int ret = TCL_OK, unmounted = 0; + Tcl_DStringInit(&dsm); + WriteLock(); if (!ZipFS.initialized) { goto done; @@ -2507,8 +2692,10 @@ TclZipfs_Unmount( * But an absolute name is needed as mount point here. */ - Tcl_DStringInit(&dsm); - mountPoint = CanonicalPath("", mountPoint, &dsm, 1); + if (NormalizeMountPoint(interp, mountPoint, &dsm) != TCL_OK) { + goto done; + } + mountPoint = Tcl_DStringValue(&dsm); hPtr = Tcl_FindHashEntry(&ZipFS.zipHash, mountPoint); /* don't report no-such-mount as an error */ @@ -2538,6 +2725,7 @@ TclZipfs_Unmount( done: Unlock(); + Tcl_DStringFree(&dsm); if (unmounted) { Tcl_FSMountsChanged(NULL); } @@ -2632,34 +2820,25 @@ ZipFSMountBufferObjCmd( int objc, /* Number of arguments. */ Tcl_Obj *const objv[]) /* Argument objects. */ { - const char *mountPoint; /* Mount point path. */ - unsigned char *data; + const char *mountPoint = NULL; /* Mount point path. */ + unsigned char *data = NULL; Tcl_Size length; if (objc > 3) { Tcl_WrongNumArgs(interp, 1, objv, "?data? ?mountpoint?"); return TCL_ERROR; } - if (objc < 2) { - int ret; - - ReadLock(); - ret = ListMountPoints(interp); - Unlock(); - return ret; - } - - if (objc < 3) { - ReadLock(); - DescribeMounted(interp, Tcl_GetString(objv[1])); - Unlock(); - return TCL_OK; - } - data = Tcl_GetBytesFromObj(interp, objv[1], &length); - mountPoint = Tcl_GetString(objv[2]); - if (data == NULL) { - return TCL_ERROR; + if (objc > 1) { + if (objc == 2) { + mountPoint = Tcl_GetString(objv[1]); + } else { + data = Tcl_GetBytesFromObj(interp, objv[1], &length); + mountPoint = Tcl_GetString(objv[2]); + if (data == NULL) { + return TCL_ERROR; + } + } } return TclZipfs_MountBuffer(interp, data, length, mountPoint, 1); } diff --git a/tests/zipfs.test b/tests/zipfs.test index 55cce1f..88bee93 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -913,10 +913,16 @@ namespace eval test_ns_zipfs { -body [list zipfs canonical {*}$cmdargs] \ -result $result {*}$args } - testzipfscanonical basic-relative PATH [file join [zipfs root] PATH] - testzipfscanonical basic-absolute /PATH [file join [zipfs root] PATH] - testzipfscanonical mountpoint-relative {MT PATH} [file join [zipfs root] MT PATH] - testzipfscanonical mountpoint-absolute {MT /PATH} [file join [zipfs root] PATH] + testzipfscanonical default-relative PATH [file join [zipfs root] PATH] + testzipfscanonical default-absolute /PATH [file join [zipfs root] PATH] + testzipfscanonical root-relative-1 [list [zipfs root] PATH] [file join [zipfs root] PATH] + testzipfscanonical root-relative-2 [list / PATH] [file join [zipfs root] PATH] + testzipfscanonical root-absolute-1 [list [zipfs root] /PATH] [file join [zipfs root] PATH] + testzipfscanonical root-absolute-2 [list / /PATH] [file join [zipfs root] PATH] + testzipfscanonical absolute-relative {/MT PATH} [file join [zipfs root] MT PATH] + testzipfscanonical absolute-absolute {/MT /PATH} [file join [zipfs root] PATH] + testzipfscanonical relative-relative {MT PATH} [file join [zipfs root] MT PATH] + testzipfscanonical relative-absolute {MT /PATH} [file join [zipfs root] PATH] testzipfscanonical mountpoint-trailslash-relative {MT/ PATH} [file join [zipfs root] MT PATH] testzipfscanonical mountpoint-trailslash-absolute {MT/ /PATH} [file join [zipfs root] PATH] testzipfscanonical mountpoint-root-relative [list [zipfs root] PATH] [file join [zipfs root] PATH] @@ -925,9 +931,12 @@ namespace eval test_ns_zipfs { testzipfscanonical driveletter X: [zipfs root] -constraints win testzipfscanonical drivepath X:/foo/bar [file join [zipfs root] foo bar] -constraints win + testzipfscanonical drivepath {MT X:/foo/bar} [file join [zipfs root] MT foo bar] -constraints win # (backslashes need additional escaping passed to testzipfscanonical) testzipfscanonical backslashes X:\\\\foo\\\\bar [file join [zipfs root] foo bar] -constraints win testzipfscanonical backslashes-1 X:/foo\\\\bar [file join [zipfs root] foo bar] -constraints win + testzipfscanonical zipfspath //zipfs:/x/y [file join [zipfs root] x y] + testzipfscanonical zipfspath {MT //zipfs:/x/y} [file join [zipfs root] mt x y] # # Read/uncompress -- cgit v0.12 From 8c3b029c0757d433637c1911a74cd55c2766e3e9 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 14 Oct 2023 17:07:20 +0000 Subject: Eliminate CanonicalPath - obsolete --- generic/tclZipfs.c | 213 +++++------------------------------------------------ 1 file changed, 17 insertions(+), 196 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index a7c96c4..d840224 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -1107,7 +1107,7 @@ MapPathToZipfs(Tcl_Interp *interp, const char **partsPtr; Tcl_SplitPath(path, &numParts, &partsPtr); Tcl_DStringFree(dsPtr); - partsPtr[0] = ZIPFS_VOLUME; + partsPtr[0] = mountPath; (void)Tcl_JoinPath(numParts, partsPtr, dsPtr); ckfree(partsPtr); } @@ -1128,181 +1128,6 @@ MapPathToZipfs(Tcl_Interp *interp, Tcl_DecrRefCount(normalizedObj); return Tcl_DStringValue(dsPtr); } - -/* - *------------------------------------------------------------------------- - * - * CanonicalPath -- - * - * This function computes the canonical path from a directory and file - * name components into the specified Tcl_DString. - * - * Results: - * Returns the pointer to the canonical path contained in the specified - * Tcl_DString. - * - * Side effects: - * Modifies the specified Tcl_DString. - * - *------------------------------------------------------------------------- - */ - -static char * -CanonicalPath( - const char *root, - const char *tail, - Tcl_DString *dsPtr, - int inZipfs) -{ - char *path; - int i, j, c, isUNC = 0, isVfs = 0, n = 0; - int haveZipfsPath = 1; - -#ifdef _WIN32 - if (tail[0] != '\0' && strchr(drvletters, tail[0]) && tail[1] == ':') { - tail += 2; - haveZipfsPath = 0; - } - /* UNC style path */ - if (tail[0] == '\\') { - root = ""; - ++tail; - haveZipfsPath = 0; - } - if (tail[0] == '\\') { - root = "/"; - ++tail; - haveZipfsPath = 0; - } -#endif /* _WIN32 */ - - if (haveZipfsPath) { - /* UNC style path */ - if (root && strncmp(root, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) == 0) { - isVfs = 1; - } else if (tail && - strncmp(tail, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) == 0) { - isVfs = 2; - } - if (isVfs != 1 && (root[0] == '/') && (root[1] == '/')) { - isUNC = 1; - } - } - - if (isVfs != 2) { - if (tail[0] == '/') { - if (isVfs != 1) { - root = ""; - } - ++tail; - isUNC = 0; - } - if (tail[0] == '/') { - if (isVfs != 1) { - root = "/"; - } - ++tail; - isUNC = 1; - } - } - i = strlen(root); - j = strlen(tail); - - switch (isVfs) { - case 1: - if (i > ZIPFS_VOLUME_LEN) { - Tcl_DStringSetLength(dsPtr, i + j + 1); - path = Tcl_DStringValue(dsPtr); - memcpy(path, root, i); - path[i++] = '/'; - memcpy(path + i, tail, j); - } else { - Tcl_DStringSetLength(dsPtr, i + j); - path = Tcl_DStringValue(dsPtr); - memcpy(path, root, i); - memcpy(path + i, tail, j); - } - break; - case 2: - Tcl_DStringSetLength(dsPtr, j); - path = Tcl_DStringValue(dsPtr); - memcpy(path, tail, j); - break; - default: - if (inZipfs) { - /* pathLen = zipfs vol len + root len + separator + tail len */ - Tcl_DStringInit(dsPtr); - (void) Tcl_DStringAppend(dsPtr, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN); - if (i) { - (void) Tcl_DStringAppend(dsPtr, root, i); - if (root[i-1] != '/') { - Tcl_DStringAppend(dsPtr, "/", 1); - } - } - path = Tcl_DStringAppend(dsPtr, tail, j); - } else { - Tcl_DStringSetLength(dsPtr, i + j + 1); - path = Tcl_DStringValue(dsPtr); - memcpy(path, root, i); - path[i++] = '/'; - memcpy(path + i, tail, j); - } - break; - } - -#ifdef _WIN32 - for (i = 0; path[i] != '\0'; i++) { - if (path[i] == '\\') { - path[i] = '/'; - } - } -#endif /* _WIN32 */ - - if (inZipfs) { - n = ZIPFS_VOLUME_LEN; - } else { - n = 0; - } - - for (i = j = n; (c = path[i]) != '\0'; i++) { - if (c == '/') { - int c2 = path[i + 1]; - - if (c2 == '\0' || c2 == '/') { - continue; - } - if (c2 == '.') { - int c3 = path[i + 2]; - - if ((c3 == '/') || (c3 == '\0')) { - i++; - continue; - } - if ((c3 == '.') - && ((path[i + 3] == '/') || (path[i + 3] == '\0'))) { - i += 2; - while ((j > 0) && (path[j - 1] != '/')) { - j--; - } - if (j > isUNC) { - --j; - while ((j > 1 + isUNC) && (path[j - 2] == '/')) { - j--; - } - } - continue; - } - } - } - path[j++] = c; - } - if (j == 0) { - path[j++] = '/'; - } - path[j] = 0; - Tcl_DStringSetLength(dsPtr, j); - return Tcl_DStringValue(dsPtr); -} /* *------------------------------------------------------------------------- @@ -4139,34 +3964,30 @@ ZipFSCanonicalObjCmd( int objc, /* Number of arguments. */ Tcl_Obj *const objv[]) /* Argument objects. */ { - char *mntpoint = NULL; - char *filename = NULL; - char *result; - Tcl_DString dPath; + char *mntPoint = NULL; + Tcl_DString dsPath, dsMount; - if (objc < 2 || objc > 4) { - Tcl_WrongNumArgs(interp, 1, objv, "?mountpoint? filename ?inZipfs?"); + if (objc < 2 || objc > 3) { + Tcl_WrongNumArgs(interp, 1, objv, "?mountpoint? filename"); return TCL_ERROR; } - Tcl_DStringInit(&dPath); + + Tcl_DStringInit(&dsPath); + Tcl_DStringInit(&dsMount); + if (objc == 2) { - filename = Tcl_GetString(objv[1]); - result = CanonicalPath("", filename, &dPath, 1); - } else if (objc == 3) { - mntpoint = Tcl_GetString(objv[1]); - filename = Tcl_GetString(objv[2]); - result = CanonicalPath(mntpoint, filename, &dPath, 1); + mntPoint = ZIPFS_VOLUME; } else { - int zipfs = 0; - - if (Tcl_GetBooleanFromObj(interp, objv[3], &zipfs)) { + if (NormalizeMountPoint(interp, Tcl_GetString(objv[1]), &dsMount) != TCL_OK) { return TCL_ERROR; } - mntpoint = Tcl_GetString(objv[1]); - filename = Tcl_GetString(objv[2]); - result = CanonicalPath(mntpoint, filename, &dPath, zipfs); + mntPoint = Tcl_DStringValue(&dsMount); } - Tcl_SetObjResult(interp, Tcl_NewStringObj(result, -1)); + (void)MapPathToZipfs(interp, + mntPoint, + Tcl_GetString(objv[objc - 1]), + &dsPath); + Tcl_SetObjResult(interp, Tcl_DStringToObj(&dsPath)); return TCL_OK; } -- cgit v0.12 From 3ce94d7a92879b852d784527197413482fb4573b Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Sat, 14 Oct 2023 21:25:42 +0000 Subject: Missing !endif (in previous commit) --- win/rules.vc | 1 + 1 file changed, 1 insertion(+) diff --git a/win/rules.vc b/win/rules.vc index 32a5024..fc816ac 100644 --- a/win/rules.vc +++ b/win/rules.vc @@ -881,6 +881,7 @@ USE_THREAD_ALLOC= 0 !message *** Force 64-bit time_t _USE_64BIT_TIME_T = 1 !endif +!endif # Yes, it's weird that the "symbols" option controls DEBUG and # the "pdbs" option controls SYMBOLS. That's historical. -- cgit v0.12 From 8c2f47321dea1fe813bb8ca865d2cff1a9b08236 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sun, 15 Oct 2023 06:32:31 +0000 Subject: More fixes, update docs --- doc/zipfs.3 | 47 ++++++++++++++++++------- doc/zipfs.n | 6 ++++ generic/tclZipfs.c | 87 ++++++++++++++++++++++++++++----------------- tests/zipfs.test | 101 ++++++++++++++++++++++++++++++++--------------------- 4 files changed, 157 insertions(+), 84 deletions(-) diff --git a/doc/zipfs.3 b/doc/zipfs.3 index 77a6a57..af1281c 100644 --- a/doc/zipfs.3 +++ b/doc/zipfs.3 @@ -91,20 +91,43 @@ The function \fImay\fR modify the variables pointed to by \fIargcPtr\fR and \fIargvPtr\fR to remove arguments; the current implementation does not do so, but callers \fIshould not\fR assume that this will be true in the future. .PP -\fBTclZipfs_Mount\fR mounts the ZIP archive \fIzipname\fR on the mount point -given in \fImountpoint\fR using the optional ZIP password \fIpassword\fR. -Errors during that process are reported in the interpreter \fIinterp\fR. If -\fImountpoint\fR is a NULL pointer, information on all currently mounted ZIP -file systems is written into \fIinterp\fR's result as a sequence of mount -points and ZIP file names. The result of this call is a standard Tcl result +\fBTclZipfs_Mount\fR is used to mount ZIP archives and to retrieve information +about currently mounted archives. If \fImountpoint\fR and \fIzipname\fR are both +specified (i.e. non-NULL), the function mounts the ZIP archive \fIzipname\fR on +the mount point given in \fImountpoint\fR. If \fIpassword\fR is not NULL, it +should point to the NUL terminated password protecting the archive. If not under +the zipfs file system root, \fImountpoint\fR is normalized with respect to it. +For example, a mount point passed as either \fBmt\fR \fB/mt\fR would be +normalized to \fB//zipfs:/mt\fR. An error is raised if the mount point includes +a drive or UNC volume. On success, \fIinterp\fR's result is set to the +normalized mount point path. +.PP +If \fImountpoint\fR is a NULL pointer, information on all currently mounted ZIP +file systems is stored in \fIinterp\fR's result as a sequence of mount +points and ZIP file names. +.PP +If \fImountpoint\fR is not NULL but \fIzipfile\fR +is NULL, the path to the archive mounted at that mount point is stored +as \fIinterp\fR's result. The function returns a standard Tcl result code. .PP -\fBTclZipfs_MountBuffer\fR mounts the ZIP archive in the buffer pointed to by -\fIdata\fR on the mount point given in \fImountpoint\fR. The ZIP archive is -assumed to be not password protected. Errors during that process are reported -in the interpreter \fIinterp\fR. The \fIcopy\fR argument determines whether -the buffer is internally copied before mounting or not. The result of this -call is a standard Tcl result code. +\fBTclZipfs_MountBuffer\fR is similar to \fBTclZipfs_Mount\fR except that the +content of a ZIP archive is passed in the buffer pointed to by \fIdata\fR. +If \fImountpoint\fR and +\fIdata\fR are both non-NULL, the function +mounts the ZIP archive content \fIdata\fR on the mount point +given in \fImountpoint\fR. +The +\fIcopy\fR argument determines whether the buffer is internally copied before +mounting or not. The ZIP archive is assumed to be not password protected. +On success, \fIinterp\fR's result is set to the normalized mount point +path. +If \fImountpoint\fR is a NULL pointer, information on all currently mounted ZIP +file systems is stored in \fIinterp\fR's result as a sequence of mount +points and ZIP file names. If \fImountpoint\fR is not NULL but \fIdata\fR +is NULL, the path to the archive mounted at that mount point is stored +as \fIinterp\fR's result. The function returns a standard Tcl result +code. .PP \fBTclZipfs_Unmount\fR undoes the effect of \fBTclZipfs_Mount\fR, i.e., it unmounts the mounted ZIP file system that was mounted from \fIzipname\fR (at diff --git a/doc/zipfs.n b/doc/zipfs.n index fa361ef..a730497 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -123,6 +123,12 @@ filesystem at \fImountpoint\fR. After this command executes, files contained in \fIzipfile\fR will appear to Tcl to be regular files at the mount point. If \fImountpoint\fR is specified as an empty string, it is defaulted to the \fB[zipfs root]\fR. +The command returns the normalized mount point path. +.PP +If not under the zipfs file system root, \fImountpoint\fR is normalized with +respect to it. For example, a mount point passed as either \fBmt\fR \fB/mt\fR +would be normalized to \fB//zipfs:/mt\fR. An error is raised if the mount point +includes a drive or UNC volume. .PP \fBNB:\fR because the current working directory is a concept maintained by the operating system, using \fBcd\fR into a mounted archive will only work in the diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index d840224..a074db1 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -1011,7 +1011,6 @@ NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPt * Several things need to happen here * - Absolute paths containing volumes (drive letter or UNC) raise error * except of course if the volume is zipfs root - * - \ need to be converted to / * - \ -> / and // -> / conversions (except if UNC which is error) * - . and .. have to be dealt with * The first is explicitly checked, the others are dealt with a @@ -1029,7 +1028,7 @@ NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPt unnormalizedObj = Tcl_DStringToObj(dsPtr); } else { if (joinedPath[0] != '/' || joinedPath[1] == '/') { - /* D:/x, D:x or //unc */ + /* mount path was D:/x, D:x or //unc */ goto invalidMountPath; } unnormalizedObj = Tcl_ObjPrintf(ZIPFS_VOLUME "%s", joinedPath + 1); @@ -1099,10 +1098,16 @@ MapPathToZipfs(Tcl_Interp *interp, joiner[0] = mountPath; joiner[1] = path; +#ifndef _WIN32 + /* On Unix C:/foo/bat is not treated as absolute by JoinPath so check ourself */ + if (path[0] && path[1] == ':') { + joiner[1] += 2; + } +#endif joinedPath = Tcl_JoinPath(2, joiner, dsPtr); if (strncmp(ZIPFS_VOLUME, joinedPath, ZIPFS_VOLUME_LEN)) { - /* path was not relative. Strip off the volume */ + /* path was not relative. Strip off the volume (e.g. UNC) */ Tcl_Size numParts; const char **partsPtr; Tcl_SplitPath(path, &numParts, &partsPtr); @@ -2347,26 +2352,52 @@ TclZipfs_Mount( ret = DescribeMounted(interp, mountPoint); Unlock(); } else { + /* Have both a mount point and a file (name) to mount there. */ + + Tcl_Obj *zipPathObj; + Tcl_Obj *normZipPathObj; + Unlock(); - /* Have both a mount point and a file (name) to mount there. */ - if (passwd == NULL || - (ret = IsPasswordValid(interp, passwd, strlen(passwd))) == TCL_OK) { - zf = AllocateZipFile(interp, strlen(mountPoint)); - if (zf == NULL) { - ret = TCL_ERROR; - } else { - ret = ZipFSOpenArchive(interp, zipname, 1, zf); - if (ret != TCL_OK) { - ckfree(zf); - } else { - ret = ZipFSCatalogFilesystem( - interp, zf, mountPoint, passwd, zipname); - /* Note zf is already freed on error! */ + zipPathObj = Tcl_NewStringObj(zipname, -1); + Tcl_IncrRefCount(zipPathObj); + normZipPathObj = Tcl_FSGetNormalizedPath(interp, zipPathObj); + if (normZipPathObj == NULL) { + Tcl_SetObjResult( + interp, + Tcl_ObjPrintf("could not normalize zip filename \"%s\"", zipname)); + Tcl_SetErrorCode(interp, "TCL", "OPERATION", "NORMALIZE", NULL); + ret = TCL_ERROR; + } else { + Tcl_IncrRefCount(normZipPathObj); + const char *normPath = Tcl_GetString(normZipPathObj); + if (passwd == NULL || + (ret = IsPasswordValid(interp, passwd, strlen(passwd))) == + TCL_OK) { + zf = AllocateZipFile(interp, strlen(mountPoint)); + if (zf == NULL) { + ret = TCL_ERROR; + } + else { + ret = ZipFSOpenArchive(interp, normPath, 1, zf); + if (ret != TCL_OK) { + ckfree(zf); + } + else { + ret = ZipFSCatalogFilesystem( + interp, zf, mountPoint, passwd, normPath); + /* Note zf is already freed on error! */ + } } } + Tcl_DecrRefCount(normZipPathObj); + if (ret == TCL_OK && interp) { + Tcl_DStringResult(interp, &ds); + } } + Tcl_DecrRefCount(zipPathObj); } + Tcl_DStringFree(&ds); return ret; } @@ -2377,7 +2408,7 @@ TclZipfs_Mount( * TclZipfs_MountBuffer -- * * This procedure is invoked to mount a given ZIP archive file on a given - * mountpoint with optional ZIP password. + * mountpoint. * * Results: * A standard Tcl result. @@ -2472,6 +2503,9 @@ TclZipfs_MountBuffer( ret = ZipFSCatalogFilesystem( interp, zf, mountPoint, NULL, "Memory Buffer"); } + if (ret == TCL_OK && interp) { + Tcl_DStringResult(interp, &ds); + } } done: @@ -2581,7 +2615,6 @@ ZipFSMountObjCmd( Tcl_Obj *const objv[]) /* Argument objects. */ { const char *mountPoint = NULL, *zipFile = NULL, *password = NULL; - Tcl_Obj *zipFileObj = NULL; int result; if (objc > 4) { @@ -2598,16 +2631,7 @@ ZipFSMountObjCmd( mountPoint = Tcl_GetString(objv[1]); } else { /* 2 < objc < 4 */ - zipFileObj = Tcl_FSGetNormalizedPath(interp, objv[1]); - if (!zipFileObj) { - Tcl_SetObjResult( - interp, - Tcl_NewStringObj("could not normalize zip filename", -1)); - Tcl_SetErrorCode(interp, "TCL", "OPERATION", "NORMALIZE", NULL); - return TCL_ERROR; - } - Tcl_IncrRefCount(zipFileObj); - zipFile = Tcl_GetString(zipFileObj); + zipFile = Tcl_GetString(objv[1]); mountPoint = Tcl_GetString(objv[2]); if (objc > 3) { password = Tcl_GetString(objv[3]); @@ -2616,9 +2640,6 @@ ZipFSMountObjCmd( } result = TclZipfs_Mount(interp, zipFile, mountPoint, password); - if (zipFileObj != NULL) { - Tcl_DecrRefCount(zipFileObj); - } return result; } @@ -3964,7 +3985,7 @@ ZipFSCanonicalObjCmd( int objc, /* Number of arguments. */ Tcl_Obj *const objv[]) /* Argument objects. */ { - char *mntPoint = NULL; + const char *mntPoint = NULL; Tcl_DString dsPath, dsMount; if (objc < 2 || objc > 3) { diff --git a/tests/zipfs.test b/tests/zipfs.test index 88bee93..025d4c1 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -390,7 +390,7 @@ namespace eval test_ns_zipfs { # Wrapper to ease transition if Tcl changes order of argument to zipfs mount # or the zipfs prefix proc mount [list zippath [list mountpoint $defMountPt]] { - zipfs mount $zippath $mountpoint + return [zipfs mount $zippath $mountpoint] } # Make full path to zip file @@ -464,23 +464,21 @@ namespace eval test_ns_zipfs { proc testmount {id zippath checkPath mountpoint args} { set zippath [zippath $zippath] test zipfs-mount-$id "zipfs mount $id" -body { - mount $zippath $mountpoint - set canon [zipfs canonical $mountpoint] + set canon [mount $zippath $mountpoint] list [file exists [file join $canon $checkPath]] \ - [mounttarget $canon] + [zipfs mount $canon] [zipfs mount $mountpoint] } -cleanup { zipfs unmount $mountpoint - } -result [list 1 $zippath] {*}$args + } -result [list 1 $zippath $zippath] {*}$args # Mount memory buffer test zipfs-mount_data-$id "zipfs mount_data $id" -body { - zipfs mount_data [readbin $zippath] $mountpoint - set canon [zipfs canonical $mountpoint] + set canon [zipfs mount_data [readbin $zippath] $mountpoint] list [file exists [file join $canon $checkPath]] \ - [mounttarget $canon] + [zipfs mount $canon] [zipfs mount $mountpoint] } -cleanup { cleanup - } -result [list 1 {Memory Buffer}] {*}$args + } -result [list 1 {Memory Buffer} {Memory Buffer}] {*}$args } @@ -498,12 +496,28 @@ namespace eval test_ns_zipfs { testbadmount bad-file-count-high incons-file-count-high.zip "truncated directory" testbadmount bad-file-count-low incons-file-count-low.zip "short file count" + test zipfs-mount-on-drive "Mount point include drive" -body { + zipfs mount [zippath test.zip] C:/foo + } -result {Invalid mount path "C:/foo"} -returnCodes error -constraints win + test zipfs-mount_data-on-drive "Mount point include drive" -body { + zipfs mount_data [readbin [zippath test.zip]] C:/foo + } -result {Invalid mount path "C:/foo"} -returnCodes error -constraints win + test zipfs-mount-on-unc "Mount point is unc" -body { + zipfs mount [zippath test.zip] //unc/share/foo + } -result {Invalid mount path "//unc/share/foo"} -returnCodes error + test zipfs-mount_data-on-unc "Mount point include unc" -body { + zipfs mount_data [readbin [zippath test.zip]] //unc/share/foo + } -result {Invalid mount path "//unc/share/foo"} -returnCodes error + + # Good mounts testmount basic test.zip testdir/test2 $defMountPt testmount basic-on-default test.zip testdir/test2 "" testmount basic-on-root test.zip testdir/test2 [zipfs root] testmount basic-on-slash test.zip testdir/test2 / + testmount basic-on-bslash test.zip testdir/test2 \\ -constraints win testmount basic-on-relative test.zip testdir/test2 testmount testmount basic-on-absolute test.zip testdir/test2 /testmount + testmount basic-on-absolute-bslash test.zip testdir/test2 \\testmount -constraints win testmount zip-at-end junk-at-start.zip testdir/test2 $defMountPt testmount zip-at-start junk-at-end.zip testdir/test2 $defMountPt testmount zip-in-zip [file join [zipfs root] test2 test.zip] testdir/test2 $defMountPt -setup { @@ -672,6 +686,18 @@ namespace eval test_ns_zipfs { } -result {{} {test2 test3} test2-overlay} # + # paths inside a zip + # TODO - paths encoded in utf-8 vs fallback encoding + test zipfs-content-paths-1 "Test absolute and full paths" -setup { + mount [zippath test-paths.zip] + } -cleanup { + cleanup + } -body { + # Primarily verifies that drive letters are stripped and paths maintained + lsort [zipfs list] + } -result {//zipfs:/testmount //zipfs:/testmount/filename.txt //zipfs:/testmount/src //zipfs:/testmount/src/tcltk //zipfs:/testmount/src/tcltk/wip //zipfs:/testmount/src/tcltk/wip/tcl //zipfs:/testmount/src/tcltk/wip/tcl/tests //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles/abspath.txt //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles/fullpath.txt} + + # # zipfs list testnumargs "zipfs list" "" "?(-glob|-regexp)? ?pattern?" @@ -899,44 +925,41 @@ namespace eval test_ns_zipfs { } -result {path "//zipfs:/testmt/a" not found in any zipfs volume} -returnCodes error # - # zipfs canonical - - # TODO - semantics are very unclear. Can produce nonsensical paths like - # //zipfs:/n/zipfs:/m/test. Minimal sanity tests for now. + # zipfs canonical test zipfs-canonical-minargs {zipfs canonical min args} -body { zipfs canonical - } -returnCodes error -result {wrong # args: should be "zipfs canonical ?mountpoint? filename ?inZipfs?"} + } -returnCodes error -result {wrong # args: should be "zipfs canonical ?mountpoint? filename"} test zipfs-canonical-maxargs {zipfs canonical max args} -body { - zipfs canonical a b c d - } -returnCodes error -result {wrong # args: should be "zipfs canonical ?mountpoint? filename ?inZipfs?"} + zipfs canonical a b c + } -returnCodes error -result {wrong # args: should be "zipfs canonical ?mountpoint? filename"} proc testzipfscanonical {id cmdargs result args} { test zipfs-canonical-$id "zipfs canonical $id" \ -body [list zipfs canonical {*}$cmdargs] \ -result $result {*}$args } - testzipfscanonical default-relative PATH [file join [zipfs root] PATH] - testzipfscanonical default-absolute /PATH [file join [zipfs root] PATH] - testzipfscanonical root-relative-1 [list [zipfs root] PATH] [file join [zipfs root] PATH] - testzipfscanonical root-relative-2 [list / PATH] [file join [zipfs root] PATH] - testzipfscanonical root-absolute-1 [list [zipfs root] /PATH] [file join [zipfs root] PATH] - testzipfscanonical root-absolute-2 [list / /PATH] [file join [zipfs root] PATH] - testzipfscanonical absolute-relative {/MT PATH} [file join [zipfs root] MT PATH] - testzipfscanonical absolute-absolute {/MT /PATH} [file join [zipfs root] PATH] - testzipfscanonical relative-relative {MT PATH} [file join [zipfs root] MT PATH] - testzipfscanonical relative-absolute {MT /PATH} [file join [zipfs root] PATH] - testzipfscanonical mountpoint-trailslash-relative {MT/ PATH} [file join [zipfs root] MT PATH] - testzipfscanonical mountpoint-trailslash-absolute {MT/ /PATH} [file join [zipfs root] PATH] - testzipfscanonical mountpoint-root-relative [list [zipfs root] PATH] [file join [zipfs root] PATH] - testzipfscanonical mountpoint-root-absolute [list [zipfs root] /PATH] [file join [zipfs root] PATH] - testzipfscanonical mountpoint-empty-relative {{} PATH} [file join [zipfs root] PATH] - - testzipfscanonical driveletter X: [zipfs root] -constraints win - testzipfscanonical drivepath X:/foo/bar [file join [zipfs root] foo bar] -constraints win - testzipfscanonical drivepath {MT X:/foo/bar} [file join [zipfs root] MT foo bar] -constraints win - # (backslashes need additional escaping passed to testzipfscanonical) - testzipfscanonical backslashes X:\\\\foo\\\\bar [file join [zipfs root] foo bar] -constraints win - testzipfscanonical backslashes-1 X:/foo\\\\bar [file join [zipfs root] foo bar] -constraints win - testzipfscanonical zipfspath //zipfs:/x/y [file join [zipfs root] x y] - testzipfscanonical zipfspath {MT //zipfs:/x/y} [file join [zipfs root] mt x y] + testzipfscanonical default-relative [list a] [file join [zipfs root] a] + testzipfscanonical default-absolute [list /a] [file join [zipfs root] a] + testzipfscanonical root-relative-1 [list [zipfs root] a] [file join [zipfs root] a] + testzipfscanonical root-relative-2 [list / a] [file join [zipfs root] a] + testzipfscanonical root-absolute-1 [list [zipfs root] /a] [file join [zipfs root] a] + testzipfscanonical root-absolute-2 [list / /a] [file join [zipfs root] a] + testzipfscanonical absolute-relative [list /MT a] [file join [zipfs root] MT a] + testzipfscanonical absolute-absolute [list /MT /a] [file join [zipfs root] MT a] + testzipfscanonical relative-relative [list MT a] [file join [zipfs root] MT a] + testzipfscanonical relative-absolute [list MT /a] [file join [zipfs root] MT a] + testzipfscanonical mountpoint-trailslash-relative [list MT/ a] [file join [zipfs root] MT a] + testzipfscanonical mountpoint-trailslash-absolute [list MT/ /a] [file join [zipfs root] MT a] + testzipfscanonical mountpoint-root-relative [list [zipfs root] a] [file join [zipfs root] a] + testzipfscanonical mountpoint-root-absolute [list [zipfs root] /a] [file join [zipfs root] a] + testzipfscanonical mountpoint-empty-relative [list {} a] [file join [zipfs root] a] + + testzipfscanonical driveletter [list X:] [zipfs root] -constraints win + testzipfscanonical drivepath [list X:/foo/bar] [file join [zipfs root] foo bar] -constraints win + testzipfscanonical drivepath [list MT X:/foo/bar] [file join [zipfs root] MT foo bar] -constraints win + testzipfscanonical backslashes [list X:\\\\foo\\\\bar] [file join [zipfs root] foo bar] -constraints win + testzipfscanonical backslashes-1 [list X:/foo\\\\bar] [file join [zipfs root] foo bar] -constraints win + testzipfscanonical zipfspath [list //zipfs:/x/y] [file join [zipfs root] x y] + testzipfscanonical zipfspath [list MT //zipfs:/x/y] [file join [zipfs root] x y] # # Read/uncompress -- cgit v0.12 From fead0fac7d5a920009e8dd9cc9a3a9992151c9a0 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sun, 15 Oct 2023 10:12:55 +0000 Subject: Fix zipfs long path memory leaks [9525f4c8bc] --- generic/tclZipfs.c | 37 +++++++++++++++++++++---------------- tests/zipfs.test | 12 ++++++++++-- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index a074db1..4e38b09 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -889,7 +889,7 @@ static char * DecodeZipEntryText( const unsigned char *inputBytes, unsigned int inputLength, - Tcl_DString *dstPtr) + Tcl_DString *dstPtr) /* Must have been initialized by caller! */ { Tcl_Encoding encoding; const char *src; @@ -897,7 +897,6 @@ DecodeZipEntryText( int dstLen, srcLen = inputLength, flags; Tcl_EncodingState state; - Tcl_DStringInit(dstPtr); if (inputLength < 1) { return Tcl_DStringValue(dstPtr); } @@ -996,7 +995,9 @@ DecodeZipEntryText( *------------------------------------------------------------------------ */ static int -NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPtr) +NormalizeMountPoint(Tcl_Interp *interp, + const char *mountPath, + Tcl_DString *dsPtr) /* Must be initialized by caller! */ { const char *joiner[2]; char *joinedPath; @@ -1004,8 +1005,7 @@ NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPt Tcl_Obj *normalizedObj; const char *normalizedPath; Tcl_Size normalizedLen; - - Tcl_DStringInit(dsPtr); + Tcl_DString dsJoin; /* * Several things need to happen here @@ -1020,12 +1020,13 @@ NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPt joiner[0] = ZIPFS_VOLUME; joiner[1] = mountPath; - joinedPath = Tcl_JoinPath(2, joiner, dsPtr); + Tcl_DStringInit(&dsJoin); + joinedPath = Tcl_JoinPath(2, joiner, &dsJoin); /* Now joinedPath has all \ -> / and // -> / (except UNC) converted. */ if (!strncmp(ZIPFS_VOLUME, joinedPath, ZIPFS_VOLUME_LEN)) { - unnormalizedObj = Tcl_DStringToObj(dsPtr); + unnormalizedObj = Tcl_DStringToObj(&dsJoin); } else { if (joinedPath[0] != '/' || joinedPath[1] == '/') { /* mount path was D:/x, D:x or //unc */ @@ -1044,7 +1045,7 @@ NormalizeMountPoint(Tcl_Interp *interp, const char *mountPath, Tcl_DString *dsPt /* normalizedObj owned by Tcl!! Do NOT DecrRef without an IncrRef */ normalizedPath = Tcl_GetStringFromObj(normalizedObj, &normalizedLen); - Tcl_DStringFree(dsPtr); /* Reset */ + Tcl_DStringFree(&dsJoin); Tcl_DStringAppend(dsPtr, normalizedPath, normalizedLen); Tcl_DecrRefCount(normalizedObj); return TCL_OK; @@ -1057,7 +1058,7 @@ invalidMountPath: } errorReturn: - Tcl_DStringFree(dsPtr); + Tcl_DStringFree(&dsJoin); return TCL_ERROR; } @@ -1084,7 +1085,8 @@ static char * MapPathToZipfs(Tcl_Interp *interp, const char *mountPath, /* Must be fully normalized */ const char *path, /* Archive content path to map */ - Tcl_DString *dsPtr) /* Must be cleared on success return */ + Tcl_DString *dsPtr) /* Must be initialized and cleared + by caller */ { const char *joiner[2]; char *joinedPath; @@ -1092,9 +1094,9 @@ MapPathToZipfs(Tcl_Interp *interp, Tcl_Obj *normalizedObj; const char *normalizedPath; Tcl_Size normalizedLen; + Tcl_DString dsJoin; assert(TclIsZipfsPath(mountPath)); - Tcl_DStringInit(dsPtr); joiner[0] = mountPath; joiner[1] = path; @@ -1104,19 +1106,20 @@ MapPathToZipfs(Tcl_Interp *interp, joiner[1] += 2; } #endif - joinedPath = Tcl_JoinPath(2, joiner, dsPtr); + Tcl_DStringInit(&dsJoin); + joinedPath = Tcl_JoinPath(2, joiner, &dsJoin); if (strncmp(ZIPFS_VOLUME, joinedPath, ZIPFS_VOLUME_LEN)) { /* path was not relative. Strip off the volume (e.g. UNC) */ Tcl_Size numParts; const char **partsPtr; Tcl_SplitPath(path, &numParts, &partsPtr); - Tcl_DStringFree(dsPtr); + Tcl_DStringFree(&dsJoin); partsPtr[0] = mountPath; - (void)Tcl_JoinPath(numParts, partsPtr, dsPtr); + (void)Tcl_JoinPath(numParts, partsPtr, &dsJoin); ckfree(partsPtr); } - unnormalizedObj = Tcl_DStringToObj(dsPtr); /* Also resets dsPtr */ + unnormalizedObj = Tcl_DStringToObj(&dsJoin); /* Also resets dsJoin */ Tcl_IncrRefCount(unnormalizedObj); normalizedObj = Tcl_FSGetNormalizedPath(interp, unnormalizedObj); if (normalizedObj == NULL) { @@ -1127,7 +1130,6 @@ MapPathToZipfs(Tcl_Interp *interp, Tcl_DecrRefCount(unnormalizedObj); /* normalizedObj owned by Tcl!! Do NOT DecrRef without an IncrRef */ - Tcl_DStringFree(dsPtr); /* Reset */ normalizedPath = Tcl_GetStringFromObj(normalizedObj, &normalizedLen); Tcl_DStringAppend(dsPtr, normalizedPath, normalizedLen); Tcl_DecrRefCount(normalizedObj); @@ -1971,6 +1973,7 @@ ZipFSCatalogFilesystem( pathlen = ZipReadShort(start, end, q + ZIP_CENTRAL_PATHLEN_OFFS); comlen = ZipReadShort(start, end, q + ZIP_CENTRAL_FCOMMENTLEN_OFFS); extra = ZipReadShort(start, end, q + ZIP_CENTRAL_EXTRALEN_OFFS); + Tcl_DStringSetLength(&ds, 0); path = DecodeZipEntryText(q + ZIP_CENTRAL_HEADER_LEN, pathlen, &ds); if ((pathlen > 0) && (path[pathlen - 1] == '/')) { Tcl_DStringSetLength(&ds, pathlen - 1); @@ -2336,6 +2339,7 @@ TclZipfs_Mount( } Tcl_DString ds; + Tcl_DStringInit(&ds); ret = NormalizeMountPoint(interp, mountPoint, &ds); if (ret != TCL_OK) { Unlock(); @@ -2448,6 +2452,7 @@ TclZipfs_MountBuffer( } Tcl_DString ds; + Tcl_DStringInit(&ds); ret = NormalizeMountPoint(interp, mountPoint, &ds); if (ret != TCL_OK) { Unlock(); diff --git a/tests/zipfs.test b/tests/zipfs.test index 025d4c1..a7a6a1d 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -694,8 +694,8 @@ namespace eval test_ns_zipfs { cleanup } -body { # Primarily verifies that drive letters are stripped and paths maintained - lsort [zipfs list] - } -result {//zipfs:/testmount //zipfs:/testmount/filename.txt //zipfs:/testmount/src //zipfs:/testmount/src/tcltk //zipfs:/testmount/src/tcltk/wip //zipfs:/testmount/src/tcltk/wip/tcl //zipfs:/testmount/src/tcltk/wip/tcl/tests //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles/abspath.txt //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles/fullpath.txt} + lsort [zipfs find $defMountPt] + } -result {//zipfs:/testmount/filename.txt //zipfs:/testmount/src //zipfs:/testmount/src/tcltk //zipfs:/testmount/src/tcltk/wip //zipfs:/testmount/src/tcltk/wip/tcl //zipfs:/testmount/src/tcltk/wip/tcl/tests //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles/abspath.txt //zipfs:/testmount/src/tcltk/wip/tcl/tests/zipfiles/fullpath.txt} # # zipfs list @@ -1917,6 +1917,14 @@ namespace eval test_ns_zipfs { read $fd close $fd } -result "" + + # Following will only show a leak with valgrind + test bug-9525f4c8bc "Memory leak with long mount paths" -body { + set mt //zipfs:[string repeat /x 240] + zipfs mount [zippath test.zip] $mt + zipfs unmount $mt + } -result "" + } -- cgit v0.12