agiledivider / vagrant-hostsupdater

MIT License
1.15k stars 130 forks source link

Hosts entries appended incorrectly to hosts file on Mac #141

Open Kellman3000 opened 7 years ago

Kellman3000 commented 7 years ago

Using 1.0.2 on a Mac, I found that if there is no blank line at the end of the hosts file, the plugin simply appends to the last line, rather than adding a new line. If there is a blank line already there, it appends to the blank line. MacOS 10.12.6

cgsmith commented 6 years ago

@Kellman3000 Can you test with version 1.1.0? If it is still an issue I will find out how to fix it.

kgillis-atmosol commented 6 years ago

I'm seeing this issue with 1.1.1.160 on Mac High Sierra.

ian-rose commented 6 years ago

I'm also seeing this issue with 1.1.1.160 on Mac High Sierra (10.13.5)

danepowell commented 6 years ago

Also seeing this with 1.1.1.160 on Mac High Sierra.

cgsmith commented 6 years ago

I would think it wouldn't be an issue unless there needs to be a \r\n? See this part of the code for the plugin:

https://github.com/cogitatio/vagrant-hostsupdater/blob/master/lib/vagrant-hostsupdater/HostsUpdater.rb#L156-L161

danepowell commented 6 years ago

I think I've figured out why this is happening. That code you linked to is never actually invoked on most systems, because the hosts file is owned by root and not writeable.

Instead, this line writes to the hosts file without a newline: https://github.com/cogitatio/vagrant-hostsupdater/blob/bdfa1c70bb808c848f7012322d4f5907ca83d84c/lib/vagrant-hostsupdater/HostsUpdater.rb#L143

FYI, this is easily reproducible on a standard Linux/Mac OS by saving your hosts file without a new line at the end, and then running vagrant up with hostsupdater enabled.

I haven't quite figured out how to solve this. The problem is that if you fix the conditional to actually work as intended, your hosts file quickly blows up with a whole bunch of trailing newlines. I'm not sure the best way to prevent those.

danepowell commented 6 years ago

I've opened a PR to fix this.

Note that this may actually be a dangerous bug. When vagrant hostsupdater appends to a hosts file without a newline, its output gets concatenated with the last line of the host file. Then when hostsupdater removes its lines, it would probably remove what used to be the last line of the file. That's a pretty destructive maneuver :fearful:

cgsmith commented 6 years ago

I'll take a look at #165 within the upcoming week. hostsupdater looks for the GUID to remove the line. So it will not matter where the line is in the hosts file - it does not remove the last line.

danepowell commented 6 years ago

@cgsmith actually I just confirmed via a test that Vagrant Hostsupdater will destructively modify your hosts file if it does not contain a trailing newline.

Easy to test, just add a line to the end of your hosts file that you don't care about and save it without a newline: 127.0.0.1 foobar

Now run vagrant up on something, and examine the hosts file with cat -A /etc/hosts: 127.0.0.1 foobar192.168.223.178 local.baz.com # VAGRANT: 95a9b2d48c7c4dfd09aeb13cee9c1d63 (bwd) / 797b93b2-98b4-4784-a529-89565f136b37$

Now run vagrant halt, and poof the foobar entry is removed from your hosts file.

cgsmith commented 6 years ago

I see what you mean. Thanks for the clarification