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

Implement Seek for io::Empty #78029

Closed
oberien opened this issue Oct 16, 2020 · 4 comments · Fixed by #78044
Closed

Implement Seek for io::Empty #78029

oberien opened this issue Oct 16, 2020 · 4 comments · Fixed by #78044
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@oberien
Copy link
Contributor

oberien commented Oct 16, 2020

You can create an empty io-reader with std::io::empty(), which returns an std::io::Empty. That Empty doesn't implement the Seek trait. However, I don't see any reason why it shouldn't.

@jonas-schievink jonas-schievink added A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 16, 2020
@pickfire
Copy link
Contributor

I don't understand how a reader which is always at EOF should be seekable. Can you please explain more about it?

@oberien
Copy link
Contributor Author

oberien commented Oct 17, 2020

You can think of a reader which is always at EOF as a reader which is always at position 0 with a stream end of position 0. I don't think there is any disadvantage of having Seek implemented for Empty. It allows it to be passed to more generic code.

@KodrAus
Copy link
Contributor

KodrAus commented Oct 23, 2020

@oberien Did you hit a case where you actually wanted to do this?

@oberien
Copy link
Contributor Author

oberien commented Oct 23, 2020

In algesten/ureq#187 ureq is discussing ways to implement a 307/308 HTTP redirect. Those two codes (in contrast to 301/302/303) require the payload to be sent after the redirect. However, parts of the payload may already have been sent to the server, before the server responds with the redirect. With a generic API taking an impl Read as Payload, it's not possible to seek back to the beginning. Thus, one suggestion is to use impl Read + Seek instead. To allow an empty body (e.g. for a GET request), it would be nice to keep using io::Empty.

While ureq will probably decide to follow a different way for those redirects, I still think this pattern makes sense to support.

@bors bors closed this as completed in f1cd179 Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants