Fix [00018ec7a0], [f91ee30d33], [23dd83ce7c] - zipfs r+, w+ modes, zero byte allocation

3 files changed, 138 insertions, 87 deletions

diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c
index 0e14784..213ae23 100644
--- a/generic/tclZipfs.c
+++ b/generic/tclZipfs.c
@@ -1551,8 +1551,10 @@ ZipFSOpenArchive(
 	    ZIPFS_POSIX_ERROR(interp, "seek error");
 	    goto error;
 	}
-	if ((zf->length - ZIP_CENTRAL_END_LEN)
-		> (64 * 1024 * 1024 - ZIP_CENTRAL_END_LEN)) {
+        /* What's the magic about 64 * 1024 * 1024 ? */
+	if ((zf->length <= ZIP_CENTRAL_END_LEN) ||
+	    (zf->length - ZIP_CENTRAL_END_LEN) >
+		(64 * 1024 * 1024 - ZIP_CENTRAL_END_LEN)) {
 	    ZIPFS_ERROR(interp, "illegal file size");
 	    ZIPFS_ERROR_CODE(interp, "FILE_SIZE");
 	    goto error;
@@ -2265,7 +2267,15 @@ TclZipfs_MountBuffer(
 
     /*
      * 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;
+    }
 
     zf = AllocateZipFile(interp, strlen(mountPoint));
     if (!zf) {
@@ -4205,26 +4215,38 @@ ZipChannelClose(
 	memset(info->keys, 0, sizeof(info->keys));
     }
     if (info->isWriting) {
+	/*
+	 * Copy channel data back into original file in archive.
+	 * TODO - there seems to be no locking here to protect access from
+	 * multiple threads. The channel (info) may be thread specific (?)
+	 * but the ZipEntry is not afaict
+	 */
 	ZipEntry *z = info->zipEntryPtr;
 	assert(info->ubufToFree && info->ubuf);
-	unsigned char *newdata =
-	    (unsigned char *)attemptckrealloc(info->ubufToFree, info->numRead);
-
-	if (newdata) {
-	    info->ubufToFree = NULL;/* Now newdata! */
-	    info->ubuf = NULL;
-	    if (z->data) {
-		ckfree(z->data);
-	    }
-	    z->data = newdata;
-	    z->numBytes = z->numCompressedBytes = info->numBytes;
-	    z->compressMethod = ZIP_COMPMETH_STORED;
-	    z->timestamp = time(NULL);
-	    z->isDirectory = 0;
-	    z->isEncrypted = 0;
-	    z->offset = 0;
-	    z->crc32 = 0;
+	unsigned char *newdata;
+	newdata = (unsigned char *)attemptckrealloc(
+	    info->ubufToFree,
+	    info->numBytes ? info->numBytes : 1); /* Bug [23dd83ce7c] */
+	if (newdata == NULL) {
+	    /* Could not reallocate, keep existing buffer */
+	    newdata = info->ubufToFree;
 	}
+	info->ubufToFree = NULL; /* Now newdata! */
+	info->ubuf = NULL;
+
+	/* Replace old content */
+	if (z->data) {
+	    ckfree(z->data);
+	}
+	z->data = newdata; /* May be NULL */
+	z->numBytes = z->numCompressedBytes = info->numBytes;
+	assert(z->data || z->numBytes == 0);
+	z->compressMethod = ZIP_COMPMETH_STORED;
+	z->timestamp = time(NULL);
+	z->isDirectory = 0;
+	z->isEncrypted = 0;
+	z->offset = 0;
+	z->crc32 = 0;
     }
     WriteLock();
     info->zipFilePtr->numOpen--;
@@ -4513,14 +4535,27 @@ static Tcl_Channel
 ZipChannelOpen(
     Tcl_Interp *interp,		/* Current interpreter. */
     char *filename,		/* What are we opening. */
-    int wr,			/* True if we're opening in write mode. */
-    int trunc)			/* True if we're opening in truncate mode. */
+    int mode)			/* O_WRONLY O_RDWR O_TRUNC flags */
 {
     ZipEntry *z;
     ZipChannel *info;
     int flags = 0;
     char cname[128];
 
+    int wr = (mode & (O_WRONLY | O_RDWR)) != 0;
+
+    /* Check for unsupported modes. */
+
+    if ((mode & O_APPEND) || ((ZipFS.wrmax <= 0) && wr)) {
+	Tcl_SetErrno(EACCES);
+	if (interp) {
+	    Tcl_SetObjResult(interp, Tcl_ObjPrintf(
+		    "%s not supported: %s",
+		    mode & O_APPEND ? "append mode" : "write access",
+		    Tcl_PosixError(interp)));
+	}
+	return NULL;
+    }
     /*
      * Is the file there?
      */
@@ -4564,19 +4599,33 @@ ZipChannelOpen(
 	ZIPFS_ERROR_CODE(interp, "COMP_METHOD");
 	goto error;
     }
-    if (!trunc) {
-	flags |= TCL_READABLE;
-	if (z->isEncrypted && (z->zipFilePtr->passBuf[0] == 0)) {
-	    ZIPFS_ERROR(interp, "decryption failed");
-	    ZIPFS_ERROR_CODE(interp, "DECRYPT");
-	    goto error;
-	} else if (wr && !z->data && (z->numBytes > ZipFS.wrmax)) {
-	    ZIPFS_ERROR(interp, "file too large");
-	    ZIPFS_ERROR_CODE(interp, "FILE_SIZE");
+    if (wr) {
+	if ((mode & O_TRUNC) == 0 && !z->data && (z->numBytes > ZipFS.wrmax)) {
+	    Tcl_SetErrno(EFBIG);
+	    ZIPFS_POSIX_ERROR(interp, "file size exceeds max writable");
 	    goto error;
 	}
-    } else {
 	flags = TCL_WRITABLE;
+	if (mode & O_RDWR)
+	    flags |= TCL_READABLE;
+    } else {
+	/* Read-only */
+	flags |= TCL_READABLE;
+    }
+    if (flags & TCL_READABLE) {
+	if (z->isEncrypted) {
+	    if (z->numCompressedBytes < 12) {
+		ZIPFS_ERROR(interp, "decryption failed: truncated decryption header");
+		ZIPFS_ERROR_CODE(interp, "DECRYPT");
+		goto error;
+
+	    }
+	    if (z->zipFilePtr->passBuf[0] == 0) {
+		ZIPFS_ERROR(interp, "decryption failed - no password provided");
+		ZIPFS_ERROR_CODE(interp, "DECRYPT");
+		goto error;
+	    }
+	}
     }
 
     info = AllocateZipChannel(interp);
@@ -4586,31 +4635,23 @@ ZipChannelOpen(
     info->zipFilePtr = z->zipFilePtr;
     info->zipEntryPtr = z;
     if (wr) {
-	/*
-	 * Set up a writable channel.
-	 */
+	/* Set up a writable channel. */
 
-	flags |= TCL_WRITABLE;
-	if (InitWritableChannel(interp, info, z, trunc) == TCL_ERROR) {
+	if (InitWritableChannel(interp, info, z, mode & O_TRUNC) == TCL_ERROR) {
 	    ckfree(info);
 	    goto error;
 	}
     } else if (z->data) {
-	/*
-	 * Set up a readable channel for direct data.
-	 */
+	/* Set up a readable channel for direct data. */
 
-	flags |= TCL_READABLE;
 	info->numBytes = z->numBytes;
 	info->ubuf = z->data;
 	info->ubufToFree = NULL; /* Not dynamically allocated */
-    }
-    else {
+    } else {
 	/*
 	 * Set up a readable channel.
 	 */
 
-	flags |= TCL_READABLE;
 	if (InitReadableChannel(interp, info, z) == TCL_ERROR) {
 	    ckfree(info);
 	    goto error;
@@ -4689,11 +4730,13 @@ InitWritableChannel(
     info->isWriting = 1;
     info->maxWrite = ZipFS.wrmax;
 
-    info->ubufToFree = (unsigned char *) attemptckalloc(info->maxWrite);
+    info->ubufToFree =
+	(unsigned char *)attemptckalloc(info->maxWrite ? info->maxWrite : 1);
     info->ubuf = info->ubufToFree;
     if (!info->ubuf) {
 	goto memoryError;
     }
+    /* TODO - why is the memset necessary? Not cheap for default maxWrite. */
     memset(info->ubuf, 0, info->maxWrite);
 
     if (trunc) {
@@ -4749,8 +4792,11 @@ InitWritableChannel(
 	    if (z->isEncrypted) {
 		unsigned int j;
 
+		/* Min length 12 for keys should already been checked. */
+		assert(stream.avail_in >= 12);
+
 		stream.avail_in -= 12;
-		cbuf = (unsigned char *) attemptckalloc(stream.avail_in);
+		cbuf = (unsigned char *) attemptckalloc(stream.avail_in ? stream.avail_in : 1);
 		if (!cbuf) {
 		    goto memoryError;
 		}
@@ -4812,8 +4858,8 @@ InitWritableChannel(
     goto error_cleanup;
 
   tooBigError:
-    ZIPFS_ERROR(interp, "file size exceeds max writable");
-    ZIPFS_ERROR_CODE(interp, "TOOBIG");
+    Tcl_SetErrno(EFBIG);
+    ZIPFS_POSIX_ERROR(interp, "file size exceeds max writable");
     goto error_cleanup;
 
   corruptionError:
@@ -4906,8 +4952,9 @@ InitReadableChannel(
 	stream.opaque = Z_NULL;
 	stream.avail_in = z->numCompressedBytes;
 	if (info->isEncrypted) {
+	    assert(stream.avail_in >= 12);
 	    stream.avail_in -= 12;
-	    ubuf = (unsigned char *) attemptckalloc(stream.avail_in);
+	    ubuf = (unsigned char *) attemptckalloc(stream.avail_in ? stream.avail_in : 1);
 	    if (!ubuf) {
 		goto memoryError;
 	    }
@@ -4921,7 +4968,7 @@ InitReadableChannel(
 	    stream.next_in = info->ubuf;
 	}
 	info->ubufToFree = (unsigned char *)
-		attemptckalloc(info->numBytes);
+		attemptckalloc(info->numBytes ? info->numBytes : 1);
 	info->ubuf = info->ubufToFree;
 	stream.next_out = info->ubuf;
 	if (!info->ubuf) {
@@ -5117,27 +5164,9 @@ ZipFSOpenFileChannelProc(
 	return NULL;
     }
 
-    int trunc = (mode & O_TRUNC) != 0;
-    int wr = (mode & (O_WRONLY | O_RDWR)) != 0;
-
-    /*
-     * Check for unsupported modes.
-     */
-
-    if ((mode & O_APPEND) || ((ZipFS.wrmax <= 0) && wr)) {
-	Tcl_SetErrno(EACCES);
-	if (interp) {
-	    Tcl_SetObjResult(interp, Tcl_ObjPrintf(
-		    "%s not supported: %s",
-		    mode & O_APPEND ? "append mode" : "write access",
-		    Tcl_PosixError(interp)));
-	}
-	return NULL;
-    }
-
-    return ZipChannelOpen(interp, Tcl_GetString(pathPtr), wr, trunc);
+    return ZipChannelOpen(interp, Tcl_GetString(pathPtr), mode);
 }
-
+
 /*
  *-------------------------------------------------------------------------
  *
diff --git a/tests/zipfiles/empty.zip b/tests/zipfiles/empty.zip
new file mode 100644
index 0000000..0ed493a
--- /dev/null
+++ b/tests/zipfiles/empty.zip
diff --git a/tests/zipfs.test b/tests/zipfs.test
index cb45b05..705f26f 100644
--- a/tests/zipfs.test
+++ b/tests/zipfs.test
@@ -358,10 +358,10 @@ test zipfs-4.5 {zipfs lmkimg: making image from mounted} -constraints zipfs -set
 
 test zipfs-5.1 {zipfs mount_data: short data} -constraints zipfs -body {
     zipfs mount_data {} gorp
-} -returnCodes error -result {archive directory end signature not found}
+} -returnCodes error -result {illegal file size}
 test zipfs-5.2 {zipfs mount_data: short data} -constraints zipfs -body {
     zipfs mount_data gorpGORPgorp gorp
-} -returnCodes error -result {archive directory end signature not found}
+} -returnCodes error -result {illegal file size}
 test zipfs-5.3 {zipfs mount_data: short data} -constraints zipfs -body {
     set data PK\x03\x04.....................................
     append data PK\x01\x02.....................................
@@ -567,7 +567,7 @@ namespace eval test_ns_zipfs {
         lappend result [string equal $chans [lsort [chan names]]]
     } -cleanup {
         cleanup
-    } -result {1 {decryption failed} 1}
+    } -result {1 {decryption failed - no password provided} 1}
 
     test zipfs-mount-password-3 "mount - verify compressed cipher unreadable without password" -body {
         zipfs mount [zippath test-password.zip] $defaultMountPoint
@@ -578,7 +578,7 @@ namespace eval test_ns_zipfs {
         lappend result [string equal $chans [lsort [chan names]]]
     } -cleanup {
         cleanup
-    } -result {1 {decryption failed} 1}
+    } -result {1 {decryption failed - no password provided} 1}
 
     test zipfs-mount-nested-1 "mount - nested mount on non-existing path" -setup {
         mount [zippath test.zip]
@@ -966,6 +966,7 @@ namespace eval test_ns_zipfs {
     testzipfsread stored  test.zip          test           test
     testzipfsread stored  teststored.zip    aaaaaaaaaaaaaa
     testzipfsread deflate testdeflated2.zip aaaaaaaaaaaaaa
+    testzipfsread bug-23dd83ce7c  empty.zip    {} empty.txt
     # Test open modes - see bug [4645658689]
     testzipfsread stored-rw  teststored.zip    aaaaaaaaaaaaaa abac-repeat.txt r+
     testzipfsread deflate-rw testdeflated2.zip aaaaaaaaaaaaaa abac-repeat.txt r+
@@ -1031,17 +1032,18 @@ namespace eval test_ns_zipfs {
 
     }
     testzipfswrite create-w test.zip  "file not found \"//zipfs:/testmount/newfile\": no such file or directory" newfile w -returnCodes error
-    testzipfswrite create-wr test.zip "file not found \"//zipfs:/testmount/newfile\": no such file or directory" newfile w+ -returnCodes error
+    testzipfswrite create-w+ test.zip "file not found \"//zipfs:/testmount/newfile\": no such file or directory" newfile w+ -returnCodes error
     testzipfswrite create-a test.zip  "append mode not supported: permission denied" newfile a -returnCodes error
-    testzipfswrite create-ar test.zip "file not found \"//zipfs:/testmount/newfile\": no such file or directory" newfile a+ -returnCodes error
+    testzipfswrite create-a+ test.zip "file not found \"//zipfs:/testmount/newfile\": no such file or directory" newfile a+ -returnCodes error
     testzipfswrite store-w teststored.zip      "xyz" abac-repeat.txt w
     testzipfswrite deflate-w testdeflated2.zip "xyz" abac-repeat.txt w
-    testzipfswrite store-wr teststored.zip     "xyz" abac-repeat.txt w+
-    testzipfswrite deflate-wr testdeflated2.zip "xyz" abac-repeat.txt w+
+    testzipfswrite store-w+ teststored.zip     "xyz" abac-repeat.txt w+
+    testzipfswrite deflate-w+ testdeflated2.zip "xyz" abac-repeat.txt w+
     testzipfswrite stored-a teststored.zip     "append mode not supported: permission denied" abac-repeat.txt a -returnCodes error
     testzipfswrite deflate-a testdeflated2.zip "append mode not supported: permission denied" abac-repeat.txt a -returnCodes error
-    testzipfswrite store-ar teststored.zip     "aaaaaaaaaaaaaa\nbbbbbbbbbbbbbb\naaaaaaaaaaaaaa\ncccccccccccccc\nxyz" abac-repeat.txt a+
-    testzipfswrite deflate-ar testdeflated2.zip "aaaaaaaaaaaaaa\nbbbbbbbbbbbbbb\naaaaaaaaaaaaaa\ncccccccccccccc\nxyz" abac-repeat.txt a+
+    testzipfswrite store-a+ teststored.zip     "aaaaaaaaaaaaaa\nbbbbbbbbbbbbbb\naaaaaaaaaaaaaa\ncccccccccccccc\nxyz" abac-repeat.txt a+
+    testzipfswrite deflate-a+ testdeflated2.zip "aaaaaaaaaaaaaa\nbbbbbbbbbbbbbb\naaaaaaaaaaaaaa\ncccccccccccccc\nxyz" abac-repeat.txt a+
+    testzipfswrite bug-23dd83ce7c-w  empty.zip  "xyz"   empty.txt w
 
     test zipfs-write-unreadable "Reads not allowed on file opened for write" -setup {
         mount [zippath test.zip]
@@ -1135,14 +1137,18 @@ namespace eval test_ns_zipfs {
             append result [gets $fd],
             set pos [tell $fd]
             append result $pos,
-            puts -nonewline $fd "xyz"
+            puts -nonewline $fd "0123456789"
             append result [gets $fd],
             seek $fd $pos
             append result [gets $fd],
             seek $fd -6 end
+            append result [read $fd]|
+            close $fd
+            # Reopen after closing - bug [f91ee30d3]
+            set fd [open $path rb]
             append result [read $fd]
         }
-        test zipfs-readwrite-$id "zipfs read/seek/write $id" -setup {
+        test zipfs-rw-$id "zipfs read/seek/write $id" -setup {
             unset -nocomplain fd
             zipfs mount $zippath $defaultMountPoint
         } -cleanup {
@@ -1154,7 +1160,7 @@ namespace eval test_ns_zipfs {
         } -body $body -result $expected {*}$args
 
         set data [readbin $zippath]
-        test zipfs-readwrite-memory-$id "zipfs read/seek/write in-memory $id" -setup {
+        test zipfs-rw-memory-$id "zipfs read/seek/write in-memory $id" -setup {
             unset -nocomplain fd
             zipfs mount_data $data $defaultMountPoint
         } -cleanup {
@@ -1166,9 +1172,25 @@ namespace eval test_ns_zipfs {
         } -body $body -result $expected {*}$args
 
     }
-    testzipfsrw store-r+ teststored.zip      "aaaaaaaaaaaaaa,15,bbbbbbbbbbb,xyzbbbbbbbbbbb,ccccc\n" abac-repeat.txt r+
-    testzipfsrw store-w+ teststored.zip      ",0,,xyz,yz" abac-repeat.txt w+ -constraints bug-00018ec7a0
-    testzipfsrw store-a+ teststored.zip      ",60,,xyz,cc\nxyz" abac-repeat.txt a+
+    testzipfsrw store-r+ teststored.zip      "aaaaaaaaaaaaaa,15,bbbb,0123456789bbbb,ccccc\n|aaaaaaaaaaaaaa\n0123456789bbbb\naaaaaaaaaaaaaa\ncccccccccccccc\n" abac-repeat.txt r+
+    testzipfsrw store-w+ teststored.zip      ",0,,0123456789,456789|0123456789" abac-repeat.txt w+
+    testzipfsrw store-a+ teststored.zip      ",60,,0123456789,456789|aaaaaaaaaaaaaa\nbbbbbbbbbbbbbb\naaaaaaaaaaaaaa\ncccccccccccccc\n0123456789" abac-repeat.txt a+
+    testzipfsrw deflate-r+ testdeflated2.zip      "aaaaaaaaaaaaaa,15,bbbb,0123456789bbbb,ccccc\n|aaaaaaaaaaaaaa\n0123456789bbbb\naaaaaaaaaaaaaa\ncccccccccccccc\n" abac-repeat.txt r+
+    testzipfsrw deflate-w+ testdeflated2.zip      ",0,,0123456789,456789|0123456789" abac-repeat.txt w+
+    testzipfsrw deflate-a+ testdeflated2.zip      ",60,,0123456789,456789|aaaaaaaaaaaaaa\nbbbbbbbbbbbbbb\naaaaaaaaaaaaaa\ncccccccccccccc\n0123456789" abac-repeat.txt a+
+    test zipfs-rw-bug-f91ee30d33 "Bug f91ee30d33 - truncates at last read" -setup {
+        mount [zippath test.zip]
+    } -cleanup {
+        close $fd
+        cleanup
+    } -body {
+        set path [file join $defaultMountPoint test]
+        set fd [open $path r+]
+        puts -nonewline $fd X
+        close $fd
+        set fd [open $path r]
+        read $fd
+    } -result "Xest\n"
 
     #
     # Password protected
@@ -1199,10 +1221,10 @@ namespace eval test_ns_zipfs {
     testpassword plain-nopassword plain.txt "" plaintext
     testpassword plain-badpassword plain.txt xxx plaintext
     testpassword cipher cipher.bin password ciphertext -constraints bug-bbe7c6ff9e
-    testpassword cipher-nopassword cipher.bin {} "decryption failed" -returnCodes error
+    testpassword cipher-nopassword cipher.bin {} "decryption failed - no password provided" -returnCodes error
     testpassword cipher-badpassword cipher.bin xxx "invalid CRC" -returnCodes error
     testpassword cipher-deflate cipher-deflate.bin password [lseq 100] -constraints bug-bbe7c6ff9e
-    testpassword cipher-deflate-nopassword cipher-deflate.bin {} "decryption failed" -returnCodes error
+    testpassword cipher-deflate-nopassword cipher-deflate.bin {} "decryption failed - no password provided" -returnCodes error
     testpassword cipher-deflate-badpassword cipher-deflate.bin xxx "decompression error" -returnCodes error
 
     #