ant-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Roman Horváth <roman.horv...@truni.sk>
Subject Knowledge sharing related to Apache Ant, mainly ZipFile class…
Date Tue, 16 Jun 2020 14:06:47 GMT
I wish you a good day,

this mail is mainly related to the Apache Ant ZipFile class. This mail shares some knowledge
and makes some small recommendations for the ZipFile class and one recommendation for the
ZipOutputStream class. (I separated them by few —.)

———

The first sentence within the file comments sounds:

	Replacement for java.util.ZipFile.

But in fact, it should sound:

	Replacement for java.util.zip.ZipFile.

(Similarly, a few lines later.)

———

In the constructor ZipFile(final File f, final String encoding, final boolean useUnicodeExtraFields)
a tiny performance improvement may be made. In the block finally, the lines:

			closed = !success;
			if (!success) {

may sound:

			closed = !success;
			if (closed) {

It takes the same result. (The second “not” operation is not needed.)

———

I noticed that the ZipFile class got a new method – getName(). This method does not have
a description. It may be (e.g.) simply (I guessed the version – this was the version when
I noticed it):

	/**
	 * Gets the archive name.
	 * 
	 * @return current archive name.
	 * @since 1.10.7
	 */

Yet the list item in the beginning: “There is no getName method,” is no longer true.

———

I suggest adding a new feature to this class – the overall archive comment. (Because the
class ZipOutputStream has the method setComment.)

It would take those few actions:

1. Add new attribute (a String comment) between existing ones, e.g. between the String archiveName
and the RandomAccessFile archive:

	/**
	 * File name of actual source.
	 */
	private final String archiveName;

	/**
	 * File (the overall archive) comment.
	 */
	private String comment = null;

	/**
	 * The actual data source.
	 */
	private final RandomAccessFile archive;

2. Add the method getComment, e.g. after the method getName:

	public String getName() {
		return archiveName;
	}

	/**
	 * Returns the file comment, or null (if there is no file comment
	 * or the file comment has been ignored intentionally).
	 * @see ZipOutputStream#setComment(String)
	 */
	public String getComment() {
		return comment;
	}

3. Add new flag EOCD_SIG, e.g. after the flag CFH_SIG:

	private static final long CFH_SIG =
		ZipLong.getValue(ZipOutputStream.CFH_SIG); // 0x02014B50L

	private static final long EOCD_SIG = 0x06054B50L;

4. Add the following code to the positionAtCentralDirectory method:

	private void positionAtCentralDirectory() throws IOException
	{
		positionAtEndOfCentralDirectoryRecord(); // Existing line


		// Read the file comment
		long archivePos = archive.getFilePointer();
		try
		{
			if (archivePos + 22 <= archive.length())
			{
				// Read the comment length only:
				archive.skipBytes(20);
				archive.readFully(SHORT_BUF);
				int commentLen = ZipShort.getValue(SHORT_BUF);

				if (0 != commentLen)
				{
					// Cropping:
					if (archive.getFilePointer() + commentLen >=
						archive.length()) commentLen = (int)(archive.length()
							- archive.getFilePointer());

					// Reading and decoding the file comment:
					final byte[] comment = new byte[commentLen];
					archive.readFully(comment);
					this.comment = zipEncoding.decode(comment);
				}
			}
		}
		catch (Throwable t)
		{
			// Swallow, if anything goes wrong, the comment just stays as is.
		}
		finally
		{
			// Return to the last active position:
			archive.seek(archivePos);
		}


		boolean found = false; // Existing line


It took me a few days. I took the inspiration from several public sources. The code should
work properly, but I recommend taking several tests (using test zip files in different conditions,
which I don’t have) to prove it flawless. (I did it several weeks ago, and I did some basic
testing, but to be sure I recommend making more tests.)

———

In the class ZipOutputStream, in the method setLevel(int level); a minor refactoring can be
made. On the last few lines of code:

		if (this.level == level) {
			return;
		}
		hasCompressionLevelChanged = true;
		this.level = level;

the return statement will become pointless after rewriting the code in a more effective way
(negating the condition):

		if (this.level != level) {
			hasCompressionLevelChanged = true;
			this.level = level;
		}

(According to https://stackoverflow.com/a/23354212/2036423 this shouldn’t be any problem.)

———

Best regards,
Roman Horváth


---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@ant.apache.org
For additional commands, e-mail: user-help@ant.apache.org


Mime
View raw message