| msg277824 - (view) |
Author: Dimitri Merejkowsky (Dimitri Merejkowsky) * |
Date: 2016-10-01 15:35 |
| If $HOME is not set, netrc will raise an exception. Attached patch fixes the problem by using `os.path.expanduser` instead |
|
|
| msg277847 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-10-02 04:00 |
| Thanks for the patch, Dimitri. I think this is a reasonable improvement. However, since we are changing the behavior of the netrc() class, I'm not sure this can be considered as a bug fix. In any case, 3.3 and 3.4 are in security-fix-only mode so I'm going to remove them from the versions field. We need two things from you to move this forward: 1. A test. It should go in Lib/test/test_netrc.py. You can use env = support.EnvironmentVarGuard() env.unset('HOME') to test the new behavior. 2. A CLA form. You can sign it online at https://www.python.org/psf/contrib/contrib-form/ |
|
|
| msg277894 - (view) |
Author: Dimitri Merejkowsky (Dimitri Merejkowsky) * |
Date: 2016-10-02 14:50 |
| > However, since we are changing the behavior of the netrc() class, I'm not sure this can be considered as a bug fix. I was not sure either. The patch does change behavior in subtle ways... I've added a new patch, and sent the CLA form. Thanks for your time! |
|
|
| msg278276 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2016-10-07 22:07 |
| On Windows, HOME is both split into two variables and replaced by USERPROFILE. C:\Users\Terry>set HOME HOMEDRIVE=C: HOMEPATH=\Users\Terry C:\Users\Terry>set USERPROFILE USERPROFILE=C:\Users\Terry So if it make sense to run this on Windows*, I would call it a bug. * I am guessing that Windows' FTP clients do not use .netrc. The doc says "the file .netrc in the user’s home directory will be read." without qualification by "if $HOME is set". I don't remember how os.path.expanduser might otherwise find the home directory on unix and don't know what unix users might reasonbly expect. |
|
|
| msg278438 - (view) |
Author: Dimitri Merejkowsky (Dimitri Merejkowsky) * |
Date: 2016-10-10 18:09 |
| > if it make sense to run this on Windows I found this issue while running cross-platform code. I needed to store some credentials, did not mind having them in plain-text and I thought .netrc was a good place for this. (did not need to re-invent the wheel ...) > don't know what unix users might reasonbly expect. Well, I guess as a bonus of this patch we could add a link to the `os.path.expanduser` section[1] in `netrc` documentation. The behavior when $HOME is not set is properly documented there. [1] https://docs.python.org/3/library/os.path.html#os.path.expanduser |
|
|
| msg278447 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2016-10-10 23:33 |
| FWIW, some history: The $HOME lookup was in Guido's original 1998-12-22 patch: if not file: file = os.path.join(os.environ['HOME'], ".netrc") The lookup was wrapped in try-except 4 years later to give a 'consistent error message'. Module ntpath dates back to 1994. Expanduser only used HOMEDRIVE and HOMEPATH. Guido added support for HOME in 1997. Support for USERPROFILE was added a decade later. Since the module is NOT marked 'Unix-only', I think it a bug to not use os.path.expanduser. Larry, Ned, do either of you have an opinion on whether the change should be made in current versions or only 3.7? |
|
|
| msg278932 - (view) |
Author: Dimitri Merejkowsky (Dimitri Merejkowsky) * |
Date: 2016-10-18 19:58 |
| During review SilentGhost suggested that maybe a test was not essential. In any case, I think patching documentation about the new behavior won't hurt. Do you want me to do that? |
|
|
| msg306950 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-11-25 10:37 |
| New changeset 8d9bb11d8fcbf10cc9b1eb0a647bcf3658a4e3dd by Berker Peksag in branch 'master': bpo-28334: netrc() now uses expanduser() to find .netrc file (GH-4537) https://github.com/python/cpython/commit/8d9bb11d8fcbf10cc9b1eb0a647bcf3658a4e3dd |
|
|
| msg306951 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-11-25 10:39 |
| Thanks, Dimitri. |
|
|