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.

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

Incorrect conversion of RangeTo in BytesContentRange conversion #4428

Closed
sameer opened this issue Apr 3, 2024 · 6 comments
Closed

Incorrect conversion of RangeTo in BytesContentRange conversion #4428

sameer opened this issue Apr 3, 2024 · 6 comments

Comments

@sameer
Copy link
Contributor

sameer commented Apr 3, 2024

Hello,

First off just wanted to say thanks for this project, makes integrating with cloud storage providers a lot easier.

Just wanted to report some unexpected behavior with range reads. Specifically, when passing a RangeTo like ..END, it gets interpreted as object.len()-END..object.len() instead. I thought this test line should be equal to H not ! based on the equivalent Rust Playground example, but the meaning is different for Range according to MDN.

In range conversion, maybe an unbounded start needs to be turned into Some(0)? https://docs.rs/opendal/0.45.1/src/opendal/raw/http_util/bytes_range.rs.html#218

@sameer sameer changed the title Incorrect conversion of RangeBounds in BytesContentRange conversion Incorrect conversion of RangeTo in BytesContentRange conversion Apr 3, 2024
@sameer
Copy link
Contributor Author

sameer commented Apr 3, 2024

I guess I missed the doc line that indicates this behavior, but it's really unexpected for Rust:

..1024 means read bytes in range (n - 1024, n) of file

@Xuanwo
Copy link
Member

Xuanwo commented Apr 4, 2024

I guess I missed the doc line that indicates this behavior, but it's really unexpected for Rust:

I'm working on changing this.

@Xuanwo
Copy link
Member

Xuanwo commented Apr 16, 2024

This shoud already been fixed.

@sameer
Copy link
Contributor Author

sameer commented Apr 16, 2024

This shoud already been fixed.

Hey Xuanwo, that's good to hear. What commit/PR was that done in?

@Xuanwo
Copy link
Member

Xuanwo commented Apr 17, 2024

Hey Xuanwo, that's good to hear. What commit/PR was that done in?

Fixed during a refactor: #4381

@sameer
Copy link
Contributor Author

sameer commented Apr 17, 2024

Hey Xuanwo, that's good to hear. What commit/PR was that done in?

Fixed during a refactor: #4381

Sounds good, thanks!

@sameer sameer closed this as completed Apr 17, 2024
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

No branches or pull requests

2 participants