jonhadfield / python-hosts

a hosts file manager library written in python
MIT License
125 stars 33 forks source link

support duplicate names #39

Closed TheTesla closed 2 years ago

TheTesla commented 2 years ago

e. g.:

1.2.3.4 examle1.com managed
5.6.7.8 example2.com managed

adding a new entry:

9.10.11.12 example3.com managed

does not work - nothing is added - but, using force=True, it deletes all the existing entries

The bug may be here: https://github.com/jonhadfield/python-hosts/blob/f85397383e74512ba25ddaa496559e96ad993d75/python_hosts/hosts.py#L389

jonhadfield commented 2 years ago

Hi, this behaviour is intended. Having the same name against different IP addresses doesn't really make sense; I assume any OS would either choose the first match every time or complain.

TheTesla commented 2 years ago

Correct, the os chooses the first match! But, if i have no entry already matching the name and insert the whole data at once, your lib accepts it and I have the duplicated names in it. The other fact is: If I insert the entry like shown in my first post, "9.10.11.12 example3.com" won't be added, because "managed" is already in.

jonhadfield commented 2 years ago

I see your point, but not sure how changing the behaviour would improve it.

I could error on load, but that would be breaking behaviour for existing users that don't need to replace overlapping entries. I could error before applying, but force is meant to overwrite. I could overwrite only the first matching overlap, but that still results in a syntactically incorrect hosts file.

Have you got any suggestions?

TheTesla commented 2 years ago

No, multiple ip addresses per name are allowed:

https://superuser.com/questions/312240/expected-behavior-with-duplicate-nt-hosts-file-entries https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2003/cc787573(v=ws.10)?redirectedfrom=MSDN

so, add() should add the new entry without error. "force" is also a bad word for "overwrite". Maybe the concept here is a bit messed up. Maybe it should be the other way around: It should not force overwrite by name but by ip address.

See it that way: the hosts file is a dictionary of sets:

hostsFile = {"1.2.3.4": {"examle1.com", "managed"}, "5.6.7.8": {"example2.com", "managed"}}
def self..add(newEntriesDict, force=False):
    if force is True:
      self.hosts.update(newEntriesDict)
   else:
      self.hosts = {k: v+newEntriesDict[k] if k in newEntriesDict else v for k, v in self.hosts.items()}
jonhadfield commented 2 years ago

Thanks for the info.

The default behaviour is probably what most users would expect, i.e. no duplicates. It's also the safest, based on how many different ways OSes and apps treat duplicates, and the possible side-effects that could creep into apps that may use this library.

There's already an option to allow ip address duplicates when adding an entry, so the simplest solution is probably to add an option to allow duplicate names also. Would that work for you?

At some point I will look at a refresh of this library and that would involve changing parameters and defaults. I agree that "overwrite" would have been a better choice.

TheTesla commented 2 years ago

yes, this would be nice.

github-actions[bot] commented 2 years ago

Stale issue message