{"thread":{"id":"35314","subject":"[PATCH] git-send-email: Added the ability to query the number of smtp password questions","startedAt":"2013-11-09T10:21:48Z","lastAt":"2013-11-12T21:23:17Z","messageCount":6,"participants":["silvio@port1024.net","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"230354","messageId":"1383992508-2097-1-git-send-email-silvio@port1024.net","threadId":"35314","inReplyTo":null,"subject":"[PATCH] git-send-email: Added the ability to query the number of smtp password questions","fromName":"","fromEmail":"silvio@port1024.net","sentAt":"2013-11-09T10:21:48Z","receivedAt":"2013-11-09T10:21:48Z","isPatch":true,"sender":{"key":"silvio@port1024.net","avatar":null},"body":"From: Silvio F \n\nSigned-off-by: Silvio F \n---\n Documentation/git-send-email.txt | 4 ++++\n git-send-email.perl | 32 +++++++++++++++++++++-----------\n 2 files changed, 25 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex f0e57a5..ac993d6 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -364,6 +364,10 @@ sendemail.confirm::\n \tone of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'\n \tin the previous section for the meaning of these values.\n \n+sendmail.askpasswordcount::\n+\tNumber of times the smtp password can be entered before sending mail is\n+\taborted. Default is 1.\n+\n EXAMPLE\n -------\n Use gmail as the smtp server\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 3782c3b..9b2eda3 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -203,6 +203,7 @@ my ($validate, $confirm);\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n+my ($askpasswordcount) = 1;\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -237,6 +238,7 @@ my %config_settings = (\n \"from\" => \\$sender,\n \"assume8bitencoding\" => \\$auto_8bit_encoding,\n \"composeencoding\" => \\$compose_encoding,\n+ \"askpasswordcount\" => \\$askpasswordcount,\n );\n \n my %config_path_settings = (\n@@ -360,6 +362,10 @@ sub read_config {\n \t\t}\n \t}\n \n+\tif ($askpasswordcount < 1) {\n+\t\t$askpasswordcount = 1\n+\t}\n+\n \tif (!defined $smtp_encryption) {\n \t\tmy $enc = Git::config(@repo, \"$prefix.smtpencryption\");\n \t\tif (defined $enc) {\n@@ -1069,17 +1075,21 @@ sub smtp_auth_maybe {\n \t# TODO: Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n-\t$auth = Git::credential({\n-\t\t'protocol' => 'smtp',\n-\t\t'host' => smtp_host_string(),\n-\t\t'username' => $smtp_authuser,\n-\t\t# if there's no password, \"git credential fill\" will\n-\t\t# give us one, otherwise it'll just pass this one.\n-\t\t'password' => $smtp_authpass\n-\t}, sub {\n-\t\tmy $cred = shift;\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n-\t});\n+\tfor my $i (1 .. $askpasswordcount) {\n+\t\t$auth = Git::credential({\n+\t\t\t'protocol' => 'smtp',\n+\t\t\t'host' => smtp_host_string(),\n+\t\t\t'username' => $smtp_authuser,\n+\t\t\t# if there's no password, \"git credential fill\" will\n+\t\t\t# give us one, otherwise it'll just pass this one.\n+\t\t\t'password' => $smtp_authpass\n+\t\t}, sub {\n+\t\t\tmy $cred = shift;\n+\t\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t});\n+\n+\t\tlast if ($auth);\n+\t}\n \n \treturn $auth;\n }\n-- \n1.8.4.2\n"},{"id":"230365","messageId":"1384084613-12260-1-git-send-email-silvio@port1024.net","threadId":"35314","inReplyTo":"1383992508-2097-1-git-send-email-silvio@port1024.net","subject":"[PATCH v2] git-send-mail: ask smtp password several times","fromName":"","fromEmail":"silvio@port1024.net","sentAt":"2013-11-10T11:56:52Z","receivedAt":"2013-11-10T11:56:52Z","isPatch":true,"sender":{"key":"silvio@port1024.net","avatar":null},"body":"Hi,\n\nHere the second version of my patch.\n\nWith this patch we are able to enter more than one time the password for the\nsmtp server. Before this patch we had only one trial.\n\nI'm a newbie in perl. Please have attention to newbie failures for perl.\n\nTo me it was not possible to implement a test for my changes. Has someone\nsuggestion to this?\n\nv2:\n\t* rebase to master-branch (0ecd94d7d728)\n\t* fix some perl-styles\n\nv1:\n\t* first version. patch was based on next branch\n\nCheers,\n\tSilvio\n"},{"id":"230366","messageId":"1384084613-12260-2-git-send-email-silvio@port1024.net","threadId":"35314","inReplyTo":"1384084613-12260-1-git-send-email-silvio@port1024.net","subject":"[[PATCH v2]] git-send-email: Added the ability to query the number of smtp password questions","fromName":"","fromEmail":"silvio@port1024.net","sentAt":"2013-11-10T11:56:53Z","receivedAt":"2013-11-10T11:56:53Z","isPatch":true,"sender":{"key":"silvio@port1024.net","avatar":null},"body":"From: Silvio F \n\nWith this patch \"git-send-mail\" ask a configurable number of questions to\ninput the smtp password. Without this patch we have only one trial.\n\nSigned-off-by: Silvio F \n---\n Documentation/git-send-email.txt | 4 ++++\n git-send-email.perl | 32 +++++++++++++++++++++-----------\n 2 files changed, 25 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex f0e57a5..ac993d6 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -364,6 +364,10 @@ sendemail.confirm::\n \tone of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'\n \tin the previous section for the meaning of these values.\n \n+sendmail.askpasswordcount::\n+\tNumber of times the smtp password can be entered before sending mail is\n+\taborted. Default is 1.\n+\n EXAMPLE\n -------\n Use gmail as the smtp server\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 3782c3b..aeb2e6d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -203,6 +203,7 @@ my ($validate, $confirm);\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n+my ($askpasswordcount) = 1;\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -237,6 +238,7 @@ my %config_settings = (\n \"from\" => \\$sender,\n \"assume8bitencoding\" => \\$auto_8bit_encoding,\n \"composeencoding\" => \\$compose_encoding,\n+ \"askpasswordcount\" => \\$askpasswordcount\n );\n \n my %config_path_settings = (\n@@ -360,6 +362,10 @@ sub read_config {\n \t\t}\n \t}\n \n+\tif ($askpasswordcount < 1) {\n+\t\t$askpasswordcount = 1;\n+\t}\n+\n \tif (!defined $smtp_encryption) {\n \t\tmy $enc = Git::config(@repo, \"$prefix.smtpencryption\");\n \t\tif (defined $enc) {\n@@ -1069,17 +1075,21 @@ sub smtp_auth_maybe {\n \t# TODO: Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n-\t$auth = Git::credential({\n-\t\t'protocol' => 'smtp',\n-\t\t'host' => smtp_host_string(),\n-\t\t'username' => $smtp_authuser,\n-\t\t# if there's no password, \"git credential fill\" will\n-\t\t# give us one, otherwise it'll just pass this one.\n-\t\t'password' => $smtp_authpass\n-\t}, sub {\n-\t\tmy $cred = shift;\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n-\t});\n+\tfor my $i (1 .. $askpasswordcount) {\n+\t\t$auth = Git::credential({\n+\t\t\t'protocol' => 'smtp',\n+\t\t\t'host' => smtp_host_string(),\n+\t\t\t'username' => $smtp_authuser,\n+\t\t\t# if there's no password, \"git credential fill\" will\n+\t\t\t# give us one, otherwise it'll just pass this one.\n+\t\t\t'password' => $smtp_authpass\n+\t\t}, sub {\n+\t\t\tmy $cred = shift;\n+\t\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t});\n+\n+\t\tlast if ($auth);\n+\t}\n \n \treturn $auth;\n }\n-- \n1.8.4.2\n"},{"id":"230503","messageId":"xmqqwqkdh59o.fsf@gitster.dls.corp.google.com","threadId":"35314","inReplyTo":"1384084613-12260-2-git-send-email-silvio@port1024.net","subject":"Re: [[PATCH v2]] git-send-email: Added the ability to query the number of smtp password questions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-12T17:57:39Z","receivedAt":"2013-11-12T17:57:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"silvio@port1024.net writes:\n\n> From: Silvio F \n>\n> With this patch \"git-send-mail\" ask a configurable number of questions to\n> input the smtp password. Without this patch we have only one trial.\n>\n> Signed-off-by: Silvio F \n> ---\n\nI wonder if Git::credential (or even the underlying lower level\ncredential_fill() in the helper API) should give hints to the caller\nif calling it again may yield a different result. An interactive\nprompt may allow the user to mistype the password and then a later\ncall may return a correct one, but the .netrc helper will read from\nthe file and will return a fixed result, so there is no use calling\ncredential_fill() again. And in the latter case, you do not want to\nloop $askpasswordcount times.\n\nI also have to wonder if this logic belongs to git-send-email.\nSpecifically, I wonder if we can place the looping logic in\nGit::credential, so that other users of the library can take\nadvantage of it?\n\nThanks.\n\n[jc: cc'ed peff@ for thoughts on credential helper API]\n\n> Documentation/git-send-email.txt | 4 ++++\n> git-send-email.perl | 32 +++++++++++++++++++++-----------\n> 2 files changed, 25 insertions(+), 11 deletions(-)\n>\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index f0e57a5..ac993d6 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -364,6 +364,10 @@ sendemail.confirm::\n> \tone of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'\n> \tin the previous section for the meaning of these values.\n> \n> +sendmail.askpasswordcount::\n> +\tNumber of times the smtp password can be entered before sending mail is\n> +\taborted. Default is 1.\n> +\n> EXAMPLE\n> -------\n> Use gmail as the smtp server\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 3782c3b..aeb2e6d 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -203,6 +203,7 @@ my ($validate, $confirm);\n> my (@suppress_cc);\n> my ($auto_8bit_encoding);\n> my ($compose_encoding);\n> +my ($askpasswordcount) = 1;\n> \n> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n> \n> @@ -237,6 +238,7 @@ my %config_settings = (\n> \"from\" => \\$sender,\n> \"assume8bitencoding\" => \\$auto_8bit_encoding,\n> \"composeencoding\" => \\$compose_encoding,\n> + \"askpasswordcount\" => \\$askpasswordcount\n> );\n> \n> my %config_path_settings = (\n> @@ -360,6 +362,10 @@ sub read_config {\n> \t\t}\n> \t}\n> \n> +\tif ($askpasswordcount < 1) {\n> +\t\t$askpasswordcount = 1;\n> +\t}\n> +\n> \tif (!defined $smtp_encryption) {\n> \t\tmy $enc = Git::config(@repo, \"$prefix.smtpencryption\");\n> \t\tif (defined $enc) {\n> @@ -1069,17 +1075,21 @@ sub smtp_auth_maybe {\n> \t# TODO: Authentication may fail not because credentials were\n> \t# invalid but due to other reasons, in which we should not\n> \t# reject credentials.\n> -\t$auth = Git::credential({\n> -\t\t'protocol' => 'smtp',\n> -\t\t'host' => smtp_host_string(),\n> -\t\t'username' => $smtp_authuser,\n> -\t\t# if there's no password, \"git credential fill\" will\n> -\t\t# give us one, otherwise it'll just pass this one.\n> -\t\t'password' => $smtp_authpass\n> -\t}, sub {\n> -\t\tmy $cred = shift;\n> -\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n> -\t});\n> +\tfor my $i (1 .. $askpasswordcount) {\n> +\t\t$auth = Git::credential({\n> +\t\t\t'protocol' => 'smtp',\n> +\t\t\t'host' => smtp_host_string(),\n> +\t\t\t'username' => $smtp_authuser,\n> +\t\t\t# if there's no password, \"git credential fill\" will\n> +\t\t\t# give us one, otherwise it'll just pass this one.\n> +\t\t\t'password' => $smtp_authpass\n> +\t\t}, sub {\n> +\t\t\tmy $cred = shift;\n> +\t\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n> +\t\t});\n> +\n> +\t\tlast if ($auth);\n> +\t}\n> \n> \treturn $auth;\n> }\n"},{"id":"230513","messageId":"20131112203831.GB23418@sigill.intra.peff.net","threadId":"35314","inReplyTo":"xmqqwqkdh59o.fsf@gitster.dls.corp.google.com","subject":"Re: [[PATCH v2]] git-send-email: Added the ability to query the number of smtp password questions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-12T20:38:31Z","receivedAt":"2013-11-12T20:38:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 12, 2013 at 09:57:39AM -0800, Junio C Hamano wrote:\n\n> > With this patch \"git-send-mail\" ask a configurable number of questions to\n> > input the smtp password. Without this patch we have only one trial.\n> \n> I wonder if Git::credential (or even the underlying lower level\n> credential_fill() in the helper API) should give hints to the caller\n> if calling it again may yield a different result. An interactive\n> prompt may allow the user to mistype the password and then a later\n> call may return a correct one, but the .netrc helper will read from\n> the file and will return a fixed result, so there is no use calling\n> credential_fill() again. And in the latter case, you do not want to\n> loop $askpasswordcount times.\n\nIt would be pretty easy to add an \"interactive=true\" flag to the\ncredential response (patch below). Credential readers are supposed to\nignore elements that they don't understand. So storage helpers which are\ntold \"we got a password interactively\" can choose to use that\ninformation if they want, but current implementations will fall back to\nignoring it. Similarly, users of \"git credential fill\" can use it, but\nit won't hurt existing readers.\n\n> I also have to wonder if this logic belongs to git-send-email.\n> Specifically, I wonder if we can place the looping logic in\n> Git::credential, so that other users of the library can take\n> advantage of it?\n\nA very early draft of the credential code added looping, but I cut it\nout (I think before it even made it to the list). I don't recall the\nexact reason, but it was probably a combination of:\n\n 1. It's awkward to do at the credential layer in C, because you need\n input from the calling code on whether the credential worked or\n not. The perl Git::credential can take a callback, though, which\n means the credential code owns the outer loop, and it would be\n pretty easy to just loop on trying.\n\n 2. We already have a retry mechanism; it is called \"shell history\". :)\n\nThe second one is only somewhat tongue in cheek. If we were an\ninteractive program, retrying would be essential. But fetch and push\ntend to be very easy to simply re-run, as they perform only a single\naction that either works or does not. So I have not really seen anyone\nmake a request for the feature.\n\nI guess \"send-email\" does not (always) fall under the same category\nbecause the user may have put work into a cover letter, or filling\ninteractive fields. So I have no objection to adding it there, but I do\nagree we might as well put it in Git::credential.\n\n---\nHere's the \"interactive\" patch. It needs documentation and tests, and\nit would probably make sense to simplify the text bool to \"0|1\", just to\nmake life easier for other reader implementations.\n\ndiff --git a/credential.c b/credential.c\nindex e54753c..fccb944 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -126,6 +126,7 @@ static char *credential_ask_one(const char *what, struct credential *c,\n \n \tstrbuf_release(&desc);\n \tstrbuf_release(&prompt);\n+\tc->interactive = 1;\n \treturn xstrdup(r);\n }\n \n@@ -174,6 +175,8 @@ int credential_read(struct credential *c, FILE *fp)\n \t\t\tc->path = xstrdup(value);\n \t\t} else if (!strcmp(key, \"url\")) {\n \t\t\tcredential_from_url(c, value);\n+\t\t} else if (!strcmp(key, \"interactive\")) {\n+\t\t\tc->interactive = git_config_bool(\"interactive\", value);\n \t\t}\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n@@ -200,6 +203,8 @@ void credential_write(const struct credential *c, FILE *fp)\n \tcredential_write_item(fp, \"path\", c->path);\n \tcredential_write_item(fp, \"username\", c->username);\n \tcredential_write_item(fp, \"password\", c->password);\n+\tif (c->interactive)\n+\t\tcredential_write_item(fp, \"interactive\", \"true\");\n }\n \n static int run_credential_helper(struct credential *c,\ndiff --git a/credential.h b/credential.h\nindex 0c3e85e..c0e5cbc 100644\n--- a/credential.h\n+++ b/credential.h\n@@ -7,6 +7,7 @@ struct credential {\n \tstruct string_list helpers;\n \tunsigned approved:1,\n \t\t configured:1,\n+\t\t interactive:1,\n \t\t use_http_path:1;\n \n \tchar *username;\n"},{"id":"230536","messageId":"xmqqbo1pgvqy.fsf@gitster.dls.corp.google.com","threadId":"35314","inReplyTo":"20131112203831.GB23418@sigill.intra.peff.net","subject":"Re: [[PATCH v2]] git-send-email: Added the ability to query the number of smtp password questions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-12T21:23:17Z","receivedAt":"2013-11-12T21:23:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Nov 12, 2013 at 09:57:39AM -0800, Junio C Hamano wrote:\n>\n>> > With this patch \"git-send-mail\" ask a configurable number of questions to\n>> > input the smtp password. Without this patch we have only one trial.\n>> \n>> I wonder if Git::credential (or even the underlying lower level\n>> credential_fill() in the helper API) should give hints to the caller\n>> if calling it again may yield a different result. An interactive\n>> prompt may allow the user to mistype the password and then a later\n>> call may return a correct one, but the .netrc helper will read from\n>> the file and will return a fixed result, so there is no use calling\n>> credential_fill() again. And in the latter case, you do not want to\n>> loop $askpasswordcount times.\n>\n> It would be pretty easy to add an \"interactive=true\" flag to the\n> credential response (patch below). Credential readers are supposed to\n> ignore elements that they don't understand. So storage helpers which are\n> told \"we got a password interactively\" can choose to use that\n> information if they want, but current implementations will fall back to\n> ignoring it. Similarly, users of \"git credential fill\" can use it, but\n> it won't hurt existing readers.\n\nYeah, it may be a sensible way forward.\n\n>> I also have to wonder if this logic belongs to git-send-email.\n>> Specifically, I wonder if we can place the looping logic in\n>> Git::credential, so that other users of the library can take\n>> advantage of it?\n>\n> A very early draft of the credential code added looping, but I cut it\n> out (I think before it even made it to the list). I don't recall the\n> exact reason, but it was probably a combination of:\n>\n> 1. It's awkward to do at the credential layer in C, because you need\n> input from the calling code on whether the credential worked or\n> not. The perl Git::credential can take a callback, though, which\n> means the credential code owns the outer loop, and it would be\n> pretty easy to just loop on trying.\n\nI was wondering if it should go to Git::credential; I did not say it\nmight go to credential_fill() API, exactly for that \"fill does not\nknow if that was accepted\" reason. We are in full agreement, I\nthink.\n\n> I guess \"send-email\" does not (always) fall under the same category\n> because the user may have put work into a cover letter, or filling\n> interactive fields. So I have no objection to adding it there, but I do\n> agree we might as well put it in Git::credential.\n"}]}