-
Notifications
You must be signed in to change notification settings - Fork 483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
properly strip trailing null characters (\0) from tag data #636
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -78,9 +78,9 @@ public static NikonPictureControl1Directory read(byte[] bytes) throws IOExceptio | |||||||||||||
|
||||||||||||||
NikonPictureControl1Directory directory = new NikonPictureControl1Directory(); | ||||||||||||||
|
||||||||||||||
directory.setObject(TAG_PICTURE_CONTROL_VERSION, reader.getStringValue(4, Charsets.UTF_8)); | ||||||||||||||
directory.setObject(TAG_PICTURE_CONTROL_NAME, reader.getStringValue(20, Charsets.UTF_8)); | ||||||||||||||
directory.setObject(TAG_PICTURE_CONTROL_BASE, reader.getStringValue(20, Charsets.UTF_8)); | ||||||||||||||
directory.setString(TAG_PICTURE_CONTROL_VERSION, reader.getNullTerminatedStringAndSkipToNextPosition(4, Charsets.UTF_8).toString()); | ||||||||||||||
directory.setString(TAG_PICTURE_CONTROL_NAME, reader.getNullTerminatedStringAndSkipToNextPosition(20, Charsets.UTF_8).toString()); | ||||||||||||||
directory.setString(TAG_PICTURE_CONTROL_BASE, reader.getNullTerminatedStringAndSkipToNextPosition(20, Charsets.UTF_8).toString()); | ||||||||||||||
Comment on lines
+81
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to preserve the
Suggested change
|
||||||||||||||
reader.skip(4); | ||||||||||||||
directory.setObject(TAG_PICTURE_CONTROL_ADJUST, reader.getUInt8()); | ||||||||||||||
directory.setObject(TAG_PICTURE_CONTROL_QUICK_ADJUST, reader.getUInt8()); | ||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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.setString(TAG_PICTURE_CONTROL_VERSION, reader.getNullTerminatedStringAndSkipToNextPosition(4, Charsets.UTF_8).toString()); | ||||||||||||||
directory.setString(TAG_PICTURE_CONTROL_NAME, reader.getNullTerminatedStringAndSkipToNextPosition(20, Charsets.UTF_8).toString()); | ||||||||||||||
directory.setString(TAG_PICTURE_CONTROL_BASE, reader.getNullTerminatedStringAndSkipToNextPosition(20, Charsets.UTF_8).toString()); | ||||||||||||||
Comment on lines
+89
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto:
Suggested change
|
||||||||||||||
|
||||||||||||||
reader.skip(4); | ||||||||||||||
directory.setObject(TAG_PICTURE_CONTROL_ADJUST, reader.getByte()); | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -241,24 +241,39 @@ public void testGetNullTerminatedString() throws IOException | |
} | ||
|
||
@Test | ||
public void testGetNullTerminatedStringCursorPositionTest() throws IOException | ||
{ | ||
public void testGetNullTerminatedStringCursorPositionTest() throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please avoid reformatting code in PRs that also include functional changes. Formatting changes are appreciated when they change code towards the predominant style in the repo. We don't need the |
||
byte NULL = 0x00; | ||
byte[] bytes = new byte[]{0x41, 0x42, NULL, NULL, NULL, 0x43, 0x44, NULL, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46}; //AB\0\0\0CD\0ABCDEF | ||
SequentialReader reader = createReader(bytes); | ||
|
||
// try to read first five values | ||
assertEquals("AB", reader.getNullTerminatedString(5, Charsets.UTF_8)); | ||
/* | ||
tried to read first five values | ||
*/ | ||
assertEquals("AB", reader.getNullTerminatedString(5, Charsets.UTF_8).toString()); | ||
|
||
// the cursor is after B (third) position | ||
/* | ||
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).toString()); | ||
|
||
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).toString()); | ||
} | ||
|
||
@Test | ||
public void testGetNullTerminatedStringAndSkipToNextPosition() throws IOException { | ||
byte NULL = 0x00; | ||
byte[] bytes = new byte[]{0x41, 0x42, NULL, NULL, NULL, 0x43, 0x44, NULL, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46}; | ||
SequentialReader reader = createReader(bytes); | ||
|
||
assertEquals("AB", reader.getNullTerminatedStringAndSkipToNextPosition(5, Charsets.UTF_8).toString()); | ||
assertEquals("CD", reader.getNullTerminatedStringAndSkipToNextPosition(3, Charsets.UTF_8).toString()); | ||
assertEquals("ABCDEF", reader.getNullTerminatedStringAndSkipToNextPosition(6, Charsets.UTF_8).toString()); | ||
} | ||
|
||
@Test | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to add a new method. We can call
getNullTerminatedStringValue
instead.