{"thread":{"id":"57830","subject":"[PATCH] http: add custom hostname to IP address resolves","startedAt":"2022-05-02T08:37:13Z","lastAt":"2022-05-16T08:39:15Z","messageCount":20,"participants":["Christian Couder","Junio C Hamano","Carlo Marcelo Arenas Belón","Carlo Arenas","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"454709","messageId":"20220502083639.610279-1-chriscool@tuxfamily.org","threadId":"57830","inReplyTo":null,"subject":"[PATCH] http: add custom hostname to IP address resolves","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-02T08:36:39Z","receivedAt":"2022-05-02T08:37:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Libcurl has a CURLOPT_RESOLVE easy option that allows\nhostname resolve information in the following form to\nbe passed:\n\n\t[+]HOST:PORT:ADDRESS[,ADDRESS]\n\nThis way, redirects and everything operating against the\nHOST+PORT will use the provided ADDRESS(s).\n\nThe following form is also allowed to stop using these\nresolves:\n\n\t-HOST:PORT\n\nLet's add a corresponding \"http.hostResolve\" config\noption that takes advantage of CURLOPT_RESOLVE.\n\nEach value configured for the \"http.hostResolve\" key\nis passed \"as is\" to curl through CURLOPT_RESOLVE, so it\nshould be in one of the above 2 forms. This keeps the\nimplementation simple and makes us consistent with\nlibcurl's CURLOPT_RESOLVE, and with curl's corresponding\n`--resolve` command line option.\n\nThe implementation is similar to what is done for the\n\"http.extraHeader\" config option, except that we use\nCURLOPT_RESOLVE only in get_active_slot() which is\ncalled by all the HTTP request sending functions.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n\nI am not sure if some tests could/should be added. Ideas about how to\ntest this are welcome.\n\nDocumentation/config/http.txt | 16 ++++++++++++++++\n http.c                        | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 48 insertions(+)\n\ndiff --git a/Documentation/config/http.txt b/Documentation/config/http.txt\nindex 7003661c0d..37b293a73b 100644\n--- a/Documentation/config/http.txt\n+++ b/Documentation/config/http.txt\n@@ -98,6 +98,22 @@ http.version::\n \t- HTTP/2\n \t- HTTP/1.1\n \n+http.hostResolve::\n+\tHostname resolve information that will be used first when sending\n+\tHTTP requests.  This information should be in one of the following\n+\tforms:\n+\n+\t- [+]HOST:PORT:ADDRESS[,ADDRESS]\n+\t- -HOST:PORT\n+\n++\n+The first form redirects all requests to the given `HOST:PORT`\n+to the provided `ADDRESS`(s). The second form clears all previous\n+config values for that `HOST:PORT` combination.  To allow easy\n+overriding of all the settings inherited from the system config,\n+an empty value will reset all resolve information to the empty\n+list.\n+\n http.sslVersion::\n \tThe SSL version to use when negotiating an SSL connection, if you\n \twant to force the default.  The available and default version\ndiff --git a/http.c b/http.c\nindex 229da4d148..e9cc46ee52 100644\n--- a/http.c\n+++ b/http.c\n@@ -128,6 +128,9 @@ static struct curl_slist *pragma_header;\n static struct curl_slist *no_pragma_header;\n static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;\n \n+static struct curl_slist *host_resolves;\n+static struct string_list http_host_resolve = STRING_LIST_INIT_DUP;\n+\n static struct active_request_slot *active_queue_head;\n \n static char *cached_accept_language;\n@@ -393,6 +396,17 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.hostresolve\", var)) {\n+\t\tif (!value) {\n+\t\t\treturn config_error_nonbool(var);\n+\t\t} else if (!*value) {\n+\t\t\tstring_list_clear(&http_host_resolve, 0);\n+\t\t} else {\n+\t\t\tstring_list_append(&http_host_resolve, value);\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.followredirects\", var)) {\n \t\tif (value && !strcmp(value, \"initial\"))\n \t\t\thttp_follow_config = HTTP_FOLLOW_INITIAL;\n@@ -985,6 +999,17 @@ static void set_from_env(const char **var, const char *envname)\n \t\t*var = val;\n }\n \n+static struct curl_slist *http_copy_host_resolve(void)\n+{\n+\tstruct curl_slist *hosts = NULL;\n+\tconst struct string_list_item *item;\n+\n+\tfor_each_string_list_item(item, &http_host_resolve)\n+\t\thosts = curl_slist_append(hosts, item->string);\n+\n+\treturn hosts;\n+}\n+\n void http_init(struct remote *remote, const char *url, int proactive_auth)\n {\n \tchar *low_speed_limit;\n@@ -1048,6 +1073,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n \tno_pragma_header = curl_slist_append(http_copy_default_headers(),\n \t\t\"Pragma:\");\n \n+\thost_resolves = http_copy_host_resolve();\n+\n \t{\n \t\tchar *http_max_requests = getenv(\"GIT_HTTP_MAX_REQUESTS\");\n \t\tif (http_max_requests != NULL)\n@@ -1124,6 +1151,7 @@ void http_cleanup(void)\n \tcurl_global_cleanup();\n \n \tstring_list_clear(&extra_http_headers, 0);\n+\tstring_list_clear(&http_host_resolve, 0);\n \n \tcurl_slist_free_all(pragma_header);\n \tpragma_header = NULL;\n@@ -1131,6 +1159,9 @@ void http_cleanup(void)\n \tcurl_slist_free_all(no_pragma_header);\n \tno_pragma_header = NULL;\n \n+\tcurl_slist_free_all(host_resolves);\n+\thost_resolves = NULL;\n+\n \tif (curl_http_proxy) {\n \t\tfree((void *)curl_http_proxy);\n \t\tcurl_http_proxy = NULL;\n@@ -1211,6 +1242,7 @@ struct active_request_slot *get_active_slot(void)\n \tif (curl_save_cookies)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_COOKIEJAR, curl_cookie_file);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_RESOLVE, host_resolves);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);\n \tcurl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, NULL);\n-- \n2.36.0\n\n"},{"id":"454744","messageId":"xmqqfslrycvp.fsf@gitster.g","threadId":"57830","inReplyTo":"20220502083639.610279-1-chriscool@tuxfamily.org","subject":"Re: [PATCH] http: add custom hostname to IP address resolves","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-02T19:04:42Z","receivedAt":"2022-05-02T19:04:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Subject: Re: [PATCH] http: add custom hostname to IP address resolves\n\nI can guess what you means, but I am not able to quite parse the\nabove.  I guess the phrase that makes me hiccup when I read is\n\"resolve\" used as a noun.\n\n> Libcurl has a CURLOPT_RESOLVE easy option that allows\n> hostname resolve information in the following form to\n\nThe same here, \"... allows the result of hostname resolution in the\nfollowing format ...\", perhaps?\n\n> be passed:\n>\n> \t[+]HOST:PORT:ADDRESS[,ADDRESS]\n>\n> This way, redirects and everything operating against the\n> HOST+PORT will use the provided ADDRESS(s).\n>\n> The following form is also allowed to stop using these\n> resolves:\n>\n> \t-HOST:PORT\n\nThe above is a reasonable summary of CURLOPT_RESOLVE documentation\nthat is appropriate to have here for those of us who are not\nfamiliar with it.  For those of us who may want to learn more, it\nwould help to have an URL to the canonical documentation page, e.g.\nhttps://curl.se/libcurl/c/CURLOPT_RESOLVE.html but it is not\nrequired.  People should be able to find it easily.\n\n> Let's add a corresponding \"http.hostResolve\" config\n> option that takes advantage of CURLOPT_RESOLVE.\n\nCURLOPT_RESOLVE allows us to feed cURL a list of these <host,port>\n-> <address> mappings, so we use that mechansim to feed the values\nlisted on the multi-valued configuration variable (spell it out as\nsuch, by the way, instead of saying \"config option\", which may give\na false impression that it is a last-one-wins single string with\nmany such mapping entries on it).\n\nOK.\n\n> Each value configured for the \"http.hostResolve\" key\n> is passed \"as is\" to curl through CURLOPT_RESOLVE, so it\n> should be in one of the above 2 forms. This keeps the\n> implementation simple and makes us consistent with\n> libcurl's CURLOPT_RESOLVE, and with curl's corresponding\n> `--resolve` command line option.\n\nOK.\n\n> I am not sure if some tests could/should be added. Ideas about how to\n> test this are welcome.\n\nIt should.  Perhaps invent a totally bogus domain name, map that to\nlocalhost ::1, run a test server locally, and try to clone from that\nbogus domain?\n\n> +http.hostResolve::\n\nIs \"host\" a good prefix for it?  \n\nIn the context of HTTP(s), if there is no other thing than host that\nwe resolve, \"http.resolve\" may be sufficient.  For those who are\nlooking for CURLOPT_RESOLVE equivalent, \"http.curloptResolve\" may\nmake it easier to find.\n\n> +\tHostname resolve information that will be used first when sending\n> +\tHTTP requests.  This information should be in one of the following\n> +\tforms:\n> +\n> +\t- [+]HOST:PORT:ADDRESS[,ADDRESS]\n> +\t- -HOST:PORT\n> +\n> ++\n> +The first form redirects all requests to the given `HOST:PORT`\n> +to the provided `ADDRESS`(s). The second form clears all previous\n> +config values for that `HOST:PORT` combination.  To allow easy\n> +overriding of all the settings inherited from the system config,\n> +an empty value will reset all resolve information to the empty\n> +list.\n\nIf I understand your use case correctly, this is not something you\nwould want to hardcode in your configuration files for long term, is\nit?  I am wondering if we want to mention the expected use case here\nas well, something like\n\n    This is designed to be used primarily from the command line\n    configuration variable override, e.g.\n\n\t$ git -c http.resolve=example.com:443:127.0.0.1 \\\n\t    clone https://example.com/user/project.git\n\nperhaps?  Not a suggestion, but soliciting thoughts.\n\n> diff --git a/http.c b/http.c\n> index 229da4d148..e9cc46ee52 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -128,6 +128,9 @@ static struct curl_slist *pragma_header;\n>  static struct curl_slist *no_pragma_header;\n>  static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;\n>  \n> +static struct curl_slist *host_resolves;\n> +static struct string_list http_host_resolve = STRING_LIST_INIT_DUP;\n> +\n>  static struct active_request_slot *active_queue_head;\n>  \n>  static char *cached_accept_language;\n> @@ -393,6 +396,17 @@ static int http_options(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif (!strcmp(\"http.hostresolve\", var)) {\n> +\t\tif (!value) {\n> +\t\t\treturn config_error_nonbool(var);\n> +\t\t} else if (!*value) {\n> +\t\t\tstring_list_clear(&http_host_resolve, 0);\n\n\nOK, this is a way to \"clear\" the list of entries accumulated on this\nmulti-valued configuration variable so far.  And it is documented in\nthe above, too.  Good.\n\n> +\t\t} else {\n> +\t\t\tstring_list_append(&http_host_resolve, value);\n> +\t\t}\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (!strcmp(\"http.followredirects\", var)) {\n>  \t\tif (value && !strcmp(value, \"initial\"))\n>  \t\t\thttp_follow_config = HTTP_FOLLOW_INITIAL;\n> @@ -985,6 +999,17 @@ static void set_from_env(const char **var, const char *envname)\n>  \t\t*var = val;\n>  }\n>  \n> +static struct curl_slist *http_copy_host_resolve(void)\n> +{\n> +\tstruct curl_slist *hosts = NULL;\n> +\tconst struct string_list_item *item;\n> +\n> +\tfor_each_string_list_item(item, &http_host_resolve)\n> +\t\thosts = curl_slist_append(hosts, item->string);\n> +\n> +\treturn hosts;\n> +}\n> +\n>  void http_init(struct remote *remote, const char *url, int proactive_auth)\n>  {\n>  \tchar *low_speed_limit;\n> @@ -1048,6 +1073,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n>  \tno_pragma_header = curl_slist_append(http_copy_default_headers(),\n>  \t\t\"Pragma:\");\n>  \n> +\thost_resolves = http_copy_host_resolve();\n\nThis is curious.  I imagined that the reason why you keep the\noriginal in a string_list and copy it to a curl_slist was perhaps\nbecause you'll lose the latter every time you make a curl request,\nbut it does not appear to be the case.  You http_init() just once\nand then the same CURL *curl instance will be reused until you clear\nit with http_cleanup().  So I do not see offhand the need to have\nthe string_list at all.\n\nDoes it work equally well if you used curl_slist_append() in\nhttp_options() and maintain ONLY the curl_slist version of the\nhost_resolve list?  That would make it unnecessary to keep\nhttp_host_resolve and add http_copy_host_resolve() function, no?\n\nThanks.\n"},{"id":"454806","messageId":"CAP8UFD0hWUudP6pZVGS5yOVCjbBCm1LdK_EbrsQp9KiVPPMCyA@mail.gmail.com","threadId":"57830","inReplyTo":"xmqqfslrycvp.fsf@gitster.g","subject":"Re: [PATCH] http: add custom hostname to IP address resolves","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-04T10:07:59Z","receivedAt":"2022-05-04T10:08:17Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, May 2, 2022 at 9:04 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n> > Subject: Re: [PATCH] http: add custom hostname to IP address resolves\n>\n> I can guess what you means, but I am not able to quite parse the\n> above.  I guess the phrase that makes me hiccup when I read is\n> \"resolve\" used as a noun.\n\nOk, I will use \"http: add custom hostname to IP address resolutions\"\nin the next version then.\n\n> > Libcurl has a CURLOPT_RESOLVE easy option that allows\n> > hostname resolve information in the following form to\n>\n> The same here, \"... allows the result of hostname resolution in the\n> following format ...\", perhaps?\n\nOk, I will use your suggestion.\n\n> > be passed:\n> >\n> >       [+]HOST:PORT:ADDRESS[,ADDRESS]\n> >\n> > This way, redirects and everything operating against the\n> > HOST+PORT will use the provided ADDRESS(s).\n> >\n> > The following form is also allowed to stop using these\n> > resolves:\n> >\n> >       -HOST:PORT\n>\n> The above is a reasonable summary of CURLOPT_RESOLVE documentation\n> that is appropriate to have here for those of us who are not\n> familiar with it.  For those of us who may want to learn more, it\n> would help to have an URL to the canonical documentation page, e.g.\n> https://curl.se/libcurl/c/CURLOPT_RESOLVE.html but it is not\n> required.  People should be able to find it easily.\n\nYeah, I also thought that it wasn't required, but I will add it\nanyway, as I agree it could be useful and hopefully it doesn't change\nvery often.\n\n> > Let's add a corresponding \"http.hostResolve\" config\n> > option that takes advantage of CURLOPT_RESOLVE.\n>\n> CURLOPT_RESOLVE allows us to feed cURL a list of these <host,port>\n> -> <address> mappings, so we use that mechansim to feed the values\n> listed on the multi-valued configuration variable (spell it out as\n> such, by the way, instead of saying \"config option\", which may give\n> a false impression that it is a last-one-wins single string with\n> many such mapping entries on it).\n>\n> OK.\n>\n> > Each value configured for the \"http.hostResolve\" key\n> > is passed \"as is\" to curl through CURLOPT_RESOLVE, so it\n> > should be in one of the above 2 forms. This keeps the\n> > implementation simple and makes us consistent with\n> > libcurl's CURLOPT_RESOLVE, and with curl's corresponding\n> > `--resolve` command line option.\n>\n> OK.\n>\n> > I am not sure if some tests could/should be added. Ideas about how to\n> > test this are welcome.\n>\n> It should.  Perhaps invent a totally bogus domain name, map that to\n> localhost ::1, run a test server locally, and try to clone from that\n> bogus domain?\n\nOk, I will add a simple test like this.\n\n> > +http.hostResolve::\n>\n> Is \"host\" a good prefix for it?\n>\n> In the context of HTTP(s), if there is no other thing than host that\n> we resolve, \"http.resolve\" may be sufficient.  For those who are\n> looking for CURLOPT_RESOLVE equivalent, \"http.curloptResolve\" may\n> make it easier to find.\n\nI am Ok with just \"http.resolve\". I think using \"curlopt\" is perhaps\ngoing into too many details about the implementation of the feature,\nwhich could theoretically change if we ever decided to use something\nother than curl.\n\n> > +     Hostname resolve information that will be used first when sending\n> > +     HTTP requests.  This information should be in one of the following\n> > +     forms:\n> > +\n> > +     - [+]HOST:PORT:ADDRESS[,ADDRESS]\n> > +     - -HOST:PORT\n> > +\n> > ++\n> > +The first form redirects all requests to the given `HOST:PORT`\n> > +to the provided `ADDRESS`(s). The second form clears all previous\n> > +config values for that `HOST:PORT` combination.  To allow easy\n> > +overriding of all the settings inherited from the system config,\n> > +an empty value will reset all resolve information to the empty\n> > +list.\n>\n> If I understand your use case correctly, this is not something you\n> would want to hardcode in your configuration files for long term, is\n> it?\n\nRight, but I wonder if there are other use cases where people would\nlike to hardcode it, for example on a private network where IP\naddresses rarely change. Also a config option makes it more consistent\nwith \"http.extraHeaders\" and other such options.\n\n> I am wondering if we want to mention the expected use case here\n> as well, something like\n>\n>     This is designed to be used primarily from the command line\n>     configuration variable override, e.g.\n>\n>         $ git -c http.resolve=example.com:443:127.0.0.1 \\\n>             clone https://example.com/user/project.git\n>\n> perhaps?  Not a suggestion, but soliciting thoughts.\n\nI am also interested in others' thoughts about this. If no one thinks\nthat a config option could be useful, I am Ok with making it a\n\"--resolve\" command line option that can be passed to any Git command\nsimilar to \"-c <name>=<value>\":\n\ngit --resolve=... <command> [<args>]\n\n> > diff --git a/http.c b/http.c\n> > index 229da4d148..e9cc46ee52 100644\n> > --- a/http.c\n> > +++ b/http.c\n> > @@ -128,6 +128,9 @@ static struct curl_slist *pragma_header;\n> >  static struct curl_slist *no_pragma_header;\n> >  static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;\n> >\n> > +static struct curl_slist *host_resolves;\n> > +static struct string_list http_host_resolve = STRING_LIST_INIT_DUP;\n> > +\n> >  static struct active_request_slot *active_queue_head;\n> >\n> >  static char *cached_accept_language;\n> > @@ -393,6 +396,17 @@ static int http_options(const char *var, const char *value, void *cb)\n> >               return 0;\n> >       }\n> >\n> > +     if (!strcmp(\"http.hostresolve\", var)) {\n> > +             if (!value) {\n> > +                     return config_error_nonbool(var);\n> > +             } else if (!*value) {\n> > +                     string_list_clear(&http_host_resolve, 0);\n>\n> OK, this is a way to \"clear\" the list of entries accumulated on this\n> multi-valued configuration variable so far.  And it is documented in\n> the above, too.  Good.\n>\n> > +             } else {\n> > +                     string_list_append(&http_host_resolve, value);\n> > +             }\n> > +             return 0;\n> > +     }\n> > +\n> >       if (!strcmp(\"http.followredirects\", var)) {\n> >               if (value && !strcmp(value, \"initial\"))\n> >                       http_follow_config = HTTP_FOLLOW_INITIAL;\n> > @@ -985,6 +999,17 @@ static void set_from_env(const char **var, const char *envname)\n> >               *var = val;\n> >  }\n> >\n> > +static struct curl_slist *http_copy_host_resolve(void)\n> > +{\n> > +     struct curl_slist *hosts = NULL;\n> > +     const struct string_list_item *item;\n> > +\n> > +     for_each_string_list_item(item, &http_host_resolve)\n> > +             hosts = curl_slist_append(hosts, item->string);\n> > +\n> > +     return hosts;\n> > +}\n> > +\n> >  void http_init(struct remote *remote, const char *url, int proactive_auth)\n> >  {\n> >       char *low_speed_limit;\n> > @@ -1048,6 +1073,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n> >       no_pragma_header = curl_slist_append(http_copy_default_headers(),\n> >               \"Pragma:\");\n> >\n> > +     host_resolves = http_copy_host_resolve();\n>\n> This is curious.  I imagined that the reason why you keep the\n> original in a string_list and copy it to a curl_slist was perhaps\n> because you'll lose the latter every time you make a curl request,\n> but it does not appear to be the case.  You http_init() just once\n> and then the same CURL *curl instance will be reused until you clear\n> it with http_cleanup().  So I do not see offhand the need to have\n> the string_list at all.\n\nOk, I will remove it.\n\n> Does it work equally well if you used curl_slist_append() in\n> http_options() and maintain ONLY the curl_slist version of the\n> host_resolve list?  That would make it unnecessary to keep\n> http_host_resolve and add http_copy_host_resolve() function, no?\n\nYeah, right. I will remove http_host_resolve and http_copy_host_resolve().\n\nThanks!\n"},{"id":"454807","messageId":"20220504104601.136403-1-chriscool@tuxfamily.org","threadId":"57830","inReplyTo":"20220502083639.610279-1-chriscool@tuxfamily.org","subject":"[PATCH v2] http: add custom hostname to IP address resolutions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-04T10:46:01Z","receivedAt":"2022-05-04T10:46:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Libcurl has a CURLOPT_RESOLVE easy option that allows\nthe result of hostname resolution in the following\nformat to be passed:\n\n\t[+]HOST:PORT:ADDRESS[,ADDRESS]\n\nThis way, redirects and everything operating against the\nHOST+PORT will use the provided ADDRESS(s).\n\nThe following format is also allowed to stop using\nhostname resolutions that have already been passed:\n\n\t-HOST:PORT\n\nSee https://curl.se/libcurl/c/CURLOPT_RESOLVE.html for\nmore details.\n\nLet's add a corresponding \"http.resolve\" config option\nthat takes advantage of CURLOPT_RESOLVE.\n\nEach value configured for the \"http.resolve\" key is\npassed \"as is\" to curl through CURLOPT_RESOLVE, so it\nshould be in one of the above 2 formats. This keeps the\nimplementation simple and makes us consistent with\nlibcurl's CURLOPT_RESOLVE, and with curl's corresponding\n`--resolve` command line option.\n\nThe implementation uses CURLOPT_RESOLVE only in\nget_active_slot() which is called by all the HTTP\nrequest sending functions.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n\nChanges since v1 are the following:\n\n  - rename the new config option to \"http.resolve\" \n  - use \"resolution\" instead of \"resolve\" for the noun\n  - use \"format\" instead of \"form\"\n  - improved commit message and documentation\n  - stop using a string_list and remove unnecessary\n    related variables and functions\n  - add a simple test\n\nThanks to Junio and Carlo for the suggestions.\n\nAs this version is very different from v1, I am not\nsure a range diff would be very useful as it would\nbe very long compared to the size of the patch.\n\n Documentation/config/http.txt | 16 ++++++++++++++++\n http.c                        | 18 ++++++++++++++++++\n t/t5551-http-fetch-smart.sh   |  7 +++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/config/http.txt b/Documentation/config/http.txt\nindex 7003661c0d..86f8a5978f 100644\n--- a/Documentation/config/http.txt\n+++ b/Documentation/config/http.txt\n@@ -98,6 +98,22 @@ http.version::\n \t- HTTP/2\n \t- HTTP/1.1\n \n+http.resolve::\n+\tHostname resolution information that will be used first when sending\n+\tHTTP requests.  This information should be in one of the following\n+\tformats:\n+\n+\t- [+]HOST:PORT:ADDRESS[,ADDRESS]\n+\t- -HOST:PORT\n+\n++\n+The first format redirects all requests to the given `HOST:PORT`\n+to the provided `ADDRESS`(s). The second format clears all\n+previous config values for that `HOST:PORT` combination.  To\n+allow easy overriding of all the settings inherited from the\n+system config, an empty value will reset all resolution\n+information to the empty list.\n+\n http.sslVersion::\n \tThe SSL version to use when negotiating an SSL connection, if you\n \twant to force the default.  The available and default version\ndiff --git a/http.c b/http.c\nindex 229da4d148..7f3b7403ce 100644\n--- a/http.c\n+++ b/http.c\n@@ -128,6 +128,8 @@ static struct curl_slist *pragma_header;\n static struct curl_slist *no_pragma_header;\n static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;\n \n+static struct curl_slist *host_resolutions;\n+\n static struct active_request_slot *active_queue_head;\n \n static char *cached_accept_language;\n@@ -393,6 +395,18 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.resolve\", var)) {\n+\t\tif (!value) {\n+\t\t\treturn config_error_nonbool(var);\n+\t\t} else if (!*value) {\n+\t\t\tcurl_slist_free_all(host_resolutions);\n+\t\t\thost_resolutions = NULL;\n+\t\t} else {\n+\t\t\thost_resolutions = curl_slist_append(host_resolutions, value);\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.followredirects\", var)) {\n \t\tif (value && !strcmp(value, \"initial\"))\n \t\t\thttp_follow_config = HTTP_FOLLOW_INITIAL;\n@@ -1131,6 +1145,9 @@ void http_cleanup(void)\n \tcurl_slist_free_all(no_pragma_header);\n \tno_pragma_header = NULL;\n \n+\tcurl_slist_free_all(host_resolutions);\n+\thost_resolutions = NULL;\n+\n \tif (curl_http_proxy) {\n \t\tfree((void *)curl_http_proxy);\n \t\tcurl_http_proxy = NULL;\n@@ -1211,6 +1228,7 @@ struct active_request_slot *get_active_slot(void)\n \tif (curl_save_cookies)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_COOKIEJAR, curl_cookie_file);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_RESOLVE, host_resolutions);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);\n \tcurl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, NULL);\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex f92c79c132..d97380be87 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n \tgrep symref=HEAD:refs/heads/ trace\n '\n \n+test_expect_success 'passing hostname resolution information works' '\n+\tBOGUS_HOST=gitbogusexamplehost.com &&\n+\tBOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n+\ttest_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n+\tgit -c \"http.resolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n+'\n+\n test_done\n-- \n2.36.0.1.g9c537b8458.dirty\n\n"},{"id":"454813","messageId":"xmqqzgjxnz73.fsf@gitster.g","threadId":"57830","inReplyTo":"CAP8UFD0hWUudP6pZVGS5yOVCjbBCm1LdK_EbrsQp9KiVPPMCyA@mail.gmail.com","subject":"Re: [PATCH] http: add custom hostname to IP address resolves","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-04T14:34:56Z","receivedAt":"2022-05-04T14:35:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>> The above is a reasonable summary of CURLOPT_RESOLVE documentation\n>> that is appropriate to have here for those of us who are not\n>> familiar with it.  For those of us who may want to learn more, it\n>> would help to have an URL to the canonical documentation page, e.g.\n>> https://curl.se/libcurl/c/CURLOPT_RESOLVE.html but it is not\n>> required.  People should be able to find it easily.\n>\n> Yeah, I also thought that it wasn't required, but I will add it\n> anyway, as I agree it could be useful and hopefully it doesn't change\n> very often.\n\nAh, I didn't consider the URL going stale at all.  Forcing readers\nto look for the keyword certainly is a way to avoid it, but they\nwill do that once they realize URL went stale, so there is not a\nstrong incentive to avoid recording the now-current URL, I would\nthink.\n\n>> > +http.hostResolve::\n>>\n>> Is \"host\" a good prefix for it?\n>>\n>> In the context of HTTP(s), if there is no other thing than host that\n>> we resolve, \"http.resolve\" may be sufficient.  For those who are\n>> looking for CURLOPT_RESOLVE equivalent, \"http.curloptResolve\" may\n>> make it easier to find.\n>\n> I am Ok with just \"http.resolve\". I think using \"curlopt\" is perhaps\n> going into too many details about the implementation of the feature,\n> which could theoretically change if we ever decided to use something\n> other than curl.\n\nYou may want to step back a bit and rethink.\n\nEven if we decide to rewrite that part of the system not to depend\non cURL, end-user facing documented interface, i.e. how the mappings\nare given to the system, will stay with us, and it is clear that it\nwas modeled after CURLOPT_RESOLVE---well, it was stolen from them\nverbatim ;-).\n\nSo we may wean ourselves off of cURL, but CURLOPT_RESOLVE will stay\nwith us for this particular feature.\n\n>> I am wondering if we want to mention the expected use case here\n>> as well, something like\n>>\n>>     This is designed to be used primarily from the command line\n>>     configuration variable override, e.g.\n>>\n>>         $ git -c http.resolve=example.com:443:127.0.0.1 \\\n>>             clone https://example.com/user/project.git\n>>\n>> perhaps?  Not a suggestion, but soliciting thoughts.\n>\n> I am also interested in others' thoughts about this. If no one thinks\n> that a config option could be useful, I am Ok with making it a\n> \"--resolve\" command line option that can be passed to any Git command\n> similar to \"-c <name>=<value>\":\n>\n> git --resolve=... <command> [<args>]\n\nAbsolutely not.\n\n\"git [push|fetch|clone|ls-remote] --dns-pre-resolve=...\" that is\n*NOT* git wide, but is only for transport commands might be a\npossibility, but even then, you'd need to invent a way to do the\nsame for non cURL transports (we want to be able to pin the IP when\ngoing over SSH to a certain host, for the same reason) if we promote\nit to an officially supported command line option.\n\nUnless we do that, it is probably better to leave it as an obscure\nconfiguration meant to help server operators.  At least, with the\nname of the configuration variable prefixed with http.*, we have a\nvalid excuse when somebody complains \"the feature does not do\nanything for git:// transport\".\n\nThanks.\n"},{"id":"454850","messageId":"CAP8UFD0V6scSD8LuUiRQ0BWmG2mFATVFNQ2DgwaWFp6bigBkZw@mail.gmail.com","threadId":"57830","inReplyTo":"xmqqzgjxnz73.fsf@gitster.g","subject":"Re: [PATCH] http: add custom hostname to IP address resolves","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-05T10:48:50Z","receivedAt":"2022-05-05T10:49:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, May 4, 2022 at 4:34 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n\n> > I am Ok with just \"http.resolve\". I think using \"curlopt\" is perhaps\n> > going into too many details about the implementation of the feature,\n> > which could theoretically change if we ever decided to use something\n> > other than curl.\n>\n> You may want to step back a bit and rethink.\n>\n> Even if we decide to rewrite that part of the system not to depend\n> on cURL, end-user facing documented interface, i.e. how the mappings\n> are given to the system, will stay with us, and it is clear that it\n> was modeled after CURLOPT_RESOLVE---well, it was stolen from them\n> verbatim ;-).\n>\n> So we may wean ourselves off of cURL, but CURLOPT_RESOLVE will stay\n> with us for this particular feature.\n\nYeah, the CURLOPT_RESOLVE format will stay with us, so Ok, I will\nrename it \"http.curloptResolve\" in the next iteration then.\n\n> >> I am wondering if we want to mention the expected use case here\n> >> as well, something like\n> >>\n> >>     This is designed to be used primarily from the command line\n> >>     configuration variable override, e.g.\n> >>\n> >>         $ git -c http.resolve=example.com:443:127.0.0.1 \\\n> >>             clone https://example.com/user/project.git\n> >>\n> >> perhaps?  Not a suggestion, but soliciting thoughts.\n> >\n> > I am also interested in others' thoughts about this. If no one thinks\n> > that a config option could be useful, I am Ok with making it a\n> > \"--resolve\" command line option that can be passed to any Git command\n> > similar to \"-c <name>=<value>\":\n> >\n> > git --resolve=... <command> [<args>]\n>\n> Absolutely not.\n>\n> \"git [push|fetch|clone|ls-remote] --dns-pre-resolve=...\" that is\n> *NOT* git wide, but is only for transport commands might be a\n> possibility, but even then, you'd need to invent a way to do the\n> same for non cURL transports (we want to be able to pin the IP when\n> going over SSH to a certain host, for the same reason) if we promote\n> it to an officially supported command line option.\n\nOk with renaming and implementing it only in transport commands. I\ndon't want, and I don't think it should be necessary, to invent a way\nto do the same for non cURL transports though. I think it should be Ok\nwith the doc saying that the option has only been implemented for\nHTTP(S) yet and will have no effect when other transports are used.\n\nIf there is a simple way to do the same thing for ssh, then I might\ntake a look at it later. For \"file\" or bundle transports, I don't\nthink it makes sense, and the \"git\" transport is not used much in big\nhosting services where this feature is likely to be used.\n\n> Unless we do that, it is probably better to leave it as an obscure\n> configuration meant to help server operators.  At least, with the\n> name of the configuration variable prefixed with http.*, we have a\n> valid excuse when somebody complains \"the feature does not do\n> anything for git:// transport\".\n\nI am happy with leaving it as an obscure configuration meant to help\nserver operators. So I will just rename it \"http.curloptResolve\" in\nthe next iteration.\n\nThanks!\n"},{"id":"454851","messageId":"20220505111608.o63obxrifoqz6j5w@carlos-mbp.lan","threadId":"57830","inReplyTo":"CAP8UFD0V6scSD8LuUiRQ0BWmG2mFATVFNQ2DgwaWFp6bigBkZw@mail.gmail.com","subject":"Re: [PATCH] http: add custom hostname to IP address resolves","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-05-05T11:16:08Z","receivedAt":"2022-05-05T11:16:18Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, May 05, 2022 at 12:48:50PM +0200, Christian Couder wrote:\n> On Wed, May 4, 2022 at 4:34 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > >\n> > > I am also interested in others' thoughts about this. If no one thinks\n> > > that a config option could be useful, I am Ok with making it a\n> > > \"--resolve\" command line option that can be passed to any Git command\n> > > similar to \"-c <name>=<value>\":\n> > >\n> > > git --resolve=... <command> [<args>]\n> >\n> > Absolutely not.\n> >\n> > \"git [push|fetch|clone|ls-remote] --dns-pre-resolve=...\" that is\n> > *NOT* git wide, but is only for transport commands might be a\n> > possibility, but even then, you'd need to invent a way to do the\n> > same for non cURL transports (we want to be able to pin the IP when\n> > going over SSH to a certain host, for the same reason) if we promote\n> > it to an officially supported command line option.\n> \n> Ok with renaming and implementing it only in transport commands. I\n> don't want, and I don't think it should be necessary, to invent a way\n> to do the same for non cURL transports though. I think it should be Ok\n> with the doc saying that the option has only been implemented for\n> HTTP(S) yet and will have no effect when other transports are used.\n\nI think it will be better if git aborts with an error if it is used for\na transport that it doesn't support, instead of relying in the documentation,\nthough.\n \n> If there is a simple way to do the same thing for ssh, then I might\n> take a look at it later. For \"file\" or bundle transports, I don't\n> think it makes sense, and the \"git\" transport is not used much in big\n> hosting services where this feature is likely to be used.\n\nThis seems definitely useful also for ssh which is also used in big\nhosting services.\n\nIronically, I think would be even more useful for the \"git\" transport\nspecially because it doesn't have other protections to rely on that\nwould help prevent spoofing (like TLS), which might be also why it is\nnot that widely used anymore.\n\nCarlo\n"},{"id":"454852","messageId":"20220505112114.ktki3dgjbyukzhlm@carlos-mbp.lan","threadId":"57830","inReplyTo":"20220504104601.136403-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v2] http: add custom hostname to IP address resolutions","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-05-05T11:21:14Z","receivedAt":"2022-05-05T11:21:27Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Wed, May 04, 2022 at 12:46:01PM +0200, Christian Couder wrote:\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index f92c79c132..d97380be87 100755\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n>  \tgrep symref=HEAD:refs/heads/ trace\n>  '\n>  \n> +test_expect_success 'passing hostname resolution information works' '\n> +\tBOGUS_HOST=gitbogusexamplehost.com &&\n\nminor nitpick, but better to use example.com here which is reserved for this\ntype of uses and therefore unlikely to conflict with a possibly assigned\ndomain.\n\nCarlo\n"},{"id":"455009","messageId":"20220509153834.485871-1-chriscool@tuxfamily.org","threadId":"57830","inReplyTo":"20220504104601.136403-1-chriscool@tuxfamily.org","subject":"[PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-09T15:38:34Z","receivedAt":"2022-05-09T15:38:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Libcurl has a CURLOPT_RESOLVE easy option that allows\nthe result of hostname resolution in the following\nformat to be passed:\n\n\t[+]HOST:PORT:ADDRESS[,ADDRESS]\n\nThis way, redirects and everything operating against the\nHOST+PORT will use the provided ADDRESS(s).\n\nThe following format is also allowed to stop using\nhostname resolutions that have already been passed:\n\n\t-HOST:PORT\n\nSee https://curl.se/libcurl/c/CURLOPT_RESOLVE.html for\nmore details.\n\nLet's add a corresponding \"http.curloptResolve\" config\noption that takes advantage of CURLOPT_RESOLVE.\n\nEach value configured for the \"http.curloptResolve\" key\nis passed \"as is\" to libcurl through CURLOPT_RESOLVE, so\nit should be in one of the above 2 formats. This keeps\nthe implementation simple and makes us consistent with\nlibcurl's CURLOPT_RESOLVE, and with curl's corresponding\n`--resolve` command line option.\n\nThe implementation uses CURLOPT_RESOLVE only in\nget_active_slot() which is called by all the HTTP\nrequest sending functions.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n\nChanges since v2 are the following:\n\n  - option renamed from \"http.resolve\" to \"http.curloptResolve\"\n  - mention \"libcurl\" instead of \"curl\" in the commit message\n  - mention \"libculr\" in the doc to make it easier to understand\n    the new option name\n\nRange diff:\n\n1:  28c3bf9d02 ! 1:  3d689f8a6f http: add custom hostname to IP address resolutions\n    @@ Commit message\n         See https://curl.se/libcurl/c/CURLOPT_RESOLVE.html for\n         more details.\n     \n    -    Let's add a corresponding \"http.resolve\" config option\n    -    that takes advantage of CURLOPT_RESOLVE.\n    +    Let's add a corresponding \"http.curloptResolve\" config\n    +    option that takes advantage of CURLOPT_RESOLVE.\n     \n    -    Each value configured for the \"http.resolve\" key is\n    -    passed \"as is\" to curl through CURLOPT_RESOLVE, so it\n    -    should be in one of the above 2 formats. This keeps the\n    -    implementation simple and makes us consistent with\n    +    Each value configured for the \"http.curloptResolve\" key\n    +    is passed \"as is\" to libcurl through CURLOPT_RESOLVE, so\n    +    it should be in one of the above 2 formats. This keeps\n    +    the implementation simple and makes us consistent with\n         libcurl's CURLOPT_RESOLVE, and with curl's corresponding\n         `--resolve` command line option.\n     \n    @@ Documentation/config/http.txt: http.version::\n        - HTTP/2\n        - HTTP/1.1\n      \n    -+http.resolve::\n    -+  Hostname resolution information that will be used first when sending\n    -+  HTTP requests.  This information should be in one of the following\n    -+  formats:\n    ++http.curloptResolve::\n    ++  Hostname resolution information that will be used first by\n    ++  libcurl when sending HTTP requests.  This information should\n    ++  be in one of the following formats:\n     +\n     +  - [+]HOST:PORT:ADDRESS[,ADDRESS]\n     +  - -HOST:PORT\n    @@ http.c: static int http_options(const char *var, const char *value, void *cb)\n                return 0;\n        }\n      \n    -+  if (!strcmp(\"http.resolve\", var)) {\n    ++  if (!strcmp(\"http.curloptresolve\", var)) {\n     +          if (!value) {\n     +                  return config_error_nonbool(var);\n     +          } else if (!*value) {\n    @@ t/t5551-http-fetch-smart.sh: test_expect_success 'client falls back from v2 to v\n     +  BOGUS_HOST=gitbogusexamplehost.com &&\n     +  BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n     +  test_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n    -+  git -c \"http.resolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n    ++  git -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n     +'\n     +\n      test_done\n\n\n Documentation/config/http.txt | 16 ++++++++++++++++\n http.c                        | 18 ++++++++++++++++++\n t/t5551-http-fetch-smart.sh   |  7 +++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/config/http.txt b/Documentation/config/http.txt\nindex 7003661c0d..179d03e57b 100644\n--- a/Documentation/config/http.txt\n+++ b/Documentation/config/http.txt\n@@ -98,6 +98,22 @@ http.version::\n \t- HTTP/2\n \t- HTTP/1.1\n \n+http.curloptResolve::\n+\tHostname resolution information that will be used first by\n+\tlibcurl when sending HTTP requests.  This information should\n+\tbe in one of the following formats:\n+\n+\t- [+]HOST:PORT:ADDRESS[,ADDRESS]\n+\t- -HOST:PORT\n+\n++\n+The first format redirects all requests to the given `HOST:PORT`\n+to the provided `ADDRESS`(s). The second format clears all\n+previous config values for that `HOST:PORT` combination.  To\n+allow easy overriding of all the settings inherited from the\n+system config, an empty value will reset all resolution\n+information to the empty list.\n+\n http.sslVersion::\n \tThe SSL version to use when negotiating an SSL connection, if you\n \twant to force the default.  The available and default version\ndiff --git a/http.c b/http.c\nindex 229da4d148..8beacb95cc 100644\n--- a/http.c\n+++ b/http.c\n@@ -128,6 +128,8 @@ static struct curl_slist *pragma_header;\n static struct curl_slist *no_pragma_header;\n static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;\n \n+static struct curl_slist *host_resolutions;\n+\n static struct active_request_slot *active_queue_head;\n \n static char *cached_accept_language;\n@@ -393,6 +395,18 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.curloptresolve\", var)) {\n+\t\tif (!value) {\n+\t\t\treturn config_error_nonbool(var);\n+\t\t} else if (!*value) {\n+\t\t\tcurl_slist_free_all(host_resolutions);\n+\t\t\thost_resolutions = NULL;\n+\t\t} else {\n+\t\t\thost_resolutions = curl_slist_append(host_resolutions, value);\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.followredirects\", var)) {\n \t\tif (value && !strcmp(value, \"initial\"))\n \t\t\thttp_follow_config = HTTP_FOLLOW_INITIAL;\n@@ -1131,6 +1145,9 @@ void http_cleanup(void)\n \tcurl_slist_free_all(no_pragma_header);\n \tno_pragma_header = NULL;\n \n+\tcurl_slist_free_all(host_resolutions);\n+\thost_resolutions = NULL;\n+\n \tif (curl_http_proxy) {\n \t\tfree((void *)curl_http_proxy);\n \t\tcurl_http_proxy = NULL;\n@@ -1211,6 +1228,7 @@ struct active_request_slot *get_active_slot(void)\n \tif (curl_save_cookies)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_COOKIEJAR, curl_cookie_file);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_RESOLVE, host_resolutions);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);\n \tcurl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, NULL);\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex f92c79c132..4a8dbb7eee 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n \tgrep symref=HEAD:refs/heads/ trace\n '\n \n+test_expect_success 'passing hostname resolution information works' '\n+\tBOGUS_HOST=gitbogusexamplehost.com &&\n+\tBOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n+\ttest_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n+\tgit -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n+'\n+\n test_done\n-- \n2.36.1.20.g51170867bd\n\n"},{"id":"455010","messageId":"CAP8UFD2BTmeYuVsq2AJg32qEjPXXB5oGtU1i8KK=xjZrS39eZA@mail.gmail.com","threadId":"57830","inReplyTo":"20220505111608.o63obxrifoqz6j5w@carlos-mbp.lan","subject":"Re: [PATCH] http: add custom hostname to IP address resolves","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-09T15:40:03Z","receivedAt":"2022-05-09T15:40:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, May 5, 2022 at 1:16 PM Carlo Marcelo Arenas Belón\n<carenas@gmail.com> wrote:\n> On Thu, May 05, 2022 at 12:48:50PM +0200, Christian Couder wrote:\n\n> > Ok with renaming and implementing it only in transport commands. I\n> > don't want, and I don't think it should be necessary, to invent a way\n> > to do the same for non cURL transports though. I think it should be Ok\n> > with the doc saying that the option has only been implemented for\n> > HTTP(S) yet and will have no effect when other transports are used.\n>\n> I think it will be better if git aborts with an error if it is used for\n> a transport that it doesn't support, instead of relying in the documentation,\n> though.\n\nThis has drawbacks, as this is likely to be called through scripts or\napps and the calling code would have to be a bit more complex as\npassing the option couldn't be done independently of the transport/URL\nused.\n\nAnyway until others complain, I prefer to leave it as an obscure\nconfiguration called \"http.curloptResolve\" and meant to help server\noperators as Junio says. This avoids wondering about such issues.\n\n> > If there is a simple way to do the same thing for ssh, then I might\n> > take a look at it later. For \"file\" or bundle transports, I don't\n> > think it makes sense, and the \"git\" transport is not used much in big\n> > hosting services where this feature is likely to be used.\n>\n> This seems definitely useful also for ssh which is also used in big\n> hosting services.\n\nI am not sure it is as useful for ssh. And maybe someone will take a\nlook at implementing it if that's the case.\n\n> Ironically, I think would be even more useful for the \"git\" transport\n> specially because it doesn't have other protections to rely on that\n> would help prevent spoofing (like TLS), which might be also why it is\n> not that widely used anymore.\n\nYeah, more secure alternatives have taken over for good.\n"},{"id":"455098","messageId":"CAPUEsphA=q10wCsrf3AxR9fXz9HQHt374tDFoWBu++EPNDA-LA@mail.gmail.com","threadId":"57830","inReplyTo":"20220509153834.485871-1-chriscool@tuxfamily.org","subject":"Re: [PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-05-10T18:20:41Z","receivedAt":"2022-05-10T18:20:59Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, May 9, 2022 at 8:38 AM Christian Couder\n<christian.couder@gmail.com> wrote:\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index f92c79c132..4a8dbb7eee 100755\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n>         grep symref=HEAD:refs/heads/ trace\n>  '\n>\n> +test_expect_success 'passing hostname resolution information works' '\n> +       BOGUS_HOST=gitbogusexamplehost.com &&\n> +       BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n> +       test_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n> +       git -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n> +'\n\nIs setting it up as a command line config option the way you expect to\nuse this, and if so why not make it a full blown command line option\nwith the previous caveats that were discussed before?\n\nI also think it might be a little confusing (and probably warranted of\nan advice message) if git will decide based on a configuration\nsomewhere in its resolution tree that the IP I am connecting is\ndifferent than the one I expect it to use through the system\nconfigured resolution mechanism for such a thing.\n\nI assume that if you want to use this frequently, having that advice\ndisabled in your global config wouldn't be a hassle, but it might be\nuseful to know that I am interacting with a potentially different IP\nwhen referring to some host by name in my local repo, maybe because I\nforgot to change that setting after some debugging.\n\nI am sure all those folks that forget to edit their /etc/hosts after\nthey are done with their local site versions might instead use this\nand then be happy to be warned about it later.\n\nCarlo\n"},{"id":"455190","messageId":"CAP8UFD2F=5RsB_WVOHKmzJsD9UJgZ2YM_yreTur2_huAAJ=+Vw@mail.gmail.com","threadId":"57830","inReplyTo":"CAPUEsphA=q10wCsrf3AxR9fXz9HQHt374tDFoWBu++EPNDA-LA@mail.gmail.com","subject":"Re: [PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-12T08:29:59Z","receivedAt":"2022-05-12T08:30:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, May 10, 2022 at 8:20 PM Carlo Arenas <carenas@gmail.com> wrote:\n>\n> On Mon, May 9, 2022 at 8:38 AM Christian Couder\n> <christian.couder@gmail.com> wrote:\n> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> > index f92c79c132..4a8dbb7eee 100755\n> > --- a/t/t5551-http-fetch-smart.sh\n> > +++ b/t/t5551-http-fetch-smart.sh\n> > @@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n> >         grep symref=HEAD:refs/heads/ trace\n> >  '\n> >\n> > +test_expect_success 'passing hostname resolution information works' '\n> > +       BOGUS_HOST=gitbogusexamplehost.com &&\n> > +       BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n> > +       test_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n> > +       git -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n> > +'\n>\n> Is setting it up as a command line config option the way you expect to\n> use this, and if so why not make it a full blown command line option\n> with the previous caveats that were discussed before?\n\nYeah, it's how GitLab will likely use this, but this is the same for\nmost (if not all) config options these days in GitLab. So I don't\nthink it's a good criteria.\n\nI already talked about it, but one of the issues with a command line\noption is that such an option might not be worth implementing for SSH\n(which might not need it) or other protocols for different reasons. So\nwe would have a CLI option with probably a generic name that would\nactually work only with one (or a few) protocols, and we would need to\ndecide what to do in case this option is used along with a protocol\nthat it doesn't support.\n\n> I also think it might be a little confusing (and probably warranted of\n> an advice message) if git will decide based on a configuration\n> somewhere in its resolution tree that the IP I am connecting is\n> different than the one I expect it to use through the system\n> configured resolution mechanism for such a thing.\n\nI would be Ok to add an advice message or another kind of message\ntelling users that the IP used is based on the config variable. It\ncould break scripts parsing Git's output though (even if it's bad\npractice to do so). So we would need to decide the kind of message and\nits content. Suggestions welcome.\n\n> I assume that if you want to use this frequently, having that advice\n> disabled in your global config wouldn't be a hassle, but it might be\n> useful to know that I am interacting with a potentially different IP\n> when referring to some host by name in my local repo, maybe because I\n> forgot to change that setting after some debugging.\n\nYeah, maybe. On the other hand GIT_CURL_VERBOSE might already be the\ncanonical way to debug this and might already tell about this.\n\nYeah, it does:\n\n<= Recv header:\n== Info: Connection #0 to host gitbogusexamplehost.com left intact\n== Info: RESOLVE gitbogusexamplehost.com:5551 is - old addresses discarded!\n== Info: Added gitbogusexamplehost.com:5551:127.0.0.1 to DNS cache\n== Info: Couldn't find host gitbogusexamplehost.com in the .netrc\nfile; using defaults\n== Info: Found bundle for host gitbogusexamplehost.com: 0x5556d2bd1340\n[serially]\n== Info: Can not multiplex, even if we wanted to!\n== Info: Re-using existing connection! (#0) with host gitbogusexamplehost.com\n== Info: Connected to gitbogusexamplehost.com (127.0.0.1) port 5551 (#0)\n\nI agree it might not be very clear that it's because\n\"http.curloptResolve\" is used though. But maybe we could output a more\nexplicit warning message only if GIT_CURL_VERBOSE is set.\n\n> I am sure all those folks that forget to edit their /etc/hosts after\n> they are done with their local site versions might instead use this\n> and then be happy to be warned about it later.\n\nDo you mean that those folks might like a config option ;-)\n"},{"id":"455191","messageId":"CAP8UFD3UvjJzEQApiCKSknDWaTjbEMKkr17j1QN-8gwz7eWV3Q@mail.gmail.com","threadId":"57830","inReplyTo":"20220505112114.ktki3dgjbyukzhlm@carlos-mbp.lan","subject":"Re: [PATCH v2] http: add custom hostname to IP address resolutions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-12T08:52:59Z","receivedAt":"2022-05-12T08:54:21Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, May 5, 2022 at 1:21 PM Carlo Marcelo Arenas Belón\n<carenas@gmail.com> wrote:\n>\n> On Wed, May 04, 2022 at 12:46:01PM +0200, Christian Couder wrote:\n> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> > index f92c79c132..d97380be87 100755\n> > --- a/t/t5551-http-fetch-smart.sh\n> > +++ b/t/t5551-http-fetch-smart.sh\n> > @@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n> >       grep symref=HEAD:refs/heads/ trace\n> >  '\n> >\n> > +test_expect_success 'passing hostname resolution information works' '\n> > +     BOGUS_HOST=gitbogusexamplehost.com &&\n>\n> minor nitpick, but better to use example.com here which is reserved for this\n> type of uses and therefore unlikely to conflict with a possibly assigned\n> domain.\n\nSorry for not replying to this earlier, but Junio previously suggested\nthe following:\n\n\"Perhaps invent a totally bogus domain name, map that to\nlocalhost ::1, run a test server locally, and try to clone from that\nbogus domain?\"\n\n(See: https://lore.kernel.org/git/xmqqfslrycvp.fsf@gitster.g/)\n\nI think \"a totally bogus domain name\" refers to something other than\n\"example.com\".\n\nAlso \"example.com\" does seem to resolve to an IP address and even has\nan HTTP(S) server on it, while I think the purpose of the test would\nbe to check that there is not even a valid DNS resolution when the new\noption is not used.\n"},{"id":"455192","messageId":"CAPUEsphmN+1FVdQSX3GoRYQBh33XdR+nN-WAPZdtZmMsBAfazg@mail.gmail.com","threadId":"57830","inReplyTo":"CAP8UFD2F=5RsB_WVOHKmzJsD9UJgZ2YM_yreTur2_huAAJ=+Vw@mail.gmail.com","subject":"Re: [PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-05-12T11:55:57Z","receivedAt":"2022-05-12T11:56:20Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, May 12, 2022 at 1:30 AM Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Tue, May 10, 2022 at 8:20 PM Carlo Arenas <carenas@gmail.com> wrote:\n> > I also think it might be a little confusing (and probably warranted of\n> > an advice message) if git will decide based on a configuration\n> > somewhere in its resolution tree that the IP I am connecting is\n> > different than the one I expect it to use through the system\n> > configured resolution mechanism for such a thing.\n>\n> I would be Ok to add an advice message or another kind of message\n> telling users that the IP used is based on the config variable. It\n> could break scripts parsing Git's output though (even if it's bad\n> practice to do so).\n\nOnly if they added that config option, which is an obscure one that\nnobody should be using anyway, so very unlikely, right?\nI also think that breaking my script could be avoided by turning off\nthe advice (as suggested previously), and I MIGHT want to have my\nscript broken if I picked up this config by mistake.\n\n> So we would need to decide the kind of message and\n> its content. Suggestions welcome.\n\nI am not good at wording those, but I would think something innocuous like :\n\n  \"curl override detected to point name %s to %s instead of using DNS\"\n\n> > I assume that if you want to use this frequently, having that advice\n> > disabled in your global config wouldn't be a hassle, but it might be\n> > useful to know that I am interacting with a potentially different IP\n> > when referring to some host by name in my local repo, maybe because I\n> > forgot to change that setting after some debugging.\n>\n> Yeah, maybe. On the other hand GIT_CURL_VERBOSE might already be the\n> canonical way to debug this and might already tell about this.\n\nof course, but that is mostly used when debugging HTTP issues, not\nwhen your DNS seems to have gone nuts, and you are looking at your\nscreen in disbelief because the code you were working on before lunch\nand having released is now suddenly in production.\n\n> Yeah, it does:\n>\n> <= Recv header:\n> == Info: Connection #0 to host gitbogusexamplehost.com left intact\n> == Info: RESOLVE gitbogusexamplehost.com:5551 is - old addresses discarded!\n> == Info: Added gitbogusexamplehost.com:5551:127.0.0.1 to DNS cache\n> == Info: Couldn't find host gitbogusexamplehost.com in the .netrc\n> file; using defaults\n> == Info: Found bundle for host gitbogusexamplehost.com: 0x5556d2bd1340\n> [serially]\n> == Info: Can not multiplex, even if we wanted to!\n> == Info: Re-using existing connection! (#0) with host gitbogusexamplehost.com\n> == Info: Connected to gitbogusexamplehost.com (127.0.0.1) port 5551 (#0)\n>\n> I agree it might not be very clear that it's because\n> \"http.curloptResolve\" is used though. But maybe we could output a more\n> explicit warning message only if GIT_CURL_VERBOSE is set.\n\nAs I pointed in my example I can see this being useful also for some\nweb developing which are mostly concerned about JavaScript and might\nnot be aware of the OSI layer stack and how to get an HTTP packet\ndump, so I would think better done even when GIT_CURL_VERBOSE is not\nset.\n\n> > I am sure all those folks that forget to edit their /etc/hosts after\n> > they are done with their local site versions might instead use this\n> > and then be happy to be warned about it later.\n>\n> Do you mean that those folks might like a config option ;-)\n\nYes, and I think it is not THAT of an obscure config option if it even\nhas documentation as well ;)\n\nCarlo\n"},{"id":"455194","messageId":"Yn0FPkoUNacvctAp@ncase","threadId":"57830","inReplyTo":"CAPUEsphA=q10wCsrf3AxR9fXz9HQHt374tDFoWBu++EPNDA-LA@mail.gmail.com","subject":"Re: [PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2022-05-12T13:01:50Z","receivedAt":"2022-05-12T13:02:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, May 10, 2022 at 11:20:41AM -0700, Carlo Arenas wrote:\n> On Mon, May 9, 2022 at 8:38 AM Christian Couder\n> <christian.couder@gmail.com> wrote:\n> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> > index f92c79c132..4a8dbb7eee 100755\n> > --- a/t/t5551-http-fetch-smart.sh\n> > +++ b/t/t5551-http-fetch-smart.sh\n> > @@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n> >         grep symref=HEAD:refs/heads/ trace\n> >  '\n> >\n> > +test_expect_success 'passing hostname resolution information works' '\n> > +       BOGUS_HOST=gitbogusexamplehost.com &&\n> > +       BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n> > +       test_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n> > +       git -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n> > +'\n> \n> Is setting it up as a command line config option the way you expect to\n> use this, and if so why not make it a full blown command line option\n> with the previous caveats that were discussed before?\n\nIf you did this as a command-line option, you'd now be forced to add it\nto every single command you want to support this: git-fetch, git-pull,\ngit-remote, git-ls-remote and maybe others I forgot about. On the other\nhand, by having this as a configuration variable in `http.c` all of\nthose commands benefit the same.\n\nFurthermore, using a config option is a lot more flexible: you can\npersist it at different levels of your gitconfig, can easily inject it\nin a script via the use of environment variables, or directly override\nit when spawning a command with `-c`.\n\nOverall, I think it is preferable to keep this as an option as opposed\nto adding such an obscure parameter to all of the commands.\n\n> I also think it might be a little confusing (and probably warranted of\n> an advice message) if git will decide based on a configuration\n> somewhere in its resolution tree that the IP I am connecting is\n> different than the one I expect it to use through the system\n> configured resolution mechanism for such a thing.\n\nThat's true already though, isn't it? A user may set `url.*.insteadOf`\nand be surprised at a later point that their URLs are getting redirected\nsomewhere else. And there's probably a lot more examples where a user\nmay be confused when forgetting about certain configuration variables\nthat change the way Git behaves.\n\nI also don't think that using an advise here would be ideal. The main\nuse case of this configuration variable is going to be servers, and\nthere is a high chance that they might actually be parsing output of any\nsuch commands. Forcing them to always disable this advise doesn't feel\nlike the right thing to do.\n\nPatrick\n\n> I assume that if you want to use this frequently, having that advice\n> disabled in your global config wouldn't be a hassle, but it might be\n> useful to know that I am interacting with a potentially different IP\n> when referring to some host by name in my local repo, maybe because I\n> forgot to change that setting after some debugging.\n> \n> I am sure all those folks that forget to edit their /etc/hosts after\n> they are done with their local site versions might instead use this\n> and then be happy to be warned about it later.\n> \n> Carlo\n"},{"id":"455196","messageId":"CAPUEsphh_3oFkfAdB-mZ9ZH7F0tCm_iwapy8=6VFE45yWoOK3w@mail.gmail.com","threadId":"57830","inReplyTo":"Yn0FPkoUNacvctAp@ncase","subject":"Re: [PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2022-05-12T13:56:24Z","receivedAt":"2022-05-12T13:56:53Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On thu, May 12, 2022 at 6:01 AM Patrick Steinhardt <ps@pks.im> wrote:\n> On Tue, May 10, 2022 at 11:20:41AM -0700, Carlo Arenas wrote:\n> > On Mon, May 9, 2022 at 8:38 AM Christian Couder\n> > <christian.couder@gmail.com> wrote:\n> > > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> > > index f92c79c132..4a8dbb7eee 100755\n> > > --- a/t/t5551-http-fetch-smart.sh\n> > > +++ b/t/t5551-http-fetch-smart.sh\n> > > @@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n> > >         grep symref=HEAD:refs/heads/ trace\n> > >  '\n> > >\n> > > +test_expect_success 'passing hostname resolution information works' '\n> > > +       BOGUS_HOST=gitbogusexamplehost.com &&\n> > > +       BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n> > > +       test_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n> > > +       git -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n> > > +'\n> >\n> > Is setting it up as a command line config option the way you expect to\n> > use this, and if so why not make it a full blown command line option\n> > with the previous caveats that were discussed before?\n>\n> If you did this as a command-line option, you'd now be forced to add it\n> to every single command you want to support this: git-fetch, git-pull,\n> git-remote, git-ls-remote and maybe others I forgot about. On the other\n> hand, by having this as a configuration variable in `http.c` all of\n> those commands benefit the same.\n\nThere are ways to add common options to all commands that would help\nhere, but as Junio pointed out it is not ideal because then you have\nto ALSO provide implementations, which you don't seem interested in\ndoing.\n\n> Furthermore, using a config option is a lot more flexible: you can\n> persist it at different levels of your gitconfig, can easily inject it\n> in a script via the use of environment variables, or directly override\n> it when spawning a command with `-c`.\n>\n> Overall, I think it is preferable to keep this as an option as opposed\n> to adding such an obscure parameter to all of the commands.\n\nI think we had already decided that a config is more flexible, even if\nI personally don't agree.\n\n> > I also think it might be a little confusing (and probably warranted of\n> > an advice message) if git will decide based on a configuration\n> > somewhere in its resolution tree that the IP I am connecting is\n> > different than the one I expect it to use through the system\n> > configured resolution mechanism for such a thing.\n>\n> That's true already though, isn't it? A user may set `url.*.insteadOf`\n> and be surprised at a later point that their URLs are getting redirected\n> somewhere else. And there's probably a lot more examples where a user\n> may be confused when forgetting about certain configuration variables\n> that change the way Git behaves.\n\nThat is a good point, but unlike url.*.insteadOf, this is meant to be\nan obscure setting that shouldn't be enabled by default (or under\ncommon circumstances), so having the advice there is helpful for when\nwe find ourselves in an unexpected situation and to avoid confusion.\n\nI would even argue YOUR use of it in a server might even benefit from\nthis advice, because it could be strange to get a different IP than\nthe one you set in the command line if there is also another entry in\nsome config that you happened to read.\n\n> I also don't think that using an advise here would be ideal. The main\n> use case of this configuration variable is going to be servers.\n\nMy feedback about servers is indeed below, so I won't repeat myself\nbut keeping a global config that has this advice disabled in a server\nshouldn't be that difficult; indeed it MIGHT be already there since\nmost features that are meant for interactive users (like an advice)\nare better disabled in servers.\n\n> > I assume that if you want to use this frequently, having that advice\n> > disabled in your global config wouldn't be a hassle, but it might be\n> > useful to know that I am interacting with a potentially different IP\n> > when referring to some host by name in my local repo, maybe because I\n> > forgot to change that setting after some debugging.\n> >\n> > I am sure all those folks that forget to edit their /etc/hosts after\n> > they are done with their local site versions might instead use this\n> > and then be happy to be warned about it later.\n> >\n> > Carlo\n"},{"id":"455209","messageId":"xmqqa6bmhhe0.fsf@gitster.g","threadId":"57830","inReplyTo":"Yn0FPkoUNacvctAp@ncase","subject":"Re: [PATCH v3] http: add custom hostname to IP address resolutions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T15:58:47Z","receivedAt":"2022-05-12T15:58:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> Is setting it up as a command line config option the way you expect to\n>> use this, and if so why not make it a full blown command line option\n>> with the previous caveats that were discussed before?\n>\n> If you did this as a command-line option, you'd now be forced to add it\n> to every single command you want to support this: git-fetch, git-pull,\n> git-remote, git-ls-remote and maybe others I forgot about. On the other\n> hand, by having this as a configuration variable in `http.c` all of\n> those commands benefit the same.\n\nIt is not an argument against the command line option that you find\nit more work to implement and cumbersome to plumb the information\nthrough the callgraph, though.\n\nSubcommands like \"git commit\" shouldn't have to know how host names\nare mapped to IP addresses, and teaching the option only to\nsubcommands that the feature is relevant, and documenting the option\nin their manual pages, would make it much easier to discover and\nlearn.\n\n> Overall, I think it is preferable to keep this as an option as opposed\n> to adding such an obscure parameter to all of the commands.\n\nI favor implementing this as a configuration that is primarily meant\nto be used from the command line (i.e. \"git -c var=val\"), ONLY\nBECAUSE the feature itself is not something that should be widely\nused (the users should futz with their DNS if they need something\nmore permanent), and adding it as a configuration would be a more\nquick and dirty way that needs less developer resources now ;-)\n\nTo purists, it may make more sense to add this feature and make it\naccessible only from the command line without matching configuration\nvariable---that would enforce the assumed use case (i.e. only after\nanother part of the system asked DNS, performed some check on the\nresulting IP address, and decided to ask Git to interact with that\nURL, use this mechanism to ensure Git interacts with that IP address\nthat was vetted, to avoid TOCTOU mistakes) more clearly.\n\nI am personally open to such a purer counterproposal with working\ncode ;-)\n\n>> I also think it might be a little confusing (and probably warranted of\n>> an advice message) if git will decide based on a configuration\n>> somewhere in its resolution tree that the IP I am connecting is\n>> different than the one I expect it to use through the system\n>> configured resolution mechanism for such a thing.\n>\n> That's true already though, isn't it? A user may set `url.*.insteadOf`\n> and be surprised at a later point that their URLs are getting redirected\n> somewhere else. And there's probably a lot more examples where a user\n> may be confused when forgetting about certain configuration variables\n> that change the way Git behaves.\n>\n> I also don't think that using an advise here would be ideal. The main\n> use case of this configuration variable is going to be servers, and\n> there is a high chance that they might actually be parsing output of any\n> such commands. Forcing them to always disable this advise doesn't feel\n> like the right thing to do.\n\nAll correct.  If the users set configuration variables, the fact\nthat we honored their configuration variables settings and behaved\naccordingly is *NOT* an advise-worthy event.\n\nThanks.\n"},{"id":"455211","messageId":"xmqqzgjmg1q5.fsf@gitster.g","threadId":"57830","inReplyTo":"CAP8UFD3UvjJzEQApiCKSknDWaTjbEMKkr17j1QN-8gwz7eWV3Q@mail.gmail.com","subject":"Re: [PATCH v2] http: add custom hostname to IP address resolutions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T16:22:26Z","receivedAt":"2022-05-12T16:22:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> \"Perhaps invent a totally bogus domain name, map that to\n> localhost ::1, run a test server locally, and try to clone from that\n> bogus domain?\"\n>\n> (See: https://lore.kernel.org/git/xmqqfslrycvp.fsf@gitster.g/)\n>\n> I think \"a totally bogus domain name\" refers to something other than\n> \"example.com\".\n\nI meant a domain that should not be used for purposes other than\nbeing examples in the real world, including \"example.com\".\n\nBut RFC6761, which is an update to RFC2606, describes a set of\nproperties that make .invalid nice domain to use, including:\n\n 1.  Users are free to use \"invalid\" names as they would any other\n     domain names.  Users MAY assume that queries for \"invalid\"\n     names will always return NXDOMAIN responses.\n\n 3.  Name resolution APIs and libraries SHOULD recognize \"invalid\"\n     names as special and SHOULD always return immediate negative\n     responses.  Name resolution APIs SHOULD NOT send queries for\n     \"invalid\" names to their configured caching DNS server(s).\n       \nAnother possibility is \".test\" but it is more for testing DNS, not\napplication, i.e.\n\n 1.  Users are free to use these test names as they would any other\n     domain names.  However, since there is no central authority\n     responsible for use of test names, users SHOULD be aware that\n     these names are likely to yield different results on different\n     networks.\n\n 3.  Name resolution APIs and libraries SHOULD NOT recognize test\n     names as special and SHOULD NOT treat them differently.  Name\n     resolution APIs SHOULD send queries for test names to their\n     configured caching DNS server(s).\n\nso for a code like what we are discussing, which would not want the\nnames to be shown to DNS and yield any IP address, \".test\" makes a\npoorer \"bogus domain name\" than \".invalid\", I think.\n\nBy the way, we seem to have references to .xz top-level domain,\nwhich appeared only in earlier drafts of what became RFC2606 (which\nwas updated by RFC6761) in both documentation pages and tests.  At\nsome point we may want to update the former to \".example\" and the\nlatter to \".invalid\" as a clean-up.\n\n> Also \"example.com\" does seem to resolve to an IP address and even has\n> an HTTP(S) server on it, while I think the purpose of the test would\n> be to check that there is not even a valid DNS resolution when the new\n> option is not used.\n\nYup, that makes \".invalid\" a better candidate, I think.\n\nThanks.\n\n"},{"id":"455214","messageId":"CAP8UFD2tk7ti_G0Ao_-u0g9+hm-x4gnuSjEPHbnKPan6S741Ug@mail.gmail.com","threadId":"57830","inReplyTo":"xmqqzgjmg1q5.fsf@gitster.g","subject":"Re: [PATCH v2] http: add custom hostname to IP address resolutions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-12T18:57:50Z","receivedAt":"2022-05-12T18:58:06Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, May 12, 2022 at 6:22 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n> > \"Perhaps invent a totally bogus domain name, map that to\n> > localhost ::1, run a test server locally, and try to clone from that\n> > bogus domain?\"\n> >\n> > (See: https://lore.kernel.org/git/xmqqfslrycvp.fsf@gitster.g/)\n> >\n> > I think \"a totally bogus domain name\" refers to something other than\n> > \"example.com\".\n>\n> I meant a domain that should not be used for purposes other than\n> being examples in the real world, including \"example.com\".\n\nOk, thanks for the clarification and for copying the relevant RFC\ninformation below.\n\n> But RFC6761, which is an update to RFC2606, describes a set of\n> properties that make .invalid nice domain to use, including:\n>\n>  1.  Users are free to use \"invalid\" names as they would any other\n>      domain names.  Users MAY assume that queries for \"invalid\"\n>      names will always return NXDOMAIN responses.\n>\n>  3.  Name resolution APIs and libraries SHOULD recognize \"invalid\"\n>      names as special and SHOULD always return immediate negative\n>      responses.  Name resolution APIs SHOULD NOT send queries for\n>      \"invalid\" names to their configured caching DNS server(s).\n\nI wonder if libcurl considers itself as a name resolution library or\nnot. It has a DNS cache, so maybe in some ways it is. Also however it\nconsiders itself now, it could perhaps change in the future. Even if\nthe current developers are against such a change, a new RFC might be\nmore precise and specify something for libraries like libcurl which\ncould make it change.\n\nSo I am not so sure that using \"invalid\" is our best bet.\n\n> Another possibility is \".test\" but it is more for testing DNS, not\n> application, i.e.\n\nIn a way we are testing DNS, as we are actually testing libcurl's DNS\ncaching and its CURLOPT_RESOLVE option (even if we also test that Git\nis correctly passing the config option to libcurl at the same time).\n\n>  1.  Users are free to use these test names as they would any other\n>      domain names.  However, since there is no central authority\n>      responsible for use of test names, users SHOULD be aware that\n>      these names are likely to yield different results on different\n>      networks.\n>\n>  3.  Name resolution APIs and libraries SHOULD NOT recognize test\n>      names as special and SHOULD NOT treat them differently.  Name\n>      resolution APIs SHOULD send queries for test names to their\n>      configured caching DNS server(s).\n\nSo with this we can at least expect that the way libcurl considers\nitself will have no impact on our tests.\n\n> so for a code like what we are discussing, which would not want the\n> names to be shown to DNS and yield any IP address, \".test\" makes a\n> poorer \"bogus domain name\" than \".invalid\", I think.\n\nI would think that there are risks in both cases. I am Ok with using\nany of the following in the test:\n\nBOGUS_HOST=gitbogusexamplehost.invalid # or\nBOGUS_HOST=gitbogusexamplehost.test\n\nThe test passes for me either way.\n\n> By the way, we seem to have references to .xz top-level domain,\n> which appeared only in earlier drafts of what became RFC2606 (which\n> was updated by RFC6761) in both documentation pages and tests.  At\n> some point we may want to update the former to \".example\" and the\n> latter to \".invalid\" as a clean-up.\n\nYeah, good idea.\n\n> > Also \"example.com\" does seem to resolve to an IP address and even has\n> > an HTTP(S) server on it, while I think the purpose of the test would\n> > be to check that there is not even a valid DNS resolution when the new\n> > option is not used.\n>\n> Yup, that makes \".invalid\" a better candidate, I think.\n\nOk, I will use \"gitbogusexamplehost.invalid\" in the next iteration then.\n"},{"id":"455313","messageId":"20220516083851.202057-1-chriscool@tuxfamily.org","threadId":"57830","inReplyTo":"20220509153834.485871-1-chriscool@tuxfamily.org","subject":"[PATCH v4] http: add custom hostname to IP address resolutions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-16T08:38:51Z","receivedAt":"2022-05-16T08:39:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Libcurl has a CURLOPT_RESOLVE easy option that allows\nthe result of hostname resolution in the following\nformat to be passed:\n\n\t[+]HOST:PORT:ADDRESS[,ADDRESS]\n\nThis way, redirects and everything operating against the\nHOST+PORT will use the provided ADDRESS(s).\n\nThe following format is also allowed to stop using\nhostname resolutions that have already been passed:\n\n\t-HOST:PORT\n\nSee https://curl.se/libcurl/c/CURLOPT_RESOLVE.html for\nmore details.\n\nLet's add a corresponding \"http.curloptResolve\" config\noption that takes advantage of CURLOPT_RESOLVE.\n\nEach value configured for the \"http.curloptResolve\" key\nis passed \"as is\" to libcurl through CURLOPT_RESOLVE, so\nit should be in one of the above 2 formats. This keeps\nthe implementation simple and makes us consistent with\nlibcurl's CURLOPT_RESOLVE, and with curl's corresponding\n`--resolve` command line option.\n\nThe implementation uses CURLOPT_RESOLVE only in\nget_active_slot() which is called by all the HTTP\nrequest sending functions.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n\nChanges since v3:\n\n  - Use gitbogusexamplehost.invalid instead of gitbogusexamplehost.com\n    in the test.\n  - Rebased on top of master at 277cf0bc36 (second 0th batch of topics\n    from the previous cycle, 2022-05-11).\n\nRange diff:\n\n1:  3d689f8a6f ! 1:  a27ebf5988 http: add custom hostname to IP address resolutions\n    @@ t/t5551-http-fetch-smart.sh: test_expect_success 'client falls back from v2 to v\n      '\n      \n     +test_expect_success 'passing hostname resolution information works' '\n    -+  BOGUS_HOST=gitbogusexamplehost.com &&\n    ++  BOGUS_HOST=gitbogusexamplehost.invalid &&\n     +  BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n     +  test_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n     +  git -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n\n Documentation/config/http.txt | 16 ++++++++++++++++\n http.c                        | 18 ++++++++++++++++++\n t/t5551-http-fetch-smart.sh   |  7 +++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/config/http.txt b/Documentation/config/http.txt\nindex 7003661c0d..179d03e57b 100644\n--- a/Documentation/config/http.txt\n+++ b/Documentation/config/http.txt\n@@ -98,6 +98,22 @@ http.version::\n \t- HTTP/2\n \t- HTTP/1.1\n \n+http.curloptResolve::\n+\tHostname resolution information that will be used first by\n+\tlibcurl when sending HTTP requests.  This information should\n+\tbe in one of the following formats:\n+\n+\t- [+]HOST:PORT:ADDRESS[,ADDRESS]\n+\t- -HOST:PORT\n+\n++\n+The first format redirects all requests to the given `HOST:PORT`\n+to the provided `ADDRESS`(s). The second format clears all\n+previous config values for that `HOST:PORT` combination.  To\n+allow easy overriding of all the settings inherited from the\n+system config, an empty value will reset all resolution\n+information to the empty list.\n+\n http.sslVersion::\n \tThe SSL version to use when negotiating an SSL connection, if you\n \twant to force the default.  The available and default version\ndiff --git a/http.c b/http.c\nindex 229da4d148..8beacb95cc 100644\n--- a/http.c\n+++ b/http.c\n@@ -128,6 +128,8 @@ static struct curl_slist *pragma_header;\n static struct curl_slist *no_pragma_header;\n static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;\n \n+static struct curl_slist *host_resolutions;\n+\n static struct active_request_slot *active_queue_head;\n \n static char *cached_accept_language;\n@@ -393,6 +395,18 @@ static int http_options(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"http.curloptresolve\", var)) {\n+\t\tif (!value) {\n+\t\t\treturn config_error_nonbool(var);\n+\t\t} else if (!*value) {\n+\t\t\tcurl_slist_free_all(host_resolutions);\n+\t\t\thost_resolutions = NULL;\n+\t\t} else {\n+\t\t\thost_resolutions = curl_slist_append(host_resolutions, value);\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(\"http.followredirects\", var)) {\n \t\tif (value && !strcmp(value, \"initial\"))\n \t\t\thttp_follow_config = HTTP_FOLLOW_INITIAL;\n@@ -1131,6 +1145,9 @@ void http_cleanup(void)\n \tcurl_slist_free_all(no_pragma_header);\n \tno_pragma_header = NULL;\n \n+\tcurl_slist_free_all(host_resolutions);\n+\thost_resolutions = NULL;\n+\n \tif (curl_http_proxy) {\n \t\tfree((void *)curl_http_proxy);\n \t\tcurl_http_proxy = NULL;\n@@ -1211,6 +1228,7 @@ struct active_request_slot *get_active_slot(void)\n \tif (curl_save_cookies)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_COOKIEJAR, curl_cookie_file);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_RESOLVE, host_resolutions);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, curl_errorstr);\n \tcurl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, NULL);\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex f92c79c132..b9351a732f 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '\n \tgrep symref=HEAD:refs/heads/ trace\n '\n \n+test_expect_success 'passing hostname resolution information works' '\n+\tBOGUS_HOST=gitbogusexamplehost.invalid &&\n+\tBOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&\n+\ttest_must_fail git ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null &&\n+\tgit -c \"http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1\" ls-remote \"$BOGUS_HTTPD_URL/smart/repo.git\" >/dev/null\n+'\n+\n test_done\n-- \n2.36.1.75.ga27ebf5988\n\n"}]}