Don't use File.OpenWrite when trying to overwrite a file #93744
Conversation
File.OpenWrite will open an existing file which means if you write data that is shorter than the existing data the file will be corrupt. This is documented on https://learn.microsoft.com/en-us/dotnet/api/system.io.file.openwrite > The OpenWrite method opens a file if one already exists for the file path, or creates a new file if one does not exist. For an existing file, it does not append the new text to the existing text. Instead, it overwrites the existing characters with the new characters. If you overwrite a longer string (such as "This is a test of the OpenWrite method") with a shorter string (such as "Second run"), the file will contain a mix of the strings ("Second runtest of the OpenWrite method").
akoeplinger
requested review from
marek-safar,
radical and
MichalStrehovsky
as code owners
dotnet-issue-labeler
bot
added
the
needs-area-label
akoeplinger
added
the
area-Tools-ILLink
|
Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas Issue DetailsFile.OpenWrite will open an existing file which means if you write data that is shorter than the existing data the file will be corrupt. This is documented on https://learn.microsoft.com/en-us/dotnet/api/system.io.file.openwrite
The fix is to use the I noticed this recently when using the illink feature to dump a dependency analysis xml, the next time I ran the build after making a change that'd reduce dependencies the xml would get corrupted since it wrote less data than the existing file had.
|
akoeplinger
removed
the
needs-area-label
|
Good catch! |
| @@ -28,7 +28,7 @@ public ILAssemblyGeneratingMethodDebugInfoProvider(string fileName, DebugInforma | |||
| { | |||
| _wrappedProvider = wrappedProvider; | |||
| _fileName = fileName; | |||
| _tw = new StreamWriter(File.OpenWrite(fileName)); | |||
| _tw = new StreamWriter(new FileStream(fileName, FileMode.Create, FileAccess.Write, FileShare.None)); | |||
There was a problem hiding this comment.
More compact alternative:
| _tw = new StreamWriter(new FileStream(fileName, FileMode.Create, FileAccess.Write, FileShare.None)); | |
| _tw = new StreamWriter(File.Create(fileName)); |
There was a problem hiding this comment.
Not that it matters here but File.Create() uses FileAccess.ReadWrite instead of FileAccess.Write so it is slightly different.
I'll keep this as is to avoid restarting CI but let me know if you feel strongly.
|
we should probably backport the parts that apply to 8.0 |
The nativeaot part is related to producing a file used to debug the compiler. The code is under a non-public switch. The risk is pretty much none, but it's also not important for 8.0. This obviously only applies to that part of this pr. |
|
Backported the relevant changes in #93854 |
ghost
locked as resolved and limited conversation to collaborators
File.OpenWrite will open an existing file which means if you write data that is shorter than the existing data the file will be corrupt. This is documented on https://learn.microsoft.com/en-us/dotnet/api/system.io.file.openwrite
The fix is to use the
FileMode.Createoption in cases where the intention is to create a new file or overwrite an existing one.I noticed this recently when using the illink feature to dump a dependency analysis xml, the next time I ran the build after making a change that'd reduce dependencies the xml would get corrupted since it wrote less data than the existing file had.
There were a couple instances across the codebase, this fixes them.