classification
Title: netrc does not work if $HOME is not set
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Dimitri Merejkowsky, berker.peksag, terry.reedy, vstinner, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-10-01 15:35 by Dimitri Merejkowsky, last changed 2017-11-25 10:39 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
netrc-use-expanduser.patch Dimitri Merejkowsky, 2016-10-01 15:35 review
netrc-use-expanduser-with-test.patch Dimitri Merejkowsky, 2016-10-02 14:50 review
Pull Requests
URL Status Linked Edit
PR 123 closed python-dev, 2017-02-15 20:32
PR 4537 merged berker.peksag, 2017-11-24 09:21
Messages (9)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2017-11-25 10:39
Thanks, Dimitri.
History
Date User Action Args
2017-11-25 10:39:33berker.peksagsetstatus: open -> closed
versions: - Python 2.7, Python 3.5, Python 3.6
messages: + msg306951

resolution: fixed
stage: patch review -> resolved
2017-11-25 10:37:26berker.peksagsetmessages: + msg306950
2017-11-24 09:21:04berker.peksagsetpull_requests: + pull_request4471
2017-02-15 20:32:48python-devsetpull_requests: + pull_request86
2016-11-28 05:08:39xiang.zhangsetnosy: + xiang.zhang
2016-10-18 21:26:53vstinnersetnosy: + vstinner
2016-10-18 19:58:51Dimitri Merejkowskysetmessages: + msg278932
2016-10-10 23:33:19terry.reedysetmessages: + msg278447
2016-10-10 18:09:55Dimitri Merejkowskysetmessages: + msg278438
2016-10-07 22:07:01terry.reedysetnosy: + terry.reedy
messages: + msg278276
2016-10-02 14:50:53Dimitri Merejkowskysetfiles: + netrc-use-expanduser-with-test.patch

messages: + msg277894
2016-10-02 04:00:57berker.peksagsetversions: - Python 3.3, Python 3.4
nosy: + berker.peksag

messages: + msg277847

stage: patch review
2016-10-01 15:35:54Dimitri Merejkowskycreate