msg130193 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-03-06 21:14 |
I have a netrc file with two entries for the same host. The netrc module only returns the last entry.
$ cat > .netrc
machine host.com
login foo
password foo
machine host.com
login bar
password bar
$ python -c 'import netrc; print netrc.netrc()'
machine host.com
login 'bar'
password 'bar'
My Linux ftp clients (ftp, lftp) always use the first entry for a given host. Also lftp can use the password from the proper entry if I supply the host and login.
With the netrc module in Python 2.6.6 (as tested on Debian Squeeze), I can only retrieve the last entry.
|
msg130618 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-03-11 21:43 |
Thanks for the report. Would you like to work on a patch? Guidelines are found at http://docs.python.org/devguide
|
msg130686 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-03-12 16:04 |
I could spend some time on a small patch.
Here are the changes I would consider:
- netrc.authenticators() could return the first entry matching a given host (now it returns the last)
- it could also accept an extra parameter (with default=None), a login to select the right account/password for a given host
- it could also return a list of entries (tuples) matching the supplied host (now it always returns only one tuple), or a new method could do that
IMHO the last change would make the overall behaviour more consistent (BTW note the plural for "authenticators"), but it could also break existing apps.
Comments?
|
msg130689 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-03-12 16:45 |
I think whatever change we make it should be backward compatible, even though that is a bit annoying (especially given the current method name). So either a parameter that selects the new behavior and defaults to the old, or a new method that returns a list, IMO.
|
msg132421 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-03-28 21:15 |
So I finally cooked a little patch for netrc.py in python 2.6.
The patch extends netrc.authenticators() with an extra parameter to select a login name, but otherwise the behaviour remains the same (still returns the last entry for a given host).
Lightly tested, works for me.
|
msg132422 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2011-03-28 21:20 |
Can you add your tests to Lib/test/test_netrc.py?
|
msg132434 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-03-28 22:27 |
Good that you mentioned the official tests, they let me see that netrc.hosts is actually part of the API, and my first patch broke it.
Here is an updated patch, with extra tests.
|
msg137218 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-29 20:58 |
Ping? A patch is available for review.
|
msg137267 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-05-30 13:30 |
Looks good. One remark: instead of using keys() + getitem in Lib/netrc.py:109, you can use this: for host, attrs in self.allhosts.items()
|
msg137276 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-30 14:04 |
You are suggesting something like this, I suppose?
--- a/Lib/netrc.py
+++ b/Lib/netrc.py
@@ -105,8 +105,8 @@ class netrc:
def __repr__(self):
"""Dump the class data in the format of a .netrc file."""
rep = ""
- for host in self.allhosts.keys():
- for attrs in self.allhosts[host]:
+ for (host, attrlist) in self.allhosts.items():
+ for attrs in attrlist:
rep = rep + "machine "+ host + "\n\tlogin " + repr(attrs[0]) + "\n"
if attrs[1]:
rep = rep + "account " + repr(attrs[1])
|
msg137277 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-05-30 14:05 |
Exactly, minus the parens (not needed).
|
msg137278 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-30 14:14 |
Patch slightly updated after Eric's comments.
|
msg137279 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-05-30 14:18 |
I’m surprised self.assert_ does not produce a DeprecationWarning (in favor of assertTrue). The tests should use assertEqual(expected, computed) anyway, to let developers see a useful diff when the test fails. Could you change this in the tests?
|
msg137280 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-30 14:19 |
Patch formatting changed to be more review-friendly (looks like MQ-style patch isn't?), otherwise same as 2011-05-30 16:14.
|
msg137282 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-30 14:24 |
Eric: yes I can look into the asserts, but note I generated and tested my patch from a checkout of 2.6 (see my first report), so maybe that's why I didn't see any warning.
|
msg137283 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-05-30 14:32 |
Ah, okay. Your patch needs to apply to 2.7 or 3.2, which use assertEqual and not assert_.
|
msg137288 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-30 15:09 |
Here is a patch against 2.7.
|
msg137289 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-05-30 15:11 |
Great! Could you expand the docstrings and reST documentation to mention the new behavior?
|
msg137341 - (view) |
Author: Jean-Marc Saffroy (saffroy) |
Date: 2011-05-30 22:51 |
Additional patch for docstrings and documentation. Applies on top of previous patch.
|
msg137449 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-01 17:49 |
Looks good, thanks.
|
msg138071 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2011-06-10 10:30 |
This should not be applied until after the patch for #12009 is applied, at which point the test will need to be adjusted. #12009 completely refactors the netrc test suite.
I wonder if the 'hosts' attribute should be deprecated, but let's leave that discussion for another day :)
Since this requires an API change, it is a feature and can only go into 3.3. I've adjusted 'versions' accordingly.
|
msg213229 - (view) |
Author: Lucas Hoffmann (luc) |
Date: 2014-03-12 09:21 |
What is the status of this issue?
On version 3.3 I still can not get more than one entry per host. The stopping issue #12009 seems to be closed.
(Sorry, if I violate some etiquette by bumping this, just tell me.)
|
msg213247 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2014-03-12 11:02 |
This patch hasn’t been committed. The comment before yours says that it needs to be edited a bit.
|
msg213955 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2014-03-18 10:56 |
I've updated the tests to match the changes in issue 12009 and documentation a bit.
|
msg263699 - (view) |
Author: Frew Schmidt (Frew Schmidt) |
Date: 2016-04-18 22:42 |
This issue is causing a problem in OfflineIMAP. I wrote a patch that I think is a little simpler than the existing one, including tests and docs, though honestly I don't care one way or another. See attached. Note that the patch is for Python 2.7 but it mostly applies to 3.5 just fine. Any chance we could get mine or the other one applied?
Thanks
|
msg358788 - (view) |
Author: Tibor Baranya (arsyee) * |
Date: 2019-12-22 10:23 |
As I understand this issue is there up to 3.8 (and will live into 3.9). As urllib3 supports netrc and requests uses urllib3, this issue can potentially cause a lot of inconsistencies. In case of multile entries (with no user provided), curl and wget takes the first entry, while cpython netrc takes the last.
I see there is a proposed implementation, but I couldn't find it's pull request on github. Is there any chance this will be resolved in the near future?
Alternatively, I can create a pull request, but I only bother if there is an actual intention to fix the current behavior.
|
msg359297 - (view) |
Author: Tibor Baranya (arsyee) * |
Date: 2020-01-04 17:55 |
Just for further clarification on how .netrc is supposed to work, I tested some other implementations. Sorry if this comment gets too long, but it seems netrc is not really a standardized solution.
In my last comment I referred curl and wget, but of course the original implementation is ftp (which points to the netrc manual). The manual itself doesn't talk about priorities, but all the mentioned 3 tools are in common in that they take the first entry if the same machine is in the netrc file multiple times.
When trying to force using the second entry, though, breaks this consistency quickly: none of the above tools seem to support a query with host and login name provided.
ftp: doesn't support providing a user name from shell (and doesn't use netrc when opening a connection interactively)
curl: if username is provided, it interactively asks for a password and uses that (which may be an issue on my end, as source code suggested there is a login-based lookup)
wget: if username is provided without a password, it doesn't send a auth information at all (also wget doesn't send default auth information for http, which behavior can be altered somehow)
Actually these results suprised me, as I remembered I used host+login based lookup previously. And the answer was git clone via https: git's implementation is exactly what I would expect:
* if only host is provided, the first entry is used
* if both host and login is provided, it looks up the password for the appropriate login
* if no matching machine entry can be found, it uses the default entry
* it also supports login matching for the default entry
I'm about the send a pull request with my understanding on how netrc should work. Of course all comments are welcome, and I'm happy to alter the implementation (or break it up to smaller pieces if necessary). My priorities were:
* Not to break backward compatibility.
* Provide a way to consistently pick up the first entry, without the need to make modifications in all of the dependent libraries (see doc change).
* __repr__() had a bug: for the default entry, it gives 'machine default', instead of 'default'.
* another bug: 'machine default' is actually not supported, as the netrc.hosts dictionary cannot make a difference between the default entry and a machine called default (this has a partial fixed)
As r.david.murray mentioned, the hosts dictionary should probably be deprecated.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:14 | admin | set | github: 55625 |
2020-01-04 19:47:56 | Frew Schmidt | set | nosy:
- Frew Schmidt
|
2020-01-04 18:09:07 | arsyee | set | pull_requests:
+ pull_request17249 |
2020-01-04 17:55:30 | arsyee | set | messages:
+ msg359297 |
2019-12-22 10:23:49 | arsyee | set | nosy:
+ arsyee
messages:
+ msg358788 versions:
+ Python 3.6, Python 3.7, Python 3.8 |
2018-01-16 13:17:37 | Yuri.Bochkarev | set | nosy:
+ Yuri.Bochkarev
|
2016-04-18 22:42:57 | Frew Schmidt | set | files:
+ netrc.patch versions:
+ Python 2.7 nosy:
+ Frew Schmidt
messages:
+ msg263699
|
2014-03-18 10:56:31 | berker.peksag | set | files:
+ issue11416.diff versions:
+ Python 3.5, - Python 3.3 nosy:
+ berker.peksag
messages:
+ msg213955
|
2014-03-12 11:02:02 | eric.araujo | set | messages:
+ msg213247 |
2014-03-12 09:21:13 | luc | set | nosy:
+ luc messages:
+ msg213229
|
2011-06-10 10:30:56 | r.david.murray | set | type: behavior -> enhancement messages:
+ msg138071 versions:
- Python 2.7, Python 3.2 |
2011-06-01 17:49:16 | eric.araujo | set | messages:
+ msg137449 |
2011-05-30 22:51:07 | saffroy | set | files:
+ netrc-doc.patch
messages:
+ msg137341 |
2011-05-30 15:11:24 | eric.araujo | set | messages:
+ msg137289 |
2011-05-30 15:10:53 | eric.araujo | set | files:
- netrc.patch |
2011-05-30 15:10:51 | eric.araujo | set | files:
- netrc.patch |
2011-05-30 15:10:48 | eric.araujo | set | files:
- netrc.patch |
2011-05-30 15:09:39 | saffroy | set | files:
+ netrc.patch
messages:
+ msg137288 |
2011-05-30 14:32:18 | eric.araujo | set | messages:
+ msg137283 |
2011-05-30 14:24:29 | saffroy | set | messages:
+ msg137282 |
2011-05-30 14:19:34 | saffroy | set | files:
+ netrc.patch
messages:
+ msg137280 |
2011-05-30 14:18:37 | eric.araujo | set | messages:
+ msg137279 |
2011-05-30 14:14:01 | saffroy | set | files:
+ netrc.patch
messages:
+ msg137278 |
2011-05-30 14:05:53 | eric.araujo | set | messages:
+ msg137277 versions:
- Python 3.1 |
2011-05-30 14:04:35 | saffroy | set | messages:
+ msg137276 |
2011-05-30 13:30:57 | eric.araujo | set | messages:
+ msg137267 |
2011-05-29 21:10:05 | brian.curtin | set | keywords:
+ needs review stage: patch review |
2011-05-29 20:58:17 | saffroy | set | messages:
+ msg137218 |
2011-05-29 20:57:47 | saffroy | set | files:
- netrc.patch |
2011-03-28 22:27:28 | saffroy | set | files:
+ netrc.patch
messages:
+ msg132434 |
2011-03-28 21:20:35 | brian.curtin | set | nosy:
+ brian.curtin messages:
+ msg132422
|
2011-03-28 21:15:41 | saffroy | set | files:
+ netrc.patch keywords:
+ patch messages:
+ msg132421
|
2011-03-12 16:45:06 | r.david.murray | set | messages:
+ msg130689 |
2011-03-12 16:04:08 | saffroy | set | messages:
+ msg130686 |
2011-03-11 21:43:14 | eric.araujo | set | nosy:
+ rhettinger, eric.araujo
messages:
+ msg130618 versions:
+ Python 3.1, Python 2.7, Python 3.2, Python 3.3, - Python 2.6 |
2011-03-06 23:29:05 | r.david.murray | set | nosy:
+ r.david.murray
|
2011-03-06 21:14:07 | saffroy | create | |