password-cache for smtpmail.el

password-cache for smtpmail.el

[Justus-bulk]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Hi -

Following up to an exchange I had almost two years ago:

Simon Josefsson wrote on Tue, 04 Apr 2006 15:34:24 +0200:

> ... The proper fix is for smtpmail.el to use password.el.

I attach a simple patch that does this. I tested it with emacs 22.1.

The patch also applies against the latest CVS (MAIN/HEAD) version
1.97, except that "password-cache" should be autoloaded instead of
"password" for recent development emacsen.

As a password key I simply reuse the prompt, as it appears to contain
just the right information.

The current version of smtpmail.el does not remember the password if
.authinfo is used. I removed this condition because it prevents
password caching for people who do not store passwords in .authinfo,
and I do not see any harm caused by removing it.

Could you test and apply the patch?

Thanks,
Justus


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: smtpmail-password-cache.diff --]
[-- Type: text/x-diff, Size: 1960 bytes --]

*** /home/piater/share/emacs/lisp/smtpmail-e22.1.el	2008-02-13 10:29:28.911165917 +0100
--- /home/piater/share/emacs/lisp/smtpmail.el	2008-02-13 09:46:37.914414498 +0100
***************
*** 78,83 ****
--- 78,84 ----
  (autoload 'netrc-parse "netrc")
  (autoload 'netrc-machine "netrc")
  (autoload 'netrc-get "netrc")
+ (autoload 'password-read "password") ; for new emacsen: "password-cache"
  
  ;;;
  (defgroup smtpmail nil
***************
*** 546,557 ****
                               (netrc-get hostentry "password"))))
  		 (smtpmail-find-credentials
  		  smtpmail-auth-credentials host port)))
  	 (passwd (when cred
  		   (or (smtpmail-cred-passwd cred)
! 		       (read-passwd
! 			(format "SMTP password for %s:%s: "
! 				(smtpmail-cred-server cred)
! 				(smtpmail-cred-port cred))))))
  	 ret)
      (when (and cred mech)
        (cond
--- 547,558 ----
                               (netrc-get hostentry "password"))))
  		 (smtpmail-find-credentials
  		  smtpmail-auth-credentials host port)))
+ 	 (prompt (when cred (format "SMTP password for %s:%s: "
+ 				    (smtpmail-cred-server cred)
+ 				    (smtpmail-cred-port cred))))
  	 (passwd (when cred
  		   (or (smtpmail-cred-passwd cred)
! 		       (password-read prompt prompt))))
  	 ret)
      (when (and cred mech)
        (cond
***************
*** 621,629 ****
         (t
  	(error "Mechanism %s not implemented" mech)))
        ;; Remember the password.
!       (when (and (not (stringp smtpmail-auth-credentials))
! 		 (null (smtpmail-cred-passwd cred)))
! 	(setcar (cdr (cdr (cdr cred))) passwd)))))
  
  (defun smtpmail-via-smtp (recipient smtpmail-text-buffer)
    (let ((process nil)
--- 622,629 ----
         (t
  	(error "Mechanism %s not implemented" mech)))
        ;; Remember the password.
!       (when (null (smtpmail-cred-passwd cred))
! 	(password-cache-add prompt passwd)))))
  
  (defun smtpmail-via-smtp (recipient smtpmail-text-buffer)
    (let ((process nil)

Re: password-cache for smtpmail.el

[Bastien Guerry]

Justus-bulk@Piater.name writes:

> The current version of smtpmail.el does not remember the password if
> .authinfo is used. I removed this condition because it prevents
> password caching for people who do not store passwords in .authinfo,
> and I do not see any harm caused by removing it.
>
> Could you test and apply the patch?

I have tested this, looks fine.  I applied the patch.

-- 
Bastien

Re: password-cache for smtpmail.el

[Glenn Morris]

Bastien Guerry wrote:

> I have tested this, looks fine.

I don't see how it can be, without notice being paid to this comment

  The patch also applies against the latest CVS (MAIN/HEAD) version
  1.97, except that "password-cache" should be autoloaded instead of
  "password" for recent development emacsen.

> I applied the patch.

The ChangeLog entry should be under the name of the patch author, not
yours. You should check for a copyright assignment before installing
code written by others. (See "Summary for the impatient" in
admin/notes/copyright.) Less than 10-15 lines (cumulative, over all
patches) is OK to install without legal papers, but should be marked
as "(tiny change)". See ChangeLog for examples.

And again, you probably want to wait more than 12 hours to allow
people to comment.

Re: password-cache for smtpmail.el

[Juanma Barranquero]

On Feb 13, 2008 11:23 PM, Glenn Morris <rgm@gnu.org> wrote:

> You should check for a copyright assignment before installing
> code written by others.

How? The copyright files are not public. I've usually used the
changelogs to see whether the person had previous non-tiny changes
committed, but that does not help with new contributors.

             Juanma

Re: password-cache for smtpmail.el

[Glenn Morris]

"Juanma Barranquero" wrote:

> On Feb 13, 2008 11:23 PM, Glenn Morris <rgm@gnu.org> wrote:
>
>> You should check for a copyright assignment before installing
>> code written by others.
>
> How? The copyright files are not public. 

It's straightforward to get a gnu.org account, then you can check.
Or you can ask someone - fsf-records@gnu.org if all else fails.

Re: password-cache for smtpmail.el

[Bastien]

Glenn Morris <rgm@gnu.org> writes:

> Bastien Guerry wrote:
>
>> I have tested this, looks fine.
>
> I don't see how it can be, without notice being paid to this comment
>
>   The patch also applies against the latest CVS (MAIN/HEAD) version
>   1.97, except that "password-cache" should be autoloaded instead of
>   "password" for recent development emacsen.

Fair enough, I will fix this.

Justus: is your full name "Justus Piater"?

> The ChangeLog entry should be under the name of the patch author, not
> yours.

I assumed that this was the case for changes about authors that have a
copyright assignement.  But since I read in admin/notes/copyright:

  "Even if the changes are trivial, consider if they combine with
  previous changes by the same author to make a non-trivial total."

I know understand the reason why the ChangeLog should always mention
the name of the author.  Thanks for the pointer.

> And again, you probably want to wait more than 12 hours to allow
> people to comment.

Right.  

-- 
Bastien

Re: password-cache for smtpmail.el

[Juanma Barranquero]

On Feb 14, 2008 2:58 AM, Glenn Morris <rgm@gnu.org> wrote:

> It's straightforward to get a gnu.org account

?

             Juanma

Re: password-cache for smtpmail.el

[Bastien]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> On Feb 13, 2008 11:23 PM, Glenn Morris <rgm@gnu.org> wrote:
>
>> You should check for a copyright assignment before installing
>> code written by others.
>
> How? The copyright files are not public. I've usually used the
> changelogs to see whether the person had previous non-tiny changes
> committed, but that does not help with new contributors.

Is there any reason why the copyright files are not public?  
I cannot think of any relevant privacy concern.

-- 
Bastien

Re: password-cache for smtpmail.el

[Glenn Morris]

"Juanma Barranquero" wrote:

> On Feb 14, 2008 2:58 AM, Glenn Morris <rgm@gnu.org> wrote:
>
>> It's straightforward to get a gnu.org account
>
> ?

To start the process, send a request to accounts@gnu.org, saying
you're an Emacs maintainer and you need a fencepost account. (See the
Preface of "Information for maintainers of GNU software".)

Re: password-cache for smtpmail.el

[Glenn Morris]

Bastien wrote:

> Is there any reason why the copyright files are not public?  
> I cannot think of any relevant privacy concern.

http://lists.gnu.org/archive/html/emacs-devel/2003-08/msg00030.html

rms:

  We don't want to publish all the info about our contributors
  that is in the file copyright.list.

Re: password-cache for smtpmail.el

[Justus-bulk]

Glenn Morris <rgm@gnu.org> wrote on Wed, 13 Feb 2008 17:23:48 -0500:

> Less than 10-15 lines (cumulative, over all patches) is OK to
> install without legal papers, but should be marked as "(tiny
> change)".

I think we have a "tiny change" here, and nothing cumulative.


Bastien <bzg@altern.org> wrote on Thu, 14 Feb 2008 01:58:47 +0000:

> Justus: is your full name "Justus Piater"?

Yes.

Thanks,
Justus

Re: password-cache for smtpmail.el

[Simon Josefsson]

Justus-bulk@Piater.name writes:

> Hi -
>
> Following up to an exchange I had almost two years ago:
>
> Simon Josefsson wrote on Tue, 04 Apr 2006 15:34:24 +0200:
>
>> ... The proper fix is for smtpmail.el to use password.el.
>
> I attach a simple patch that does this. I tested it with emacs 22.1.
>
> The patch also applies against the latest CVS (MAIN/HEAD) version
> 1.97, except that "password-cache" should be autoloaded instead of
> "password" for recent development emacsen.
>
> As a password key I simply reuse the prompt, as it appears to contain
> just the right information.
>
> The current version of smtpmail.el does not remember the password if
> .authinfo is used. I removed this condition because it prevents
> password caching for people who do not store passwords in .authinfo,
> and I do not see any harm caused by removing it.
>
> Could you test and apply the patch?

I noticed the patch was installed already, but for the record, I think
it looks fine.

Thanks!

/Simon

Re: password-cache for smtpmail.el

[Bastien]

Justus-bulk@Piater.name writes:

> Glenn Morris <rgm@gnu.org> wrote on Wed, 13 Feb 2008 17:23:48 -0500:
>
>> Less than 10-15 lines (cumulative, over all patches) is OK to
>> install without legal papers, but should be marked as "(tiny
>> change)".
>
> I think we have a "tiny change" here, and nothing cumulative.

I applied a modified[1] version of your patch and filled it under your
name in the ChangeLog.  Thanks,

Notes: 
[1]  Using password-cache.el instead of password.el

-- 
Bastien