{"thread":{"id":"46529","subject":"[PATCH 1/4] imap-send: add wrapper to get server credentials if needed","startedAt":"2017-08-07T15:22:08Z","lastAt":"2017-08-07T22:22:12Z","messageCount":8,"participants":["Nicolas Morey-Chaisemartin","Martin Ågren","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"325690","messageId":"9687e182-1e23-f84d-3fad-83fdf15dba4c@morey-chaisemartin.com","threadId":"46529","inReplyTo":null,"subject":"[PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-08-07T14:03:50Z","receivedAt":"2017-08-07T15:22:08Z","isPatch":true,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"Signed-off-by: Nicolas Morey-Chaisemartin \n---\n imap-send.c | 38 ++++++++++++++++++++++++--------------\n 1 file changed, 24 insertions(+), 14 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex b2d0b849b..38b3c817e 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -926,6 +926,29 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha\n \treturn 0;\n }\n \n+static void server_fill_credential(struct imap_server_conf *srvc)\n+{\n+\tstruct credential cred = CREDENTIAL_INIT;\n+\n+\tif (srvc->user && srvc->pass)\n+\t\treturn;\n+\n+\tcred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n+\tcred.host = xstrdup(srvc->host);\n+\n+\tcred.username = xstrdup_or_null(srvc->user);\n+\tcred.password = xstrdup_or_null(srvc->pass);\n+\n+\tcredential_fill(&cred);\n+\n+\tif (!srvc->user)\n+\t\tsrvc->user = xstrdup(cred.username);\n+\tif (!srvc->pass)\n+\t\tsrvc->pass = xstrdup(cred.password);\n+\n+\tcredential_clear(&cred);\n+}\n+\n static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n {\n \tstruct credential cred = CREDENTIAL_INIT;\n@@ -1078,20 +1101,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n \t\t}\n #endif\n \t\timap_info(\"Logging in...\\n\");\n-\t\tif (!srvc->user || !srvc->pass) {\n-\t\t\tcred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n-\t\t\tcred.host = xstrdup(srvc->host);\n-\n-\t\t\tcred.username = xstrdup_or_null(srvc->user);\n-\t\t\tcred.password = xstrdup_or_null(srvc->pass);\n-\n-\t\t\tcredential_fill(&cred);\n-\n-\t\t\tif (!srvc->user)\n-\t\t\t\tsrvc->user = xstrdup(cred.username);\n-\t\t\tif (!srvc->pass)\n-\t\t\t\tsrvc->pass = xstrdup(cred.password);\n-\t\t}\n+\t\tserver_fill_credential(srvc);\n \n \t\tif (srvc->auth_method) {\n \t\t\tstruct imap_cmd_cb cb;\n"},{"id":"325692","messageId":"CAN0heSoA_Xm+14V49hjJBMT7PS_ge0wyhmuvKiw-MHD99rApYQ@mail.gmail.com","threadId":"46529","inReplyTo":"9687e182-1e23-f84d-3fad-83fdf15dba4c@morey-chaisemartin.com","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2017-08-07T16:30:48Z","receivedAt":"2017-08-07T16:30:57Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 7 August 2017 at 16:03, Nicolas Morey-Chaisemartin\n wrote:\n> Signed-off-by: Nicolas Morey-Chaisemartin \n> ---\n> imap-send.c | 38 ++++++++++++++++++++++++--------------\n> 1 file changed, 24 insertions(+), 14 deletions(-)\n>\n> diff --git a/imap-send.c b/imap-send.c\n> index b2d0b849b..38b3c817e 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -926,6 +926,29 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha\n> return 0;\n> }\n>\n> +static void server_fill_credential(struct imap_server_conf *srvc)\n> +{\n> + struct credential cred = CREDENTIAL_INIT;\n> +\n> + if (srvc->user && srvc->pass)\n> + return;\n> +\n> + cred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n> + cred.host = xstrdup(srvc->host);\n> +\n> + cred.username = xstrdup_or_null(srvc->user);\n> + cred.password = xstrdup_or_null(srvc->pass);\n> +\n> + credential_fill(&cred);\n> +\n> + if (!srvc->user)\n> + srvc->user = xstrdup(cred.username);\n> + if (!srvc->pass)\n> + srvc->pass = xstrdup(cred.password);\n> +\n> + credential_clear(&cred);\n> +}\n> +\n> static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n> {\n> struct credential cred = CREDENTIAL_INIT;\n> @@ -1078,20 +1101,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n> }\n> #endif\n> imap_info(\"Logging in...\\n\");\n> - if (!srvc->user || !srvc->pass) {\n> - cred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n> - cred.host = xstrdup(srvc->host);\n> -\n> - cred.username = xstrdup_or_null(srvc->user);\n> - cred.password = xstrdup_or_null(srvc->pass);\n> -\n> - credential_fill(&cred);\n> -\n> - if (!srvc->user)\n> - srvc->user = xstrdup(cred.username);\n> - if (!srvc->pass)\n> - srvc->pass = xstrdup(cred.password);\n> - }\n> + server_fill_credential(srvc);\n>\n> if (srvc->auth_method) {\n> struct imap_cmd_cb cb;\n\n\"cred.username\" is checked further down, but now it will always be NULL,\nno?\n"},{"id":"325699","messageId":"fdd74c52-7d9a-b12e-2e1f-524479201701@morey-chaisemartin.com","threadId":"46529","inReplyTo":"CAN0heSoA_Xm+14V49hjJBMT7PS_ge0wyhmuvKiw-MHD99rApYQ@mail.gmail.com","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-08-07T17:04:12Z","receivedAt":"2017-08-07T17:11:09Z","isPatch":true,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"\n\nLe 07/08/2017 à 18:30, Martin Ågren a écrit :\n> On 7 August 2017 at 16:03, Nicolas Morey-Chaisemartin\n> wrote:\n>> Signed-off-by: Nicolas Morey-Chaisemartin \n>> ---\n>> imap-send.c | 38 ++++++++++++++++++++++++--------------\n>> 1 file changed, 24 insertions(+), 14 deletions(-)\n>>\n>> diff --git a/imap-send.c b/imap-send.c\n>> index b2d0b849b..38b3c817e 100644\n>> --- a/imap-send.c\n>> +++ b/imap-send.c\n>> @@ -926,6 +926,29 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha\n>> return 0;\n>> }\n>>\n>> +static void server_fill_credential(struct imap_server_conf *srvc)\n>> +{\n>> + struct credential cred = CREDENTIAL_INIT;\n>> +\n>> + if (srvc->user && srvc->pass)\n>> + return;\n>> +\n>> + cred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n>> + cred.host = xstrdup(srvc->host);\n>> +\n>> + cred.username = xstrdup_or_null(srvc->user);\n>> + cred.password = xstrdup_or_null(srvc->pass);\n>> +\n>> + credential_fill(&cred);\n>> +\n>> + if (!srvc->user)\n>> + srvc->user = xstrdup(cred.username);\n>> + if (!srvc->pass)\n>> + srvc->pass = xstrdup(cred.password);\n>> +\n>> + credential_clear(&cred);\n>> +}\n>> +\n>> static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n>> {\n>> struct credential cred = CREDENTIAL_INIT;\n>> @@ -1078,20 +1101,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n>> }\n>> #endif\n>> imap_info(\"Logging in...\\n\");\n>> - if (!srvc->user || !srvc->pass) {\n>> - cred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n>> - cred.host = xstrdup(srvc->host);\n>> -\n>> - cred.username = xstrdup_or_null(srvc->user);\n>> - cred.password = xstrdup_or_null(srvc->pass);\n>> -\n>> - credential_fill(&cred);\n>> -\n>> - if (!srvc->user)\n>> - srvc->user = xstrdup(cred.username);\n>> - if (!srvc->pass)\n>> - srvc->pass = xstrdup(cred.password);\n>> - }\n>> + server_fill_credential(srvc);\n>>\n>> if (srvc->auth_method) {\n>> struct imap_cmd_cb cb;\n> \"cred.username\" is checked further down, but now it will always be NULL,\n> no?\n\nYou're right I missed this.\nNot sure if this is needed though.\nFrom what I understand this means the username/password are store for the next access to credential. but in the current state, there is only one.\nMaybe the credential_approved can be dropped ?\n\nNicolas\n"},{"id":"325701","messageId":"CAN0heSqVmrFwP7LdjDJmH0JivoCc+DhGtUiTSBs=8nTppzG79A@mail.gmail.com","threadId":"46529","inReplyTo":"fdd74c52-7d9a-b12e-2e1f-524479201701@morey-chaisemartin.com","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2017-08-07T17:18:32Z","receivedAt":"2017-08-07T17:18:41Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 7 August 2017 at 19:04, Nicolas Morey-Chaisemartin\n wrote:\n>\n>\n> Le 07/08/2017 à 18:30, Martin Ågren a écrit :\n>> On 7 August 2017 at 16:03, Nicolas Morey-Chaisemartin\n>> wrote:\n>>> Signed-off-by: Nicolas Morey-Chaisemartin \n>>> ---\n>>> imap-send.c | 38 ++++++++++++++++++++++++--------------\n>>> 1 file changed, 24 insertions(+), 14 deletions(-)\n>>>\n>>> diff --git a/imap-send.c b/imap-send.c\n>>> index b2d0b849b..38b3c817e 100644\n>>> --- a/imap-send.c\n>>> +++ b/imap-send.c\n>>> @@ -926,6 +926,29 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha\n>>> return 0;\n>>> }\n>>>\n>>> +static void server_fill_credential(struct imap_server_conf *srvc)\n>>> +{\n>>> + struct credential cred = CREDENTIAL_INIT;\n>>> +\n>>> + if (srvc->user && srvc->pass)\n>>> + return;\n>>> +\n>>> + cred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n>>> + cred.host = xstrdup(srvc->host);\n>>> +\n>>> + cred.username = xstrdup_or_null(srvc->user);\n>>> + cred.password = xstrdup_or_null(srvc->pass);\n>>> +\n>>> + credential_fill(&cred);\n>>> +\n>>> + if (!srvc->user)\n>>> + srvc->user = xstrdup(cred.username);\n>>> + if (!srvc->pass)\n>>> + srvc->pass = xstrdup(cred.password);\n>>> +\n>>> + credential_clear(&cred);\n>>> +}\n>>> +\n>>> static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n>>> {\n>>> struct credential cred = CREDENTIAL_INIT;\n>>> @@ -1078,20 +1101,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n>>> }\n>>> #endif\n>>> imap_info(\"Logging in...\\n\");\n>>> - if (!srvc->user || !srvc->pass) {\n>>> - cred.protocol = xstrdup(srvc->use_ssl ? \"imaps\" : \"imap\");\n>>> - cred.host = xstrdup(srvc->host);\n>>> -\n>>> - cred.username = xstrdup_or_null(srvc->user);\n>>> - cred.password = xstrdup_or_null(srvc->pass);\n>>> -\n>>> - credential_fill(&cred);\n>>> -\n>>> - if (!srvc->user)\n>>> - srvc->user = xstrdup(cred.username);\n>>> - if (!srvc->pass)\n>>> - srvc->pass = xstrdup(cred.password);\n>>> - }\n>>> + server_fill_credential(srvc);\n>>>\n>>> if (srvc->auth_method) {\n>>> struct imap_cmd_cb cb;\n>> \"cred.username\" is checked further down, but now it will always be NULL,\n>> no?\n>\n> You're right I missed this.\n> Not sure if this is needed though.\n> From what I understand this means the username/password are store for the next access to credential. but in the current state, there is only one.\n> Maybe the credential_approved can be dropped ?\n\nI'm no credentials-expert, but api-credentials.txt says this:\n\n\"Credential helpers are programs executed by Git to fetch or save\ncredentials from and to long-term storage (where \"long-term\" is simply\nlonger than a single Git process; e.g., credentials may be stored\nin-memory for a few minutes, or indefinitely on disk).\"\n\nSo the calls to approve/reject probably do matter in some scenarios.\n\nThe current code is a bit non-obvious as we just discovered since it\nduplicates the strings (for good reasons, I believe) and then still\nrefers to the originals (also for good reasons, I believe). I suppose\nyour new function could be called like\n\nserver_fill_credential(&cred, srvc);\n\nThat should limit the impact of the change, but I'm not sure it's a\nbrilliant interface. Just my 2c.\n\nMartin\n"},{"id":"325732","messageId":"20170807194257.jrdmpvoseauz37uc@sigill.intra.peff.net","threadId":"46529","inReplyTo":"CAN0heSqVmrFwP7LdjDJmH0JivoCc+DhGtUiTSBs=8nTppzG79A@mail.gmail.com","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-07T19:42:57Z","receivedAt":"2017-08-07T19:43:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 07, 2017 at 07:18:32PM +0200, Martin Ågren wrote:\n\n> >> \"cred.username\" is checked further down, but now it will always be NULL,\n> >> no?\n> >\n> > You're right I missed this.\n> > Not sure if this is needed though.\n> > From what I understand this means the username/password are store for the next access to credential. but in the current state, there is only one.\n> > Maybe the credential_approved can be dropped ?\n> \n> I'm no credentials-expert, but api-credentials.txt says this:\n> \n> \"Credential helpers are programs executed by Git to fetch or save\n> credentials from and to long-term storage (where \"long-term\" is simply\n> longer than a single Git process; e.g., credentials may be stored\n> in-memory for a few minutes, or indefinitely on disk).\"\n> \n> So the calls to approve/reject probably do matter in some scenarios.\n\nRight, this is important. credential_fill() may actually prompt the\nuser, and we'd want to then pass along that credential for storage. Or\nthe user may have changed their password, and the credential_reject() is\nthe only thing that prevents us from trying an out-dated password over\nand over.\n\n> The current code is a bit non-obvious as we just discovered since it\n> duplicates the strings (for good reasons, I believe) and then still\n> refers to the originals (also for good reasons, I believe). I suppose\n> your new function could be called like\n> \n> server_fill_credential(&cred, srvc);\n> \n> That should limit the impact of the change, but I'm not sure it's a\n> brilliant interface. Just my 2c.\n\nThat would work. I'm also tempted to say that imap_server_conf could\njust store the \"struct credential\" inside it. We could even potentially\ndrop its parallel user/pass fields to minimize confusion.\n\nOnce upon a time imap-send was a fork of another program, which is why\nmost of its code isn't well-integrated with Git. But I think at this\npoint there's very little chance of merging changes back and forth\nbetween the two.\n\nOn the other hand, if we're hoping to get rid of this code in favor of\nthe curl-based approach, then it's not worth spending time on\ncosmetic refactoring, as long as it still behaves correctly in the\ninterim.\n\n-Peff\n"},{"id":"325737","messageId":"a4761418-0464-baa1-b415-836f33503eb8@morey-chaisemartin.com","threadId":"46529","inReplyTo":"20170807194257.jrdmpvoseauz37uc@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-08-07T19:55:56Z","receivedAt":"2017-08-07T19:56:08Z","isPatch":true,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"\n\nLe 07/08/2017 à 21:42, Jeff King a écrit :\n> On Mon, Aug 07, 2017 at 07:18:32PM +0200, Martin Ågren wrote:\n>\n>>>> \"cred.username\" is checked further down, but now it will always be NULL,\n>>>> no?\n>>> You're right I missed this.\n>>> Not sure if this is needed though.\n>>> From what I understand this means the username/password are store for the next access to credential. but in the current state, there is only one.\n>>> Maybe the credential_approved can be dropped ?\n>> I'm no credentials-expert, but api-credentials.txt says this:\n>>\n>> \"Credential helpers are programs executed by Git to fetch or save\n>> credentials from and to long-term storage (where \"long-term\" is simply\n>> longer than a single Git process; e.g., credentials may be stored\n>> in-memory for a few minutes, or indefinitely on disk).\"\n>>\n>> So the calls to approve/reject probably do matter in some scenarios.\n> Right, this is important. credential_fill() may actually prompt the\n> user, and we'd want to then pass along that credential for storage. Or\n> the user may have changed their password, and the credential_reject() is\n> the only thing that prevents us from trying an out-dated password over\n> and over.\n>\n>> The current code is a bit non-obvious as we just discovered since it\n>> duplicates the strings (for good reasons, I believe) and then still\n>> refers to the originals (also for good reasons, I believe). I suppose\n>> your new function could be called like\n>>\n>> server_fill_credential(&cred, srvc);\n>>\n>> That should limit the impact of the change, but I'm not sure it's a\n>> brilliant interface. Just my 2c.\n> That would work. I'm also tempted to say that imap_server_conf could\n> just store the \"struct credential\" inside it. We could even potentially\n> drop its parallel user/pass fields to minimize confusion.\n>\n> Once upon a time imap-send was a fork of another program, which is why\n> most of its code isn't well-integrated with Git. But I think at this\n> point there's very little chance of merging changes back and forth\n> between the two.\n>\n> On the other hand, if we're hoping to get rid of this code in favor of\n> the curl-based approach, then it's not worth spending time on\n> cosmetic refactoring, as long as it still behaves correctly in the\n> interim.\n>\n> -Peff\n\nLooking at the code, it seems the tunnel mode always uses the legacy imap approach.\nThis would have to be ported to curl and stabilized before dropping the legacy code.\nIn the meantime, it might be worth doing a bit of cleanup.\nAnd as I said in patch 4, I have a current IMAP account where it works without curl but not with curl (for unknown reason yet).\nMeaning legacy needs to stay for a while. But switching to curl as default to get out all the bugs (hence this slightly broken patch series)\n\nI think it would make sense to get patch 1 (fixed), 2 and 4 in to really test out the curl implementation and take some time to prepare another serie with code cleanups: dropping imap_server_conf parameters, storing cred therem etc.)\n\nNicolas\n\n\n\n"},{"id":"325739","messageId":"20170807200724.6pdfkmrrmmkdmmb6@sigill.intra.peff.net","threadId":"46529","inReplyTo":"a4761418-0464-baa1-b415-836f33503eb8@morey-chaisemartin.com","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-07T20:07:24Z","receivedAt":"2017-08-07T20:07:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 07, 2017 at 09:55:56PM +0200, Nicolas Morey-Chaisemartin wrote:\n\n> > On the other hand, if we're hoping to get rid of this code in favor of\n> > the curl-based approach, then it's not worth spending time on\n> > cosmetic refactoring, as long as it still behaves correctly in the\n> > interim.\n> \n> Looking at the code, it seems the tunnel mode always uses the legacy imap approach.\n> This would have to be ported to curl and stabilized before dropping the legacy code.\n\nUrgh. That's an important mode, I'd think, and one that I have a feeling\ncurl may not be interested in supporting, just because of it's\ncomplexity. And even if they did, it would take a while for that curl\nversion to become available.\n\nSo maybe the idea of deprecating the non-curl implementation is not\nsomething that can happen anytime soon. :(\n\n> In the meantime, it might be worth doing a bit of cleanup.\n\nIn which case, yeah, it is definitely worth cleaning up the existing\ncode. But I also agree with you that it's worth making sure the curl\nversion behaves as similarly as possible.\n\n-Peff\n"},{"id":"325771","messageId":"xmqqd187nhmk.fsf@gitster.mtv.corp.google.com","threadId":"46529","inReplyTo":"20170807200724.6pdfkmrrmmkdmmb6@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] imap-send: add wrapper to get server credentials if needed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T22:21:55Z","receivedAt":"2017-08-07T22:22:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Aug 07, 2017 at 09:55:56PM +0200, Nicolas Morey-Chaisemartin wrote:\n>\n>> > On the other hand, if we're hoping to get rid of this code in favor of\n>> > the curl-based approach, then it's not worth spending time on\n>> > cosmetic refactoring, as long as it still behaves correctly in the\n>> > interim.\n>> \n>> Looking at the code, it seems the tunnel mode always uses the legacy imap approach.\n>> This would have to be ported to curl and stabilized before dropping the legacy code.\n>\n> Urgh. That's an important mode, I'd think, and one that I have a feeling\n> curl may not be interested in supporting, just because of it's\n> complexity. And even if they did, it would take a while for that curl\n> version to become available.\n>\n> So maybe the idea of deprecating the non-curl implementation is not\n> something that can happen anytime soon. :(\n>\n>> In the meantime, it might be worth doing a bit of cleanup.\n>\n> In which case, yeah, it is definitely worth cleaning up the existing\n> code. But I also agree with you that it's worth making sure the curl\n> version behaves as similarly as possible.\n\nThanks for guiding this topic forward. I agree with all points you\nraised in your reviews.\n"}]}