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

Support typed header insertion for RequestBuilder #1982

Open
jmagnuson opened this issue Sep 22, 2023 · 2 comments · May be fixed by #2546
Open

Support typed header insertion for RequestBuilder #1982

jmagnuson opened this issue Sep 22, 2023 · 2 comments · May be fixed by #2546

Comments

@jmagnuson
Copy link

Currently the typical way to insert a single header into a RequestBuilder is something like (e.g., Range header):

let request = client
    .get(file_url)
    .header(RANGE, format!("bytes={offset}-"));

let mut response = request.send().await?;

I recently discovered the awesome headers crate and wanted to replace the string formatting with something strongly typed. However, there doesn't currently exist a direct way to plug something that implements Header.

One workaround is to construct a HeaderMap, for which headers::HeaderMapExt::typed_insert exists:

use headers::HeaderMapExt;

let request = client
    .get(file_url);

let header_map = {
    let mut header_map = headers::HeaderMap::new();
    let range_header = headers::Range::bytes(offset..).unwrap()
    header_map.typed_insert(range_header);
    header_map
};

let request = request.headers(header_map);

let mut response = request.send().await?;

But to me, the intermediate steps start to diminish the utility of RequestBuilder (at least, since there's only one custom header to insert). Another workaround would be to build the Request, and then add to its internal HeaderMap:

use headers::HeaderMapExt;

let mut request = client
    .get(file_url)
    .build()?;

let header_map = request.headers_mut();
let range_header = headers::Range::bytes(offset..).unwrap()
header_map.typed_insert(range_header);

let mut response = client.execute(request).await?;

but this somewhat feels like Request is doing the job really meant for RequestBuilder.

Ideally, reqwest provides a RequestBuilder::typed_header to directly insert a impl Header instance. Alternatively, headers could expose something like RequestBuilderExt, similar to HeaderMapExt. Although, that might well go beyond the intended scope of headers.

Are there any other solutions that I may have missed? I'm happy to submit a PR for what ever path is most agreeable.

Thank you for reqwest (and headers)!

@seanmonstar
Copy link
Owner

I don't disagree, it would be very cool! I just am not sure about making reqwest depend on headers publicly. Then breaking changes in headers means we have to break reqwest too.

@djugei
Copy link

djugei commented Feb 9, 2025

@jmagnuson thanks for that second solution, that saves an allocation, did not even think of that!

@seanmonstar a way to expose this without adding a dependency and without altering the current internal structure of holding a Result in the RequestBuilder would be to add a

.alter_headers(|&mut HeaderMap|) -> RequestBuilder

that does .as_mut().map() internally.

actually let me just contribute that real quick.

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 a pull request may close this issue.

3 participants