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

Request header support #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ShaunMaher
Copy link

Hi.

I have implemented the support for Arbitrary HTTP Request headers as suggested in #4. It allows the user to specify the "--req-header" argument multiple times to add as many headers as needed.

I hadn't written a line of Rust before yesterday so please be kind in your review. :-)

Kind Regards

Shaun.

@Hezuikn
Copy link
Contributor

Hezuikn commented May 23, 2023

async fn init_http_session(target: &Url, req_headers: &[String]) -> Trace<Uuid> {
    let mut req = CLIENT.get(join_url(target, ["open"]));

    println!("req_headers: {:?}", req_headers);

    for req_header in req_headers {
        let (name, value) = req_header.split_once(": ").unwrap();
        req = req.header(name, value);
    }
...

rest should be correct (i dont know what that /tmp git submodule thing is, does, is for)

@julianbuettner
Copy link
Owner

Thanks for implementing. Looks good in general.

Why are we inserting "key" and "value" into the headers map though?

I am currently on mobile, will look at it and test it on pc later.

@Hezuikn
Copy link
Contributor

Hezuikn commented May 23, 2023

"Why are we inserting "key" and "value" into the headers map though?" artifact from the example i gave in #4

@ShaunMaher
Copy link
Author

(i dont know what that /tmp git submodule thing is, does, is for)

Sorry. That's junk. Can I delete that from the PR in some way or do I need to create a new PR?

@julianbuettner
Copy link
Owner

You can simply delete the line, commit and push it to your branch.

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.

3 participants