Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Read with len = 0 should return 0 bytes read. #201

Closed
1 task done
fuatbasik opened this issue Dec 18, 2024 · 1 comment
Closed
1 task done

Read with len = 0 should return 0 bytes read. #201

fuatbasik opened this issue Dec 18, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@fuatbasik
Copy link
Contributor

Analytics Accelerator Library for Amazon S3 Version

0.0.1, 0.0.2

AWS Region

us-east-1

Describe the running environment

Running in EC2 instance and mac.

What happened?

According to InputStream specification, when reader tries to read 0 bytes, read method should return 0 bytes read, independent of the object size.
See:

If len is zero, then no bytes are read and 0 is returned; otherwise, there is an attempt to read at least one byte. If no byte is available because the stream is at end of file, the value -1 is returned; otherwise, at least one byte is read and stored into b.

In S3SeekableInputStream for an 0-sized object implementation of the read method will return -1.

https://github.com/awslabs/analytics-accelerator-s3/blob/main/input-stream/src/main/java/software/amazon/s3/analyticsaccelerator/S3SeekableInputStream.java#L135-L153

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@fuatbasik fuatbasik added the bug Something isn't working label Dec 18, 2024
@fuatbasik fuatbasik mentioned this issue Dec 18, 2024
Merged
3 tasks
@fuatbasik fuatbasik mentioned this issue Jan 3, 2025
Merged
fuatbasik added a commit that referenced this issue Jan 7, 2025
@fuatbasik
Fix Len = 0 and Insufficient Buffer behaviours for positioned reads (#…
32946d0 
…203)

## Description of change

According to InputStream documentation Len = 0 should return 0

`If len is zero, then no bytes are read and 0 is returned;` 

Following the InputStream implementation, this commit is adding argument
checks to Positioned Reads. This validation is implemented in the
SeekableInputStream to ensure all subclasses behave the same way in the
unhappy path.

Checks are done in the same order with InputStream implementation. 

#### Relevant issues

#201

#### Does this contribution introduce any breaking changes to the
existing APIs or behaviors?

Yes. For a 0-length read to a 0-sized object read will now return 0
instead of -1.
Insufficient Buffer Capacity will now throw IndexOutOfBoundsException
instead of IllegalArgumentException.


#### Does this contribution introduce any new public APIs or behaviors?
No, it changes existing public API behaviour. 


#### How was the contribution tested?
Added new unit tests. 

#### Does this contribution need a changelog entry?
- [N/A] I have updated the CHANGELOG or README if appropriate

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).
@fuatbasik
Copy link
Contributor Author

It is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@fuatbasik