Skip to content

[VFS-624] Fix for read() in constructors of LocalFileRandomAccessContent and RamFileRandomAccessContent#93

Merged
garydgregory merged 2 commits intoapache:masterfrom
PeterAlfredLee:VFS-624-C
Sep 6, 2020
Merged

[VFS-624] Fix for read() in constructors of LocalFileRandomAccessContent and RamFileRandomAccessContent#93
garydgregory merged 2 commits intoapache:masterfrom
PeterAlfredLee:VFS-624-C

Conversation

@PeterAlfredLee
Copy link
Member

@PeterAlfredLee PeterAlfredLee commented Jun 15, 2020

As VFS-624 described, the readof the inner class in LocalFileRandomAccessContent would return -1 if it's reading a 0xFF in file.
This is a fix for it.

And the same problem also exists in RamFileRandomAccessContent and the fix is also included in this PR.

public int read() throws IOException {
try {
return raf.readByte();
return raf.readByte() & 0xFF;
Copy link
Member

@garydgregory garydgregory Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid repeated use of a magic number. Please refactor into a constant that's reused in the code and tests. Same for -1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.
Seems we need a place to store the constant 0xFF. IMO we should have a class to store constants like 0xFF, which seems is not presented in VFS. What about adding a VFSConstants class in org.apache.commons.vfs2.util?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I would just add it as a constant in RamFileRandomAccessContent. The only other place I see it used, is once in an unrelated sandbox class.

Fix and add a test. This problem is similar to VFS-624
Replace all 0xFF to constant BYTE_VALUE_MASK
@garydgregory garydgregory merged commit d73305a into apache:master Sep 6, 2020
asfgit pushed a commit that referenced this pull request Sep 6, 2020
@PeterAlfredLee PeterAlfredLee deleted the VFS-624-C branch September 7, 2020 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants