Skip to content

Commit

Permalink
Merge pull request #647 from drewnoakes/remove-trailing-nulls
Browse files Browse the repository at this point in the history
Trim trailing null bytes from some string values
  • Loading branch information
drewnoakes authored Jan 22, 2024
2 parents ea927bd + 88da99a commit eb47da1
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 45 deletions.
16 changes: 8 additions & 8 deletions Source/com/drew/imaging/png/PngMetadataReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private static void processChunk(@NotNull Metadata metadata, @NotNull PngChunk c
SequentialReader reader = new SequentialByteArrayReader(bytes);

// Profile Name is 1-79 bytes, followed by the 1 byte null character
byte[] profileNameBytes = reader.getNullTerminatedBytes(79 + 1);
byte[] profileNameBytes = reader.getNullTerminatedBytes(79 + 1, false);
PngDirectory directory = new PngDirectory(PngChunkType.iCCP);
directory.setStringValue(PngDirectory.TAG_ICC_PROFILE_NAME, new StringValue(profileNameBytes, _latin1Encoding));
byte compressionMethod = reader.getInt8();
Expand Down Expand Up @@ -197,13 +197,13 @@ private static void processChunk(@NotNull Metadata metadata, @NotNull PngChunk c
SequentialReader reader = new SequentialByteArrayReader(bytes);

// Keyword is 1-79 bytes, followed by the 1 byte null character
StringValue keywordsv = reader.getNullTerminatedStringValue(79 + 1, _latin1Encoding);
StringValue keywordsv = reader.getNullTerminatedStringValue(79 + 1, _latin1Encoding, false);
String keyword = keywordsv.toString();

// bytes left for text is:
// total bytes length - (Keyword length + null byte)
int bytesLeft = bytes.length - (keywordsv.getBytes().length + 1);
StringValue value = reader.getNullTerminatedStringValue(bytesLeft, _latin1Encoding);
StringValue value = reader.getNullTerminatedStringValue(bytesLeft, _latin1Encoding, false);
List<KeyValuePair> textPairs = new ArrayList<KeyValuePair>();
textPairs.add(new KeyValuePair(keyword, value));
PngDirectory directory = new PngDirectory(PngChunkType.tEXt);
Expand All @@ -213,7 +213,7 @@ private static void processChunk(@NotNull Metadata metadata, @NotNull PngChunk c
SequentialReader reader = new SequentialByteArrayReader(bytes);

// Keyword is 1-79 bytes, followed by the 1 byte null character
StringValue keywordsv = reader.getNullTerminatedStringValue(79 + 1, _latin1Encoding);
StringValue keywordsv = reader.getNullTerminatedStringValue(79 + 1, _latin1Encoding, false);
String keyword = keywordsv.toString();
byte compressionMethod = reader.getInt8();

Expand Down Expand Up @@ -250,20 +250,20 @@ private static void processChunk(@NotNull Metadata metadata, @NotNull PngChunk c
SequentialReader reader = new SequentialByteArrayReader(bytes);

// Keyword is 1-79 bytes, followed by the 1 byte null character
StringValue keywordsv = reader.getNullTerminatedStringValue(79 + 1, _utf8Encoding);
StringValue keywordsv = reader.getNullTerminatedStringValue(79 + 1, _utf8Encoding, false);
String keyword = keywordsv.toString();
byte compressionFlag = reader.getInt8();
byte compressionMethod = reader.getInt8();
// TODO we currently ignore languageTagBytes and translatedKeywordBytes
byte[] languageTagBytes = reader.getNullTerminatedBytes(bytes.length);
byte[] translatedKeywordBytes = reader.getNullTerminatedBytes(bytes.length);
byte[] languageTagBytes = reader.getNullTerminatedBytes(bytes.length, false);
byte[] translatedKeywordBytes = reader.getNullTerminatedBytes(bytes.length, false);

// bytes left for compressed text is:
// total bytes length - (Keyword length + null byte + comp flag byte + comp method byte + lang length + null byte + translated length + null byte)
int bytesLeft = bytes.length - (keywordsv.getBytes().length + 1 + 1 + 1 + languageTagBytes.length + 1 + translatedKeywordBytes.length + 1);
byte[] textBytes = null;
if (compressionFlag == 0) {
textBytes = reader.getNullTerminatedBytes(bytesLeft);
textBytes = reader.getNullTerminatedBytes(bytesLeft, false);
} else if (compressionFlag == 1) {
if (compressionMethod == 0) {
try {
Expand Down
47 changes: 30 additions & 17 deletions Source/com/drew/lang/SequentialReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -331,52 +331,65 @@ public StringValue getStringValue(int bytesRequested, @Nullable Charset charset)
/**
* Creates a String from the stream, ending where <code>byte=='\0'</code> or where <code>length==maxLength</code>.
*
* @param maxLengthBytes The maximum number of bytes to read. If a zero-byte is not reached within this limit,
* reading will stop and the string will be truncated to this length.
* @param maxLengthBytes The maximum number of bytes to read. If a zero-byte is not reached within this limit,
* reading will stop and the string will be truncated to this length.
* @param moveToMaxLength When true, the reader progresses maxLengthBytes on every call. When false, the reader
* stops when the first null is encountered (or the maximum is reached).
* @return The read string.
* @throws IOException The buffer does not contain enough bytes to satisfy this request.
*/
@NotNull
public String getNullTerminatedString(int maxLengthBytes, Charset charset) throws IOException
public String getNullTerminatedString(int maxLengthBytes, Charset charset, boolean moveToMaxLength) throws IOException
{
return getNullTerminatedStringValue(maxLengthBytes, charset).toString();
return getNullTerminatedStringValue(maxLengthBytes, charset, moveToMaxLength).toString();
}

/**
* Creates a String from the stream, ending where <code>byte=='\0'</code> or where <code>length==maxLength</code>.
*
* @param maxLengthBytes The maximum number of bytes to read. If a <code>\0</code> byte is not reached within this limit,
* reading will stop and the string will be truncated to this length.
* @param charset The <code>Charset</code> to register with the returned <code>StringValue</code>, or <code>null</code> if the encoding
* is unknown
* @param maxLengthBytes The maximum number of bytes to read. If a <code>\0</code> byte is not reached within this limit,
* reading will stop and the string will be truncated to this length.
* @param charset The <code>Charset</code> to register with the returned <code>StringValue</code>, or <code>null</code> if the encoding
* is unknown
* @param moveToMaxLength When true, the reader progresses maxLengthBytes on every call. When false, the reader
* stops when the first null is encountered (or the maximum is reached).
* @return The read string.
* @throws IOException The buffer does not contain enough bytes to satisfy this request.
*/
@NotNull
public StringValue getNullTerminatedStringValue(int maxLengthBytes, Charset charset) throws IOException
public StringValue getNullTerminatedStringValue(int maxLengthBytes, Charset charset, boolean moveToMaxLength) throws IOException
{
byte[] bytes = getNullTerminatedBytes(maxLengthBytes);
byte[] bytes = getNullTerminatedBytes(maxLengthBytes, moveToMaxLength);

return new StringValue(bytes, charset);
}

/**
* Returns the sequence of bytes punctuated by a <code>\0</code> value.
*
* @param maxLengthBytes The maximum number of bytes to read. If a <code>\0</code> byte is not reached within this limit,
* the returned array will be <code>maxLengthBytes</code> long.
* @param maxLengthBytes The maximum number of bytes to read. If a <code>\0</code> byte is not reached within this limit,
* the returned array will be <code>maxLengthBytes</code> long.
* @param moveToMaxLength When true, the reader progresses maxLengthBytes on every call. When false, the reader
* stops when the first null is encountered (or the maximum is reached).
* @return The read byte array, excluding the null terminator.
* @throws IOException The buffer does not contain enough bytes to satisfy this request.
*/
@NotNull
public byte[] getNullTerminatedBytes(int maxLengthBytes) throws IOException
public byte[] getNullTerminatedBytes(int maxLengthBytes, boolean moveToMaxLength) throws IOException
{
byte[] buffer = new byte[maxLengthBytes];

// Count the number of non-null bytes
int length = 0;
while (length < buffer.length && (buffer[length] = getByte()) != 0)
length++;
byte[] buffer;

if (moveToMaxLength) {
buffer = getBytes(maxLengthBytes);
while (length < buffer.length && buffer[length] != 0)
length++;
} else {
buffer = new byte[maxLengthBytes];
while (length < buffer.length && (buffer[length] = getByte()) != 0)
length++;
}

if (length == maxLengthBytes)
return buffer;
Expand Down
2 changes: 1 addition & 1 deletion Source/com/drew/metadata/bmp/BmpReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ protected void readBitmapHeader(@NotNull final SequentialReader reader, final @N
}
reader.skip(headerOffset + profileOffset - reader.getPosition());
if (csType == ColorSpaceType.PROFILE_LINKED.getValue()) {
directory.setString(BmpHeaderDirectory.TAG_LINKED_PROFILE, reader.getNullTerminatedString(profileSize, Charsets.WINDOWS_1252));
directory.setString(BmpHeaderDirectory.TAG_LINKED_PROFILE, reader.getNullTerminatedString(profileSize, Charsets.WINDOWS_1252, true));
} else {
ByteArrayReader randomAccessReader = new ByteArrayReader(reader.getBytes(profileSize));
new IccReader().extract(randomAccessReader, metadata, directory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ public static NikonPictureControl2Directory read(byte[] bytes) throws IOExceptio

NikonPictureControl2Directory directory = new NikonPictureControl2Directory();

directory.setString(TAG_PICTURE_CONTROL_VERSION, reader.getStringValue(4, Charsets.UTF_8).toString());
directory.setString(TAG_PICTURE_CONTROL_NAME, reader.getStringValue(20, Charsets.UTF_8).toString());
directory.setString(TAG_PICTURE_CONTROL_BASE, reader.getStringValue(20, Charsets.UTF_8).toString());
directory.setStringValue(TAG_PICTURE_CONTROL_VERSION, reader.getNullTerminatedStringValue(4, Charsets.UTF_8, true));
directory.setStringValue(TAG_PICTURE_CONTROL_NAME, reader.getNullTerminatedStringValue(20, Charsets.UTF_8, true));
directory.setStringValue(TAG_PICTURE_CONTROL_BASE, reader.getNullTerminatedStringValue(20, Charsets.UTF_8, true));

reader.skip(4);
directory.setObject(TAG_PICTURE_CONTROL_ADJUST, reader.getByte());
Expand Down
2 changes: 1 addition & 1 deletion Source/com/drew/metadata/heif/boxes/HandlerBox.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public HandlerBox(SequentialReader reader, Box box) throws IOException
reader.skip(4); // Pre-defined
handlerType = reader.getString(4);
reader.skip(12); // Reserved
name = reader.getNullTerminatedString((int)box.size - 32, Charsets.UTF_8);
name = reader.getNullTerminatedString((int)box.size - 32, Charsets.UTF_8, false);
}

public String getHandlerType()
Expand Down
12 changes: 6 additions & 6 deletions Source/com/drew/metadata/heif/boxes/ItemInfoBox.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ public ItemInfoEntry(SequentialReader reader, Box box) throws IOException
if ((version == 0) || (version == 1)) {
itemID = reader.getUInt16();
itemProtectionIndex = reader.getUInt16();
itemName = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8);
contentType = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8);
itemName = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8, false);
contentType = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8, false);
if (box.size - reader.getPosition() - headerLength > 0) {
extensionType = reader.getNullTerminatedString((int) (box.size - reader.getPosition() - headerLength), Charsets.UTF_8);
extensionType = reader.getNullTerminatedString((int) (box.size - reader.getPosition() - headerLength), Charsets.UTF_8, false);
}
}
if (version == 1) {
Expand All @@ -97,11 +97,11 @@ public ItemInfoEntry(SequentialReader reader, Box box) throws IOException
itemProtectionIndex = reader.getUInt16();
itemType = reader.getString(4);

itemName = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8);
itemName = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8, false);
if (itemType.equals("mime")) {
contentType = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8);
contentType = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8, false);
if (box.size - reader.getPosition() - headerLength > 0) {
contentEncoding = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8);
contentEncoding = reader.getNullTerminatedString((int)(box.size - reader.getPosition() - headerLength), Charsets.UTF_8, false);
}
} else if (itemType.equals("uri ")) {
itemUriType = reader.getString((int)(box.size - reader.getPosition() - headerLength));
Expand Down
2 changes: 1 addition & 1 deletion Source/com/drew/metadata/mp4/Mp4BoxHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public Mp4Handler<?> processBox(@NotNull String type, @Nullable byte[] payload,
reader.skip(4); // Pre-defined
String handlerType = reader.getString(4);
reader.skip(12); // Reserved
String name = reader.getNullTerminatedString((int)boxSize - 32, Charset.defaultCharset());
String name = reader.getNullTerminatedString((int)boxSize - 32, Charset.defaultCharset(), false);

final String HANDLER_SOUND_MEDIA = "soun";
final String HANDLER_VIDEO_MEDIA = "vide";
Expand Down
34 changes: 26 additions & 8 deletions Tests/com/drew/lang/SequentialAccessTestBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,13 +231,31 @@ public void testGetNullTerminatedString() throws IOException

// Test max length
for (int i = 0; i < bytes.length; i++) {
assertEquals("ABCDEFG".substring(0, i), createReader(bytes).getNullTerminatedString(i, Charsets.UTF_8));
assertEquals("ABCDEFG".substring(0, i), createReader(bytes).getNullTerminatedString(i, Charsets.UTF_8, false));
}

assertEquals("", createReader(new byte[]{0}).getNullTerminatedString(10, Charsets.UTF_8));
assertEquals("A", createReader(new byte[]{0x41, 0}).getNullTerminatedString(10, Charsets.UTF_8));
assertEquals("AB", createReader(new byte[]{0x41, 0x42, 0}).getNullTerminatedString(10, Charsets.UTF_8));
assertEquals("AB", createReader(new byte[]{0x41, 0x42, 0, 0x43}).getNullTerminatedString(10, Charsets.UTF_8));
assertEquals("", createReader(new byte[]{0}).getNullTerminatedString(10, Charsets.UTF_8, false));
assertEquals("A", createReader(new byte[]{0x41, 0}).getNullTerminatedString(10, Charsets.UTF_8, false));
assertEquals("AB", createReader(new byte[]{0x41, 0x42, 0}).getNullTerminatedString(10, Charsets.UTF_8, false));
assertEquals("AB", createReader(new byte[]{0x41, 0x42, 0, 0x43}).getNullTerminatedString(10, Charsets.UTF_8, false));
}

@Test
public void testGetNullTerminatedString_MoveToMaxLength() throws IOException
{
byte[] bytes = new byte[]{0x41, 0, 0, 0, 0};

SequentialReader reader = createReader(bytes);
assertEquals("A", reader.getNullTerminatedString(3, Charsets.UTF_8, true));
assertEquals(3, reader.getPosition());

reader = createReader(bytes);
assertEquals("A", reader.getNullTerminatedString(3, Charsets.UTF_8, false));
assertEquals(2, reader.getPosition());

reader = createReader(bytes);
assertEquals("A", reader.getNullTerminatedString(10, Charsets.UTF_8, false));
assertEquals(2, reader.getPosition());
}

@Test
Expand All @@ -248,17 +266,17 @@ public void testGetNullTerminatedStringCursorPositionTest() throws IOException
SequentialReader reader = createReader(bytes);

// try to read first five values
assertEquals("AB", reader.getNullTerminatedString(5, Charsets.UTF_8));
assertEquals("AB", reader.getNullTerminatedString(5, Charsets.UTF_8, false));

// the cursor is after B (third) position
assertEquals(reader.getPosition(), 3);
reader.skip(2);

assertEquals("CD", reader.getNullTerminatedString(3, Charsets.UTF_8));
assertEquals("CD", reader.getNullTerminatedString(3, Charsets.UTF_8, false));

assertEquals(reader.getPosition(), 8);
//no need to skip to next position. since there's only one \0 character after "CD"
assertEquals("ABCDEF", reader.getNullTerminatedString(6, Charsets.UTF_8));
assertEquals("ABCDEF", reader.getNullTerminatedString(6, Charsets.UTF_8, false));
}

@Test
Expand Down

0 comments on commit eb47da1

Please sign in to comment.