Author: sebb Date: Thu Apr 2 18:45:02 2009 New Revision: 761372 URL: http://svn.apache.org/viewvc?rev=761372&view=rev Log: Rename TarUtils.getXXX methods as formatXXX Update Javadoc Throw IllegalArgumentException if value won't fit in buffer Treat long values as unsigned Use String instead of StringBuffer for names etc
Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java?rev=761372&r1=761371&r2=761372&view=diff ============================================================================== --- commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java (original) +++ commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java Thu Apr 2 18:45:02 2009 @@ -82,7 +82,7 @@ public class TarArchiveEntry implements TarConstants, ArchiveEntry { /** The entry's name. */ - private StringBuffer name; + private String name; /** The entry's permission mode. */ private int mode; @@ -103,16 +103,16 @@ private byte linkFlag; /** The entry's link name. */ - private StringBuffer linkName; + private String linkName; /** The entry's magic tag. */ - private StringBuffer magic; + private String magic; /** The entry's user name. */ - private StringBuffer userName; + private String userName; /** The entry's group name. */ - private StringBuffer groupName; + private String groupName; /** The entry's major device number. */ private int devMajor; @@ -139,9 +139,9 @@ * Construct an empty entry and prepares the header values. */ private TarArchiveEntry () { - this.magic = new StringBuffer(MAGIC_POSIX); - this.name = new StringBuffer(); - this.linkName = new StringBuffer(); + this.magic = MAGIC_POSIX; + this.name = ""; + this.linkName = ""; String user = System.getProperty("user.name", ""); @@ -151,8 +151,8 @@ this.userId = 0; this.groupId = 0; - this.userName = new StringBuffer(user); - this.groupName = new StringBuffer(""); + this.userName = user; + this.groupName = ""; this.file = null; } @@ -170,16 +170,16 @@ this.devMajor = 0; this.devMinor = 0; - this.name = new StringBuffer(name); + this.name = name; this.mode = isDir ? DEFAULT_DIR_MODE : DEFAULT_FILE_MODE; this.linkFlag = isDir ? LF_DIR : LF_NORMAL; this.userId = 0; this.groupId = 0; this.size = 0; this.modTime = (new Date()).getTime() / MILLIS_PER_SECOND; - this.linkName = new StringBuffer(""); - this.userName = new StringBuffer(""); - this.groupName = new StringBuffer(""); + this.linkName = ""; + this.userName = ""; + this.groupName = ""; this.devMajor = 0; this.devMinor = 0; @@ -219,22 +219,24 @@ this.file = file; - this.linkName = new StringBuffer(""); - this.name = new StringBuffer(fileName); + this.linkName = ""; if (file.isDirectory()) { this.mode = DEFAULT_DIR_MODE; this.linkFlag = LF_DIR; - int nameLength = name.length(); + int nameLength = fileName.length(); if (nameLength == 0 || name.charAt(nameLength - 1) != '/') { - this.name.append("/"); + this.name = fileName + "/"; + } else { + this.name = fileName; } this.size = 0; } else { this.mode = DEFAULT_FILE_MODE; this.linkFlag = LF_NORMAL; this.size = file.length(); + this.name = fileName; } this.modTime = file.lastModified() / MILLIS_PER_SECOND; @@ -314,7 +316,7 @@ * @param name This entry's new name. */ public void setName(String name) { - this.name = new StringBuffer(normalizeFileName(name)); + this.name = normalizeFileName(name); } /** @@ -386,7 +388,7 @@ * @param userName This entry's new user name. */ public void setUserName(String userName) { - this.userName = new StringBuffer(userName); + this.userName = userName; } /** @@ -404,7 +406,7 @@ * @param groupName This entry's new group name. */ public void setGroupName(String groupName) { - this.groupName = new StringBuffer(groupName); + this.groupName = groupName; } /** @@ -559,12 +561,12 @@ public void writeEntryHeader(byte[] outbuf) { int offset = 0; - offset = TarUtils.getNameBytes(name, outbuf, offset, NAMELEN); - offset = TarUtils.getOctalBytes(mode, outbuf, offset, MODELEN); - offset = TarUtils.getOctalBytes(userId, outbuf, offset, UIDLEN); - offset = TarUtils.getOctalBytes(groupId, outbuf, offset, GIDLEN); - offset = TarUtils.getLongOctalBytes(size, outbuf, offset, SIZELEN); - offset = TarUtils.getLongOctalBytes(modTime, outbuf, offset, MODTIMELEN); + offset = TarUtils.formatNameBytes(name, outbuf, offset, NAMELEN); + offset = TarUtils.formatOctalBytes(mode, outbuf, offset, MODELEN); + offset = TarUtils.formatOctalBytes(userId, outbuf, offset, UIDLEN); + offset = TarUtils.formatOctalBytes(groupId, outbuf, offset, GIDLEN); + offset = TarUtils.formatLongOctalBytes(size, outbuf, offset, SIZELEN); + offset = TarUtils.formatLongOctalBytes(modTime, outbuf, offset, MODTIMELEN); int csOffset = offset; @@ -573,12 +575,12 @@ } outbuf[offset++] = linkFlag; - offset = TarUtils.getNameBytes(linkName, outbuf, offset, NAMELEN); - offset = TarUtils.getNameBytes(magic, outbuf, offset, MAGICLEN); - offset = TarUtils.getNameBytes(userName, outbuf, offset, UNAMELEN); - offset = TarUtils.getNameBytes(groupName, outbuf, offset, GNAMELEN); - offset = TarUtils.getOctalBytes(devMajor, outbuf, offset, DEVLEN); - offset = TarUtils.getOctalBytes(devMinor, outbuf, offset, DEVLEN); + offset = TarUtils.formatNameBytes(linkName, outbuf, offset, NAMELEN); + offset = TarUtils.formatNameBytes(magic, outbuf, offset, MAGICLEN); + offset = TarUtils.formatNameBytes(userName, outbuf, offset, UNAMELEN); + offset = TarUtils.formatNameBytes(groupName, outbuf, offset, GNAMELEN); + offset = TarUtils.formatOctalBytes(devMajor, outbuf, offset, DEVLEN); + offset = TarUtils.formatOctalBytes(devMinor, outbuf, offset, DEVLEN); while (offset < outbuf.length) { outbuf[offset++] = 0; @@ -586,7 +588,7 @@ long chk = TarUtils.computeCheckSum(outbuf); - TarUtils.getCheckSumOctalBytes(chk, outbuf, csOffset, CHKSUMLEN); + TarUtils.formatCheckSumOctalBytes(chk, outbuf, csOffset, CHKSUMLEN); } /** Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java?rev=761372&r1=761371&r2=761372&view=diff ============================================================================== --- commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java (original) +++ commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java Thu Apr 2 18:45:02 2009 @@ -41,30 +41,35 @@ * @param length The maximum number of bytes to parse. * @return The long value of the octal string. */ - public static long parseOctal(byte[] buffer, int offset, int length) { + public static long parseOctal(byte[] buffer, final int offset, final int length) { long result = 0; boolean stillPadding = true; int end = offset + length; for (int i = offset; i < end; ++i) { - if (buffer[i] == 0) { // Found trailing null + final byte currentByte = buffer[i]; + if (currentByte == 0) { // Found trailing null break; } // Ignore leading spaces ('0' can be ignored anyway) - if (buffer[i] == (byte) ' ' || buffer[i] == '0') { + if (currentByte == (byte) ' ' || currentByte == '0') { if (stillPadding) { continue; } - if (buffer[i] == (byte) ' ') { // Found trailing space + if (currentByte == (byte) ' ') { // Found trailing space break; } } stillPadding = false; // CheckStyle:MagicNumber OFF - result = (result << 3) + (buffer[i] - '0');// TODO needs to reject invalid bytes + if (currentByte < '0' || currentByte > '7'){ + throw new IllegalArgumentException( + "Invalid octal digit at position "+i+" in '"+new String(buffer, offset, length)+"'"); + } + result = (result << 3) + (currentByte - '0');// TODO needs to reject invalid bytes // CheckStyle:MagicNumber ON } @@ -81,7 +86,7 @@ * @param length The maximum number of bytes to parse. * @return The entry name. */ - public static StringBuffer parseName(byte[] buffer, int offset, int length) { + public static String parseName(byte[] buffer, final int offset, final int length) { StringBuffer result = new StringBuffer(length); int end = offset + length; @@ -93,7 +98,7 @@ result.append((char) buffer[i]); } - return result; + return result.toString(); } /** @@ -111,7 +116,7 @@ * @param length The maximum number of header bytes to copy. * @return The updated offset, i.e. offset + length */ - public static int getNameBytes(StringBuffer name, byte[] buf, int offset, int length) { + public static int formatNameBytes(String name, byte[] buf, final int offset, final int length) { int i; // copy until end of input or output is reached. @@ -128,51 +133,54 @@ } /** - * Fill buffer with octal number, with leading zeroes + * Fill buffer with unsigned octal number, padded with leading zeroes. * - * The output for negative numbers is not specified, - * but currently the method returns a buffer filled with zeros. - * This may change. - * - * @param value number to convert to octal (assumed >=0) + * @param value number to convert to octal - treated as unsigned * @param buffer destination buffer * @param offset starting offset in buffer * @param length length of buffer to fill + * @throws IllegalArgumentException if the value will not fit in the buffer */ - public static void formatUnsignedOctalString(long value, byte[] buffer, - int offset, int length) { - length--; + public static void formatUnsignedOctalString(final long value, byte[] buffer, + final int offset, final int length) { + int remaining = length; + remaining--; if (value == 0) { - buffer[offset + length--] = (byte) '0'; + buffer[offset + remaining--] = (byte) '0'; } else { - for (long val = value; length >= 0 && val > 0; --length) { + long val = value; + for (; remaining >= 0 && val != 0; --remaining) { // CheckStyle:MagicNumber OFF - buffer[offset + length] = (byte) ((byte) '0' + (byte) (val & 7)); - val = val >> 3; + buffer[offset + remaining] = (byte) ((byte) '0' + (byte) (val & 7)); + val = val >>> 3; // CheckStyle:MagicNumber ON } + if (val != 0){ + throw new IllegalArgumentException + (value+"="+Long.toOctalString(value)+ " will not fit in octal number buffer of length "+length); + } } - for (; length >= 0; --length) { // leading zeros - buffer[offset + length] = (byte) '0'; + for (; remaining >= 0; --remaining) { // leading zeros + buffer[offset + remaining] = (byte) '0'; } } /** * Write an octal integer into a buffer. * - * Adds a trailing space and NUL to end of the buffer. - * [Appears to be standard for V7 Unix BSD] - * Converts the long value (assumed positive) to the buffer. - * Adds leading zeros to the buffer. + * Uses {...@link #formatUnsignedOctalString} to format + * the value as an octal string with leading zeros. + * The converted number is followed by space and NUL * * @param value The value to write * @param buf The buffer to receive the output * @param offset The starting offset into the buffer * @param length The size of the output buffer * @return The updated offset, i.e offset+length + * @throws IllegalArgumentException if the value (and trailer) will not fit in the buffer */ - public static int getOctalBytes(long value, byte[] buf, int offset, int length) { + public static int formatOctalBytes(final long value, byte[] buf, final int offset, final int length) { int idx=length-2; // For space and trailing null formatUnsignedOctalString(value, buf, offset, idx); @@ -185,17 +193,19 @@ /** * Write an octal long integer into a buffer. - * Converts the long value (assumed positive) to the buffer. - * Adds leading zeros to the buffer. - * The buffer is terminated with a space. + * + * Uses {...@link #formatUnsignedOctalString} to format + * the value as an octal string with leading zeros. + * The converted number is followed by a space. * * @param value The value to write as octal * @param buf The destinationbuffer. * @param offset The starting offset into the buffer. * @param length The length of the buffer * @return The updated offset + * @throws IllegalArgumentException if the value (and trailer) will not fit in the buffer */ - public static int getLongOctalBytes(long value, byte[] buf, int offset, int length) { + public static int formatLongOctalBytes(final long value, byte[] buf, final int offset, final int length) { int idx=length-1; // For space @@ -207,18 +217,19 @@ /** * Writes an octal value into a buffer. - * - * Converts the long value (assumed positive) to the buffer. - * Adds leading zeros to the buffer. - * Checksum is followed by NUL and then space. + * + * Uses {...@link #formatUnsignedOctalString} to format + * the value as an octal string with leading zeros. + * The converted number is followed by NUL and then space. * * @param value The value to convert * @param buf The destination buffer * @param offset The starting offset into the buffer. * @param length The size of the buffer. * @return The updated value of offset, i.e. offset+length + * @throws IllegalArgumentException if the value (and trailer) will not fit in the buffer */ - public static int getCheckSumOctalBytes(long value, byte[] buf, int offset, int length) { + public static int formatCheckSumOctalBytes(final long value, byte[] buf, final int offset, final int length) { int idx=length-2; // for NUL and space formatUnsignedOctalString(value, buf, offset, idx); @@ -235,7 +246,7 @@ * @param buf The tar entry's header buffer. * @return The computed checksum. */ - public static long computeCheckSum(byte[] buf) { + public static long computeCheckSum(final byte[] buf) { long sum = 0; for (int i = 0; i < buf.length; ++i) { Modified: commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java?rev=761372&r1=761371&r2=761372&view=diff ============================================================================== --- commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java (original) +++ commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java Thu Apr 2 18:45:02 2009 @@ -25,18 +25,18 @@ public void testName(){ byte [] buff = new byte[20]; - StringBuffer sb1 = new StringBuffer("abcdefghijklmnopqrstuvwxyz"); - int off = TarUtils.getNameBytes(sb1, buff, 1, buff.length-1); + String sb1 = "abcdefghijklmnopqrstuvwxyz"; + int off = TarUtils.formatNameBytes(sb1, buff, 1, buff.length-1); assertEquals(off, 20); - StringBuffer sb2 = TarUtils.parseName(buff, 1, 10); - assertEquals(sb2.toString(),sb1.substring(0,10)); + String sb2 = TarUtils.parseName(buff, 1, 10); + assertEquals(sb2,sb1.substring(0,10)); sb2 = TarUtils.parseName(buff, 1, 19); - assertEquals(sb2.toString(),sb1.substring(0,19)); + assertEquals(sb2,sb1.substring(0,19)); buff = new byte[30]; - off = TarUtils.getNameBytes(sb1, buff, 1, buff.length-1); + off = TarUtils.formatNameBytes(sb1, buff, 1, buff.length-1); assertEquals(off, 30); sb2 = TarUtils.parseName(buff, 1, buff.length-1); - assertEquals(sb1.toString(), sb2.toString()); + assertEquals(sb1, sb2); } private void fillBuff(byte []buffer, String input){ @@ -61,14 +61,17 @@ value = TarUtils.parseOctal(buffer,0, 11); assertEquals(077777777777L, value); fillBuff(buffer, "abcdef"); // Invalid input - value = TarUtils.parseOctal(buffer,0, 11); -// assertEquals(0, value); // Or perhaps an Exception? + try { + value = TarUtils.parseOctal(buffer,0, 11); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + } } private void checkRoundTripOctal(final long value) { byte [] buffer = new byte[12]; long parseValue; - TarUtils.getLongOctalBytes(value, buffer, 0, buffer.length); + TarUtils.formatLongOctalBytes(value, buffer, 0, buffer.length); parseValue = TarUtils.parseOctal(buffer,0, buffer.length); assertEquals(value,parseValue); } @@ -84,24 +87,33 @@ // Check correct trailing bytes are generated public void testTrailers() { byte [] buffer = new byte[12]; - TarUtils.getLongOctalBytes(123, buffer, 0, buffer.length); + TarUtils.formatLongOctalBytes(123, buffer, 0, buffer.length); assertEquals(' ', buffer[buffer.length-1]); assertEquals('3', buffer[buffer.length-2]); // end of number - TarUtils.getOctalBytes(123, buffer, 0, buffer.length); + TarUtils.formatOctalBytes(123, buffer, 0, buffer.length); assertEquals(0 , buffer[buffer.length-1]); assertEquals(' ', buffer[buffer.length-2]); assertEquals('3', buffer[buffer.length-3]); // end of number - TarUtils.getCheckSumOctalBytes(123, buffer, 0, buffer.length); + TarUtils.formatCheckSumOctalBytes(123, buffer, 0, buffer.length); assertEquals(' ', buffer[buffer.length-1]); assertEquals(0 , buffer[buffer.length-2]); assertEquals('3', buffer[buffer.length-3]); // end of number } public void testNegative() { - byte [] buffer = new byte[10]; + byte [] buffer = new byte[22]; TarUtils.formatUnsignedOctalString(-1, buffer, 0, buffer.length); - // Currently negative numbers generate all zero buffer. This may need to change. - assertEquals("0000000000", new String(buffer)); - + assertEquals("1777777777777777777777", new String(buffer)); + } + + public void testOverflow() { + byte [] buffer = new byte[8-1]; // a lot of the numbers have 8-byte buffers (nul term) + TarUtils.formatUnsignedOctalString(07777777L, buffer, 0, buffer.length); + assertEquals("7777777", new String(buffer)); + try { + TarUtils.formatUnsignedOctalString(017777777L, buffer, 0, buffer.length); + fail("Should have cause IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + } } }