| Message ID | 20240324011301.1553072-4-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support for arbitrary schemes in credentials | expand |
On Sun, Mar 24, 2024 at 01:12:51AM +0000, brian m. carlson wrote: > Currently we create one set of headers for all object requests and reuse > it. However, we'll need to adjust the headers for authentication > purposes in the future, so let's create a new set for each request so > that we can adjust them if the authentication changes. > > Note that the cost of allocation here is tiny compared to the fact that > we're making a network call, not to mention probably a full TLS > connection, so this shouldn't have a significant impact on performance. > Moreover, nobody who cares about performance is using the dumb HTTP > protocol anyway, since it often makes huge numbers of requests compared > to the smart protocol. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > http.c | 19 +++++++++++-------- > http.h | 2 ++ > 2 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/http.c b/http.c > index e73b136e58..1c2200da77 100644 > --- a/http.c > +++ b/http.c > @@ -128,7 +128,6 @@ static unsigned long empty_auth_useless = > | CURLAUTH_DIGEST; > > static struct curl_slist *pragma_header; > -static struct curl_slist *no_pragma_header; > static struct string_list extra_http_headers = STRING_LIST_INIT_DUP; > > static struct curl_slist *host_resolutions; Nice to see that this also allows us to get rid of one more global variable. > @@ -299,6 +298,11 @@ size_t fwrite_null(char *ptr UNUSED, size_t eltsize UNUSED, size_t nmemb, > return nmemb; > } > > +static struct curl_slist *object_request_headers(void) > +{ > + return curl_slist_append(http_copy_default_headers(), "Pragma:"); > +} > + > static void closedown_active_slot(struct active_request_slot *slot) > { > active_requests--; > @@ -1275,8 +1279,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) > > pragma_header = curl_slist_append(http_copy_default_headers(), > "Pragma: no-cache"); > - no_pragma_header = curl_slist_append(http_copy_default_headers(), > - "Pragma:"); > > { > char *http_max_requests = getenv("GIT_HTTP_MAX_REQUESTS"); > @@ -1360,8 +1362,6 @@ void http_cleanup(void) > curl_slist_free_all(pragma_header); > pragma_header = NULL; > > - curl_slist_free_all(no_pragma_header); > - no_pragma_header = NULL; > Nit: this leaves behind two consecutive empty lines. Patrick > curl_slist_free_all(host_resolutions); > host_resolutions = NULL; > @@ -2370,6 +2370,7 @@ void release_http_pack_request(struct http_pack_request *preq) > } > preq->slot = NULL; > strbuf_release(&preq->tmpfile); > + curl_slist_free_all(preq->headers); > free(preq->url); > free(preq); > } > @@ -2454,11 +2455,11 @@ struct http_pack_request *new_direct_http_pack_request( > } > > preq->slot = get_active_slot(); > + preq->headers = object_request_headers(); > curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEDATA, preq->packfile); > curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite); > curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url); > - curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, > - no_pragma_header); > + curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, preq->headers); > > /* > * If there is data present from a previous transfer attempt, > @@ -2624,13 +2625,14 @@ struct http_object_request *new_http_object_request(const char *base_url, > } > > freq->slot = get_active_slot(); > + freq->headers = object_request_headers(); > > curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq); > curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0); > curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); > curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr); > curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url); > - curl_easy_setopt(freq->slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); > + curl_easy_setopt(freq->slot->curl, CURLOPT_HTTPHEADER, freq->headers); > > /* > * If we have successfully processed data from a previous fetch > @@ -2718,5 +2720,6 @@ void release_http_object_request(struct http_object_request *freq) > release_active_slot(freq->slot); > freq->slot = NULL; > } > + curl_slist_free_all(freq->headers); > strbuf_release(&freq->tmpfile); > } > diff --git a/http.h b/http.h > index 3af19a8bf5..c5f8cc4620 100644 > --- a/http.h > +++ b/http.h > @@ -196,6 +196,7 @@ struct http_pack_request { > FILE *packfile; > struct strbuf tmpfile; > struct active_request_slot *slot; > + struct curl_slist *headers; > }; > > struct http_pack_request *new_http_pack_request( > @@ -229,6 +230,7 @@ struct http_object_request { > int zret; > int rename; > struct active_request_slot *slot; > + struct curl_slist *headers; > }; > > struct http_object_request *new_http_object_request( >
diff --git a/http.c b/http.c index e73b136e58..1c2200da77 100644 --- a/http.c +++ b/http.c @@ -128,7 +128,6 @@ static unsigned long empty_auth_useless = | CURLAUTH_DIGEST; static struct curl_slist *pragma_header; -static struct curl_slist *no_pragma_header; static struct string_list extra_http_headers = STRING_LIST_INIT_DUP; static struct curl_slist *host_resolutions; @@ -299,6 +298,11 @@ size_t fwrite_null(char *ptr UNUSED, size_t eltsize UNUSED, size_t nmemb, return nmemb; } +static struct curl_slist *object_request_headers(void) +{ + return curl_slist_append(http_copy_default_headers(), "Pragma:"); +} + static void closedown_active_slot(struct active_request_slot *slot) { active_requests--; @@ -1275,8 +1279,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) pragma_header = curl_slist_append(http_copy_default_headers(), "Pragma: no-cache"); - no_pragma_header = curl_slist_append(http_copy_default_headers(), - "Pragma:"); { char *http_max_requests = getenv("GIT_HTTP_MAX_REQUESTS"); @@ -1360,8 +1362,6 @@ void http_cleanup(void) curl_slist_free_all(pragma_header); pragma_header = NULL; - curl_slist_free_all(no_pragma_header); - no_pragma_header = NULL; curl_slist_free_all(host_resolutions); host_resolutions = NULL; @@ -2370,6 +2370,7 @@ void release_http_pack_request(struct http_pack_request *preq) } preq->slot = NULL; strbuf_release(&preq->tmpfile); + curl_slist_free_all(preq->headers); free(preq->url); free(preq); } @@ -2454,11 +2455,11 @@ struct http_pack_request *new_direct_http_pack_request( } preq->slot = get_active_slot(); + preq->headers = object_request_headers(); curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEDATA, preq->packfile); curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite); curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url); - curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, - no_pragma_header); + curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, preq->headers); /* * If there is data present from a previous transfer attempt, @@ -2624,13 +2625,14 @@ struct http_object_request *new_http_object_request(const char *base_url, } freq->slot = get_active_slot(); + freq->headers = object_request_headers(); curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq); curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0); curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr); curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url); - curl_easy_setopt(freq->slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); + curl_easy_setopt(freq->slot->curl, CURLOPT_HTTPHEADER, freq->headers); /* * If we have successfully processed data from a previous fetch @@ -2718,5 +2720,6 @@ void release_http_object_request(struct http_object_request *freq) release_active_slot(freq->slot); freq->slot = NULL; } + curl_slist_free_all(freq->headers); strbuf_release(&freq->tmpfile); } diff --git a/http.h b/http.h index 3af19a8bf5..c5f8cc4620 100644 --- a/http.h +++ b/http.h @@ -196,6 +196,7 @@ struct http_pack_request { FILE *packfile; struct strbuf tmpfile; struct active_request_slot *slot; + struct curl_slist *headers; }; struct http_pack_request *new_http_pack_request( @@ -229,6 +230,7 @@ struct http_object_request { int zret; int rename; struct active_request_slot *slot; + struct curl_slist *headers; }; struct http_object_request *new_http_object_request(
Currently we create one set of headers for all object requests and reuse it. However, we'll need to adjust the headers for authentication purposes in the future, so let's create a new set for each request so that we can adjust them if the authentication changes. Note that the cost of allocation here is tiny compared to the fact that we're making a network call, not to mention probably a full TLS connection, so this shouldn't have a significant impact on performance. Moreover, nobody who cares about performance is using the dumb HTTP protocol anyway, since it often makes huge numbers of requests compared to the smart protocol. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- http.c | 19 +++++++++++-------- http.h | 2 ++ 2 files changed, 13 insertions(+), 8 deletions(-)