Skip to content

feat: add customizable headers in proxy mode #2600

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

chanbengz
Copy link

@chanbengz chanbengz commented Mar 14, 2025

Implement #2552. As suggested, I followed up the way that Proxy::basic_auth finishes its job by extending Proxy and ProxyScheme with a new field called misc. Spare me with the name misc because I can't come up with another word with four letter so that it aligns with host and auth.

I already tested with HTTP and HTTPS proxies but yet with Custom since I haven't taken a look at the custom part. So I will leave it to a future implementation.

Found that something broke with HeaderName::from_static and no one is using it, so I guess we're only able to use those pre-defined HTTP fields for now. It works fine but weird. Why does it have to be lower case?

@seanmonstar
Copy link
Owner

Thanks so much! I've been traveling (and so popping up for emergencies) but will do a proper review very soon.

@chanbengz
Copy link
Author

Have a good trip!

@chanbengz
Copy link
Author

chanbengz commented Mar 19, 2025

I wrote a util macro to generate custom headers. Where should I put it? lib.rs for sure.

macro_rules! headers {
    () => (
        http::HeaderMap::new()
    );
    ($($key:expr => $value:expr),+ $(,)?) => ({
        let mut headers = http::HeaderMap::new();
        $(
            headers.insert($key, $value.parse().unwrap());
        )+
        headers
    });
}

So that we can do

let headers = headers![
    reqwest::header::CONTENT_TYPE => "application/json",
    reqwest::header::USER_AGENT => "reqwest",
];

Copy link
Owner

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Nice work! Well done! Just one thing inline that I'd rather not include.

fix: docs-test for `Proxy::headers`
@chanbengz
Copy link
Author

I didn't really get it but I dropped this macro. I guess I could be maintaining it for quite a long period. :)

@seanmonstar
Copy link
Owner

Hm, looks like CI is noticing a case where something is imported and not required?

@chanbengz
Copy link
Author

chanbengz commented Apr 1, 2025

Aha, it must be the auto-imported things by the editor during my development. I'll get that fixed soon!

And it's not the unused import problem. It's just that I'm missing the cfg(feature = ...) macro.

@chanbengz
Copy link
Author

Done! Please click the CI button. It should be okay by now.

@seanmonstar
Copy link
Owner

Sorry I haven't merged yet, I've been working on hyperium/hyper#3850, to eventually refactor reqwest internals with it. I believe it should be fine (I expect the Proxy struct can include a hyper-util Matcher and optional extra headers as separate fields), but waiting a few weeks felt prudent.

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