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

Bad GET request when host is an IPv6 litteral results in 400 bad Request #997

Open
benjamreis opened this issue Sep 1, 2023 · 2 comments
Labels

Comments

@benjamreis
Copy link

Hello!

I've been having trouble making IPv6 GET requests with the cohttp lib: when the host part of a Uri is an IPv6 litteral brackets needs to be added around it when creating the Host header of the request, please see below:

A request created by this lib:
bad-get

it does not have the brackets and so the Full requst uri field is not valid, whereas a simple wget from the same machine:
good-get
does have them an the URI is valid.

I've been working around it by manually adding the host field with the brackets:

          let headers = Cohttp.Header.init () in
          let headers =
            Cohttp.Header.add_unless_exists headers "host"
              (match Uri.scheme uri with
              | Some "httpunix" -> ""
              | _ -> (
                (Http.Url.maybe_wrap_IPv6_literal (Uri.host_with_default ~default:"localhost" uri))
                  ^
                  match Uri.port uri with Some p -> ":" ^ string_of_int p | None -> ""))
          in
          let request = Cohttp.Request.make ~meth:`GET ?headers:(Some headers) uri in

But the default host when not manually provided should also have them.
From what i understand the brackets should be added here when the host of uri is in IPv6 litteral.

Thanks

@mseri mseri added the Bug label Sep 1, 2023
@mseri
Copy link
Collaborator

mseri commented Sep 15, 2023

I think this should be fixed in the uri package, then all dependent libraries would benefit directly from the fix.

@bernhardkaindl
Copy link

bernhardkaindl commented Sep 19, 2023

Thanks @mseri!

If I read @dinosaure's comment correctly, he agrees with you that it should be fixed in uri:
mirage/ocaml-uri#169 (comment):

It seems correct, the domain part of email addresses also use brackets to delimit user-defined domains or IPv6 address.

benjamreis added a commit to xcp-ng-rpms/xapi that referenced this issue Feb 8, 2024
See: mirage/ocaml-cohttp#997

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>
benjamreis added a commit to xcp-ng-rpms/xapi that referenced this issue Feb 12, 2024
See: mirage/ocaml-cohttp#997

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants