{"thread":{"id":"34186","subject":"[PATCH] http.c: don't rewrite the user:passwd string multiple times","startedAt":"2013-06-18T02:00:40Z","lastAt":"2013-06-19T07:40:54Z","messageCount":11,"participants":["Brandon Casey","Eric Sunshine","Jeff King","Daniel Stenberg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"221118","messageId":"1371520840-24906-1-git-send-email-bcasey@nvidia.com","threadId":"34186","inReplyTo":null,"subject":"[PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Brandon Casey","fromEmail":"bcasey@nvidia.com","sentAt":"2013-06-18T02:00:40Z","receivedAt":"2013-06-18T02:00:40Z","isPatch":true,"sender":{"key":"bcasey@nvidia.com","avatar":null},"body":"From: Brandon Casey \n\nCurl requires that we manage any strings that we pass to it as pointers.\nSo, we should not be overwriting this strbuf after we've passed it to\ncurl.\n\nAdditionally, it is unnecessary since we only prompt for the user name\nand password once, so we end up overwriting the strbuf with the same\nsequence of characters each time. This is why in practice it has not\ncaused any problems for git's use of curl; the internal strbuf char\npointer does not change, and get's overwritten with the same string\neach time.\n\nBut it's unnecessary and potentially dangerous, so let's avoid it.\n\nSigned-off-by: Brandon Casey \n---\n http.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 92aba59..6828269 100644\n--- a/http.c\n+++ b/http.c\n@@ -228,8 +228,8 @@ static void init_curl_http_auth(CURL *result)\n #else\n \t{\n \t\tstatic struct strbuf up = STRBUF_INIT;\n-\t\tstrbuf_reset(&up);\n-\t\tstrbuf_addf(&up, \"%s:%s\",\n+\t\tif (!up.len)\n+\t\t\tstrbuf_addf(&up, \"%s:%s\",\n \t\t\t http_auth.username, http_auth.password);\n \t\tcurl_easy_setopt(result, CURLOPT_USERPWD, up.buf);\n \t}\n-- \n1.8.3.1.440.gc2bf105\n"},{"id":"221138","messageId":"CAPig+cTKBzfrB6QQ4qjHNknv1CKRro_t=f77OV+ZhabtMN6Uiw@mail.gmail.com","threadId":"34186","inReplyTo":"1371520840-24906-1-git-send-email-bcasey@nvidia.com","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-06-18T04:15:52Z","receivedAt":"2013-06-18T04:15:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 17, 2013 at 10:00 PM, Brandon Casey wrote:\n> From: Brandon Casey \n>\n> Curl requires that we manage any strings that we pass to it as pointers.\n> So, we should not be overwriting this strbuf after we've passed it to\n> curl.\n>\n> Additionally, it is unnecessary since we only prompt for the user name\n> and password once, so we end up overwriting the strbuf with the same\n> sequence of characters each time. This is why in practice it has not\n> caused any problems for git's use of curl; the internal strbuf char\n> pointer does not change, and get's overwritten with the same string\n\ns/get's/gets/\n\n> each time.\n>\n> But it's unnecessary and potentially dangerous, so let's avoid it.\n>\n> Signed-off-by: Brandon Casey \n"},{"id":"221142","messageId":"20130618051902.GA5916@sigill.intra.peff.net","threadId":"34186","inReplyTo":"1371520840-24906-1-git-send-email-bcasey@nvidia.com","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-18T05:19:03Z","receivedAt":"2013-06-18T05:19:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 17, 2013 at 07:00:40PM -0700, Brandon Casey wrote:\n\n> Curl requires that we manage any strings that we pass to it as pointers.\n> So, we should not be overwriting this strbuf after we've passed it to\n> curl.\n\nMy understanding of curl's pointer requirements are:\n\n 1. Older versions of curl (and I do not recall which version off-hand,\n but it is not important) stored just the pointer. Calling code was\n required to manage the string lifetime itself.\n\n 2. Newer versions of curl will strdup the string in curl_easy_setopt.\n\nSo we do not have to worry about newer versions, as they do not care\nabout our pointer after curl_easy_setopt returns.\n\nFor older versions, if we were to grow the strbuf, we might free() the\npointer provided to an earlier call to curl_easy_setopt. But since we\nare about to call curl_easy_setopt with the new value, I would assume\nthat curl will never actually look at the old one (i.e., when replacing\nan old pointer, it would not dereference it, but simply overwrite it\nwith the new value).\n\nSo for a single curl handle, I don't think it is a problem.\n\nIt could be a problem when we have multiple handles in play\nsimultaneously (we invalidate the pointer that another simultaneous\nhandle is using, but do not immediately reset its pointer).\n\n> Additionally, it is unnecessary since we only prompt for the user name\n> and password once, so we end up overwriting the strbuf with the same\n> sequence of characters each time. This is why in practice it has not\n> caused any problems for git's use of curl; the internal strbuf char\n> pointer does not change, and get's overwritten with the same string\n> each time.\n\nIn the current code, yes, we only do this once (and if we have a\nusername/password from the URL, we do not re-prompt if that fails).\n\n> diff --git a/http.c b/http.c\n> index 92aba59..6828269 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -228,8 +228,8 @@ static void init_curl_http_auth(CURL *result)\n> #else\n> \t{\n> \t\tstatic struct strbuf up = STRBUF_INIT;\n> -\t\tstrbuf_reset(&up);\n> -\t\tstrbuf_addf(&up, \"%s:%s\",\n> +\t\tif (!up.len)\n> +\t\t\tstrbuf_addf(&up, \"%s:%s\",\n> \t\t\t http_auth.username, http_auth.password);\n> \t\tcurl_easy_setopt(result, CURLOPT_USERPWD, up.buf);\n\nThis is correct for the current code because of the reasoning above.\nI'm slightly negative on this only because it feels like we are setting\na trap for somebody who later wants to do:\n\n for (sanity = 0; sanity < 5; sanity++) {\n int ret = http_request(...);\n if (ret != HTTP_REAUTH)\n return ret;\n }\n\nto give the user a few chances to input. We would continue to update\nthe credential struct but never actually give the new value to curl.\n\nAnother option would be to just use a static fixed-size buffer. That\nremoves all memory management issues.\n\nI dunno. Maybe I am being too picky, as I do not have plans to do\nanything like the above (since we don't do significant work before the\nhttp contact, there is no reason not to just die() and let the user\nre-run the shell command). I'd also be OK with just putting a comment\nabove the code in question to say something like \"Note that we assume we\nonly ever have a single set of credentials in a given program run, so we\ndo not have to worry about updating this buffer, only setting its\ninitial value\". Then the trap at least has a warning sign. :)\n\nWhat do you think?\n\n-Peff\n"},{"id":"221149","messageId":"alpine.DEB.2.00.1306180825460.24456@tvnag.unkk.fr","threadId":"34186","inReplyTo":"20130618051902.GA5916@sigill.intra.peff.net","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2013-06-18T06:36:59Z","receivedAt":"2013-06-18T06:36:59Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Tue, 18 Jun 2013, Jeff King wrote:\n\nTL;DR: I'm just confirming what's said here! =)\n\n> My understanding of curl's pointer requirements are:\n>\n> 1. Older versions of curl (and I do not recall which version off-hand,\n> but it is not important) stored just the pointer. Calling code was\n> required to manage the string lifetime itself.\n>\n> 2. Newer versions of curl will strdup the string in curl_easy_setopt.\n\nThat's correct. This \"new\" behavior in (2) was introduced in libcurl 7.17.0 - \nreleased in September 2007 and should thus be fairly rare by now.\n\nI mention this primarily because I think it should be noted that there will \nprobably be very little testing by users with such old libcurl versions. It \nmay increase the time between a committed change and people notice brekages \ncaused by it. Even Debian old-stable has a much newer version.\n\n> For older versions, if we were to grow the strbuf, we might free() the \n> pointer provided to an earlier call to curl_easy_setopt. But since we are \n> about to call curl_easy_setopt with the new value, I would assume that curl \n> will never actually look at the old one (i.e., when replacing an old \n> pointer, it would not dereference it, but simply overwrite it with the new \n> value).\n\nAnother accurate description.\n\n-- \n\n / daniel.haxx.se\n"},{"id":"221240","messageId":"7v38sf8mep.fsf@alter.siamese.dyndns.org","threadId":"34186","inReplyTo":"alpine.DEB.2.00.1306180825460.24456@tvnag.unkk.fr","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-18T15:32:30Z","receivedAt":"2013-06-18T15:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Stenberg writes:\n\n> On Tue, 18 Jun 2013, Jeff King wrote:\n>\n> TL;DR: I'm just confirming what's said here! =)\n\nThanks. We are very fortunate to have you as the cURL guru who\ngives prompt responses and sanity checks to us.\n"},{"id":"221294","messageId":"CA+sFfMdEvwzmnEBeO+_pwdmN3m5rkJvUCVFFJU8mtmyN+WxH6w@mail.gmail.com","threadId":"34186","inReplyTo":"20130618051902.GA5916@sigill.intra.peff.net","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2013-06-18T19:29:03Z","receivedAt":"2013-06-18T19:29:03Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Mon, Jun 17, 2013 at 10:19 PM, Jeff King wrote:\n> On Mon, Jun 17, 2013 at 07:00:40PM -0700, Brandon Casey wrote:\n>\n>> Curl requires that we manage any strings that we pass to it as pointers.\n>> So, we should not be overwriting this strbuf after we've passed it to\n>> curl.\n>\n> My understanding of curl's pointer requirements are:\n>\n> 1. Older versions of curl (and I do not recall which version off-hand,\n> but it is not important) stored just the pointer. Calling code was\n> required to manage the string lifetime itself.\n\nDaniel mentions that the change happened in libcurl 7.17. RHEL 4.X\n(yes, ancient, dead, I realize) provides 7.12 and RHEL 5.X (yes,\nancient, but still widely in use) provides 7.15. Just pointing it\nout.\n\n> 2. Newer versions of curl will strdup the string in curl_easy_setopt.\n>\n> So we do not have to worry about newer versions, as they do not care\n> about our pointer after curl_easy_setopt returns.\n\nI was probably reading the docs on one of these older platforms when I\nwrote this. I've actually had this patch sitting around for a while.\n\n> For older versions, if we were to grow the strbuf, we might free() the\n> pointer provided to an earlier call to curl_easy_setopt. But since we\n> are about to call curl_easy_setopt with the new value, I would assume\n> that curl will never actually look at the old one (i.e., when replacing\n> an old pointer, it would not dereference it, but simply overwrite it\n> with the new value).\n>\n> So for a single curl handle, I don't think it is a problem.\n>\n> It could be a problem when we have multiple handles in play\n> simultaneously (we invalidate the pointer that another simultaneous\n> handle is using, but do not immediately reset its pointer).\n\nDon't we have multiple handles in play at the same time? What's going\non in get_active_slot() when USE_CURL_MULTI is defined? It appears to\nbe maintaining a list of \"slot\" 's, each with its own curl handle\ninitialized either by curl_easy_duphandle() or get_curl_handle().\n\nSo, yeah, this is what I was referring to when I mentioned\n\"potentially dangerous\". Since the current code does not change the\nsize of the string, the pointer will never change, so we won't ever\ninvalidate a pointer that another handle is using.\n\nThe other thing I thought was potentially dangerous, was just\ntruncating the string. Again, if there are multiple curl handles in\nuse (which I thought was a possibility), then merely truncating the\nstring that contains the username/password could potentially cause a\nproblem for another handle that could be in the middle of\nauthenticating using the string. But, I don't know if there is any\nmulti-processing happening within the curl library.\n\n\n\nSnip the remaining comments about allowing the user to specify\nmultiple passwords since I'm not sure they're relevant if we are\nindeed using multiple curl handles.\n\nIf we _don't_ ever use multiple curl handles, and/or if there is no\nthreading going on in the background within libcurl, then I don't\nthink there is really any danger in what the current code does. It\nwould just be an issue of needlessly rewriting the same string over\nand over again, which is probably not a big deal depending on how\noften that happens.\n\n-Brandon\n"},{"id":"221307","messageId":"20130618221327.GA14234@sigill.intra.peff.net","threadId":"34186","inReplyTo":"CA+sFfMdEvwzmnEBeO+_pwdmN3m5rkJvUCVFFJU8mtmyN+WxH6w@mail.gmail.com","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-18T22:13:28Z","receivedAt":"2013-06-18T22:13:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 18, 2013 at 12:29:03PM -0700, Brandon Casey wrote:\n\n> > 1. Older versions of curl (and I do not recall which version off-hand,\n> > but it is not important) stored just the pointer. Calling code was\n> > required to manage the string lifetime itself.\n> \n> Daniel mentions that the change happened in libcurl 7.17. RHEL 4.X\n> (yes, ancient, dead, I realize) provides 7.12 and RHEL 5.X (yes,\n> ancient, but still widely in use) provides 7.15. Just pointing it\n> out.\n\nYeah, I didn't mean to imply \"we don't care about these versions\", only\nthat our analysis is different between the two sets. We have #ifdefs for\ncurl going back to 7.7.4. That's probably excessive, but AFAIK, we would\nstill work with such old versions.\n\n> > It could be a problem when we have multiple handles in play\n> > simultaneously (we invalidate the pointer that another simultaneous\n> > handle is using, but do not immediately reset its pointer).\n> \n> Don't we have multiple handles in play at the same time? What's going\n> on in get_active_slot() when USE_CURL_MULTI is defined? It appears to\n> be maintaining a list of \"slot\" 's, each with its own curl handle\n> initialized either by curl_easy_duphandle() or get_curl_handle().\n\nYes, we do; the dumb http walker will pipeline loose pack and object\nrequests (which makes a big difference when fetching small files). The\nsmart http code may use the curl-multi interface under the hood, but it\nshould only have a single handle, and the use of the multi interface is\njust for sharing code with the dumb fetch.\n\n> So, yeah, this is what I was referring to when I mentioned\n> \"potentially dangerous\". Since the current code does not change the\n> size of the string, the pointer will never change, so we won't ever\n> invalidate a pointer that another handle is using.\n\nAgreed. I did not so much mean to dispute your \"potentially dangerous\"\nclaim as clarify exactly what the potential is. :)\n\n> The other thing I thought was potentially dangerous, was just\n> truncating the string. Again, if there are multiple curl handles in\n> use (which I thought was a possibility), then merely truncating the\n> string that contains the username/password could potentially cause a\n> problem for another handle that could be in the middle of\n> authenticating using the string. But, I don't know if there is any\n> multi-processing happening within the curl library.\n\nI don't think curl does any threading; when we are not inside\ncurl_*_perform, there is no curl code running at all (Daniel can correct\nme if I'm wrong on that).\n\nSo I think from curl's perspective a truncation and exact rewrite is\natomic, and it sees only the final content. I don't know what would\nhappen if you truncated and put in _different_ contents. For example, if\ncurl would have written out half of the username/password, blocked and\nreturned from curl_multi_perform, then you update the buffer, then it\nresumes writing.\n\nIOW, I believe the current code is safe (though in a very subtle way),\nbut if you were to allow password update, I'm not sure if it would be or\nnot (and if not, you would need a per-handle buffer to make it safe).\n\nI'm fine with making the safety less subtle (e.g., your patch, with a\ncomment added).\n\n> If we _don't_ ever use multiple curl handles, and/or if there is no\n> threading going on in the background within libcurl, then I don't\n> think there is really any danger in what the current code does. It\n> would just be an issue of needlessly rewriting the same string over\n> and over again, which is probably not a big deal depending on how\n> often that happens.\n\nIt should be once per http request. But copying a dozen bytes is\nprobably nothing compared to the actual request.\n"},{"id":"221314","messageId":"CA+sFfMcsOx14UdzLF_JsgkpUQU6yG7DE+00eA3d+Lo-qncDgew@mail.gmail.com","threadId":"34186","inReplyTo":"20130618221327.GA14234@sigill.intra.peff.net","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2013-06-19T02:41:15Z","receivedAt":"2013-06-19T02:41:15Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Tue, Jun 18, 2013 at 3:13 PM, Jeff King wrote:\n> On Tue, Jun 18, 2013 at 12:29:03PM -0700, Brandon Casey wrote:\n\n>> > It could be a problem when we have multiple handles in play\n>> > simultaneously (we invalidate the pointer that another simultaneous\n>> > handle is using, but do not immediately reset its pointer).\n>>\n>> Don't we have multiple handles in play at the same time? What's going\n>> on in get_active_slot() when USE_CURL_MULTI is defined? It appears to\n>> be maintaining a list of \"slot\" 's, each with its own curl handle\n>> initialized either by curl_easy_duphandle() or get_curl_handle().\n>\n> Yes, we do; the dumb http walker will pipeline loose pack and object\n> requests (which makes a big difference when fetching small files). The\n> smart http code may use the curl-multi interface under the hood, but it\n> should only have a single handle, and the use of the multi interface is\n> just for sharing code with the dumb fetch.\n>\n>> So, yeah, this is what I was referring to when I mentioned\n>> \"potentially dangerous\". Since the current code does not change the\n>> size of the string, the pointer will never change, so we won't ever\n>> invalidate a pointer that another handle is using.\n>\n> Agreed. I did not so much mean to dispute your \"potentially dangerous\"\n> claim as clarify exactly what the potential is. :)\n\nAh, yes, I did read your sentence \"It could be a problem when we have\nmultiple handles in play simultaneously\" as \"It could be a problem [at\nsome point in the future] when we [modify the code to] have multiple\nhandles in play simultaneously, [but since we are not doing that now,\nit is not a problem]\". Now that I read that sentence again, I see you\nare alluding to the dumb http walker code path that I was also\nthinking about.\n\n>> The other thing I thought was potentially dangerous, was just\n>> truncating the string. Again, if there are multiple curl handles in\n>> use (which I thought was a possibility), then merely truncating the\n>> string that contains the username/password could potentially cause a\n>> problem for another handle that could be in the middle of\n>> authenticating using the string. But, I don't know if there is any\n>> multi-processing happening within the curl library.\n>\n> I don't think curl does any threading; when we are not inside\n> curl_*_perform, there is no curl code running at all (Daniel can correct\n> me if I'm wrong on that).\n>\n> So I think from curl's perspective a truncation and exact rewrite is\n> atomic, and it sees only the final content. I don't know what would\n> happen if you truncated and put in _different_ contents. For example, if\n> curl would have written out half of the username/password, blocked and\n> returned from curl_multi_perform, then you update the buffer, then it\n> resumes writing.\n>\n> IOW, I believe the current code is safe (though in a very subtle way),\n> but if you were to allow password update, I'm not sure if it would be or\n> not (and if not, you would need a per-handle buffer to make it safe).\n>\n> I'm fine with making the safety less subtle (e.g., your patch, with a\n> comment added).\n\nOk, will do.\n\n-Brandon\n"},{"id":"221318","messageId":"1371609829-31813-1-git-send-email-bcasey@nvidia.com","threadId":"34186","inReplyTo":"CA+sFfMcsOx14UdzLF_JsgkpUQU6yG7DE+00eA3d+Lo-qncDgew@mail.gmail.com","subject":"[PATCH v2] http.c: don't rewrite the user:passwd string multiple times","fromName":"Brandon Casey","fromEmail":"bcasey@nvidia.com","sentAt":"2013-06-19T02:43:49Z","receivedAt":"2013-06-19T02:43:49Z","isPatch":true,"sender":{"key":"bcasey@nvidia.com","avatar":null},"body":"From: Brandon Casey \n\nCurl older than 7.17 (RHEL 4.X provides 7.12 and RHEL 5.X provides\n7.15) requires that we manage any strings that we pass to it as\npointers. So, we really shouldn't be modifying this strbuf after we\nhave passed it to curl.\n\nOur interaction with curl is currently safe (before or after this\npatch) since the pointer that is passed to curl is never invalidated;\nit is repeatedly rewritten with the same sequence of characters but\nthe strbuf functions never need to allocate a larger string, so the\nsame memory buffer is reused.\n\nThis \"guarantee\" of safety is somewhat subtle and could be overlooked\nby someone who may want to add a more complex handling of the username\nand password. So, let's stop modifying this strbuf after we have\npassed it to curl, but also leave a note to describe the assumptions\nthat have been made about username/password lifetime and to draw\nattention to the code.\n\nSigned-off-by: Brandon Casey \n---\n http.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 92aba59..2d086ae 100644\n--- a/http.c\n+++ b/http.c\n@@ -228,9 +228,15 @@ static void init_curl_http_auth(CURL *result)\n #else\n \t{\n \t\tstatic struct strbuf up = STRBUF_INIT;\n-\t\tstrbuf_reset(&up);\n-\t\tstrbuf_addf(&up, \"%s:%s\",\n-\t\t\t http_auth.username, http_auth.password);\n+\t\t/*\n+\t\t * Note that we assume we only ever have a single set of\n+\t\t * credentials in a given program run, so we do not have\n+\t\t * to worry about updating this buffer, only setting its\n+\t\t * initial value.\n+\t\t */\n+\t\tif (!up.len)\n+\t\t\tstrbuf_addf(&up, \"%s:%s\",\n+\t\t\t\thttp_auth.username, http_auth.password);\n \t\tcurl_easy_setopt(result, CURLOPT_USERPWD, up.buf);\n \t}\n #endif\n-- \n1.8.3.1.440.gc2bf105\n"},{"id":"221327","messageId":"20130619052613.GA17500@sigill.intra.peff.net","threadId":"34186","inReplyTo":"1371609829-31813-1-git-send-email-bcasey@nvidia.com","subject":"Re: [PATCH v2] http.c: don't rewrite the user:passwd string multiple times","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-19T05:26:14Z","receivedAt":"2013-06-19T05:26:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 18, 2013 at 07:43:49PM -0700, Brandon Casey wrote:\n\n> From: Brandon Casey \n> \n> Curl older than 7.17 (RHEL 4.X provides 7.12 and RHEL 5.X provides\n> 7.15) requires that we manage any strings that we pass to it as\n> pointers. So, we really shouldn't be modifying this strbuf after we\n> have passed it to curl.\n> \n> Our interaction with curl is currently safe (before or after this\n> patch) since the pointer that is passed to curl is never invalidated;\n> it is repeatedly rewritten with the same sequence of characters but\n> the strbuf functions never need to allocate a larger string, so the\n> same memory buffer is reused.\n> \n> This \"guarantee\" of safety is somewhat subtle and could be overlooked\n> by someone who may want to add a more complex handling of the username\n> and password. So, let's stop modifying this strbuf after we have\n> passed it to curl, but also leave a note to describe the assumptions\n> that have been made about username/password lifetime and to draw\n> attention to the code.\n\nThanks.\n\nAcked-by: Jeff King \n\n-Peff\n"},{"id":"221343","messageId":"alpine.DEB.2.00.1306190927020.23103@tvnag.unkk.fr","threadId":"34186","inReplyTo":"20130618221327.GA14234@sigill.intra.peff.net","subject":"Re: [PATCH] http.c: don't rewrite the user:passwd string multiple times","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2013-06-19T07:40:54Z","receivedAt":"2013-06-19T07:40:54Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Tue, 18 Jun 2013, Jeff King wrote:\n\n>> But, I don't know if there is any multi-processing happening within the \n>> curl library.\n>\n> I don't think curl does any threading; when we are not inside \n> curl_*_perform, there is no curl code running at all (Daniel can correct me \n> if I'm wrong on that).\n\nCorrect, that's true. The default setup of libcurl never uses any threading at \nall, everything is done using non-blocking calls and state-machines.\n\nThere's but a minor exception, so let me describe that case just to be \nperfectly clear:\n\nWhen you've build libcurl with the \"threaded resolver\" backend, libcurl fires \nup a new thread to resolve host names with during the name resolving phase of \na transfer and that thread can then actually continue to run when \ncurl_multi_perform() returns.\n\nThat's however very isolated, stricly only for name resolving and there should \nbe no way for an application to mess that up. Nothing of what you've discussed \nin this thread would affect or harm that thread. The biggest impact it tends \nto have on applications (that aren't following the API properly or assume a \nlittle too much) is that it changes the nature of what file descriptors to \nwait for slightly during the name resolve phase.\n\nSome Linux distros ship their default libcurl builds using the threaded \nresolver.\n\n-- \n\n / daniel.haxx.se\n"}]}