richardszalay / pshosts

PowerShell cmdlets for modifying the hosts file on Windows, Linux, and macOS
MIT License
119 stars 18 forks source link

hosts file gets deleted when there's contention #4

Closed NickCraver closed 7 years ago

NickCraver commented 7 years ago

We see this intermittently in our scripts - code is fairly simple, we're just adding some aliases:

# Add hosts file entry
Write-Host "  Creating HOSTS entry for $baseHost to 127.0.0.1"
Get-HostEntry | Where-Object { $_.Name -eq $baseHost } | Remove-HostEntry
Add-HostEntry -Name $baseHost -Address 127.0.0.1 -Comment "auto-generated by dev-local-setup" | Out-Null

But when this a contention crash happens, the hosts file is gone at the end which is quite a setback. Here's the exception:

Error Occured: System.UnauthorizedAccessException: Access to the path 'C:\WINDOWS\system32\drivers\etc\hosts' is denied.
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
   at System.IO.FileInfo.OpenWrite()
   at RichardSzalay.Hosts.HostsFile.Save()
   at RichardSzalay.Hosts.Powershell.WriteHostEntryCommandBase.EndProcessing()
   at System.Management.Automation.CommandProcessorBase.Complete()
Details:

Exception             : System.UnauthorizedAccessException: Access to the path 'C:\WINDOWS\system32\drivers\etc\hosts' is denied.
                           at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
                           at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean
                        useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String
                        msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
                           at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
                           at System.IO.FileInfo.OpenWrite()
                           at RichardSzalay.Hosts.HostsFile.Save()
                           at RichardSzalay.Hosts.Powershell.WriteHostEntryCommandBase.EndProcessing()
                           at System.Management.Automation.CommandProcessorBase.Complete()
TargetObject          :
CategoryInfo          : NotSpecified: (:) [Add-HostEntry], UnauthorizedAccessException
FullyQualifiedErrorId : System.UnauthorizedAccessException,RichardSzalay.Hosts.Powershell.AddHostEntryCommand
ErrorDetails          :
InvocationInfo        : System.Management.Automation.InvocationInfo
ScriptStackTrace      : at Register-Websites, C:\code\dev-local-setup\Common\StackModule\Register-Websites.psm1: line 137
                        at <ScriptBlock>, C:\code\dev-local-setup\Common\StackModule\Setup-Core.psm1: line 162
                        at Out-Menu, C:\code\dev-local-setup\Common\StackModule\CommonFunctions.psm1: line 153
                        at Out-Menu, C:\code\dev-local-setup\Common\StackModule\CommonFunctions.psm1: line 146
                        at <ScriptBlock>, C:\code\dev-local-setup\Common\Setup.ps1: line 55
PipelineIterationInfo : {}
PSMessageDetails      :

Often, this will fail on the first access and not delete hosts, but if it happens during a run, hosts gets deleted. So it appears there's a in-active-change scenario that prevents it from saving correctly? I haven't had time to dig, but reliably reproducing this in both Windows 8.1 and 10.

NickCraver commented 7 years ago

Looking at FileInfoResource.cs, I see:

public Stream OpenWrite()
{
    this.file.Delete();

    return this.file.OpenWrite();
}

Is there a reason to do a .Delete()? I'm assuming this is to workaround the append behavior of .OpenWrite(). This is causing the issue if unable to re-create the file. Instead, you can use File.Create to overwrite a file if it exists, see this question for more examples.

richardszalay commented 7 years ago

Thanks Nick, I'll have a look into this. Tbh, I can't remember for the life of my why I would have added a Delete before the OpenWrite - this project is an evolution of an IIS GUI extension I wrote more than 7 years ago, and the core hosts library came over with very little changes.

richardszalay commented 7 years ago

By the way, this line:

Get-HostEntry | Where-Object { $_.Name -eq $baseHost } | Remove-HostEntry

Can be simplified to:

Get-HostEntry $baseHost | Remove-HostEntry

Or even:

Remove-HostEntry $baseHost
NickCraver commented 7 years ago

Tbh, I can't remember for the life of my why I would have added a Delete before the OpenWrite

FWIW, normally I see something like this happen because .OpenWrite() is append instead of overwrite, probably that?

On the suggestions: Does it not throw if not found in the other ways above? The current approach is a if-it's-not-there-don't-care...but I've since refactored to checking equality and not touching the file (to workaround the delete bug).

Thanks for taking a look at this!

richardszalay commented 7 years ago

This should be all sorted now in v1.1.1

Let me know if you run into any more issues!

richardszalay commented 7 years ago

Also, 1.2 (not quite ready yet) adds support for some new syntax that will clean up your usecase. Test-HostEntry checks if it exists, and Add-HostEntry -Loopback looks cleaner than "127.0.0.1".

richardszalay commented 7 years ago

1.2 is out, and also includes support for Set-HostEntry -Force which adds the entry if it's missing.