Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileInfo.Length on symbolic links should return the size of the target of the link. #16805

Closed
briangru opened this issue Mar 23, 2016 · 9 comments
Assignees
Labels
area-System.IO design-discussion Ongoing discussion about design without consensus
Milestone

Comments

@briangru
Copy link

FileInfo.Length on a symbolic link in Windows returns 0, not the length of the file referred to by the link.

For a symbolic link, Windows’ FindFirstFile returns 0 bytes as the length. This may also be true with GetFileAttributesEx. So a pattern like this prints out 0 for the length of a file that happens to be a symbolic link:

    DirectoryInfo dir = new DirectoryInfo(".");
    foreach(var file in dir.GetFiles())
    {
        Console.WriteLine("File: {0}   Size: {1}", file.Name, file.Length);
    }

Fixing this will require opening the file then getting the length, which you can do with FileStream. Jeremy Kuhne points out it's best if we open then handle without requesting read or write permission (ie calling CreateFile with the right parameters). If you look at GetFileSizeEx, it's clear you need to specify FILE_READ_ATTRIBUTES when calling CreateFile (and we should use some appropriate sharing mode, like allowing reading and deleting. Maybe writing too - yes there's a race but for monotonically increasing log files the answer is not useless).

https://msdn.microsoft.com/en-us/library/windows/desktop/aa364957(v=vs.85).aspx

Repro by creating file.txt, then running "mklink link.txt file.txt" from an admin command prompt. Then use FileInfo.Length on link.txt.

@JeremyKuhne JeremyKuhne self-assigned this Mar 23, 2016
@stephentoub
Copy link
Member

FYI, we recently fixed the Unix behavior to match the Windows behavior:
dotnet/corefx#6591
something we received explicit requests for.

@JeremyKuhne
Copy link
Member

@stephentoub We don't have a way to get the target info now, do we? Was there a call for the size to be 0? What do you think about just the size matching the target?

@briangru
Copy link
Author

FYI - this is subtle stuff, causing certain build tools to fail not when run locally, but only on some internal build machines where we use symbolic links to point to files. I suggest either fixing FileInfo.Length or consider removing it from the .NET Framework. This is a subtle trap that will hurt tool authors.

Incidentally, we hit a similar bug in MSVCRT's fstat command in all builds prior to VC 2010. This meant a whole bunch of command line tools (most often compression-related) broke when we tried introducing symbolic links into an Azure Data Lake-like batch processing environment. We backed out our sym link usage because the risk was not easily quantifiable & required users to upgrade their tools in every job they submitted. But doing so meant we were stuck with a lingering pit of bugs and bad behavior for years. In retrospect, it was the wrong call. My view now? Fix these inconsistencies with symbolic links early and forcibly, and remove subtle pits of failure.

@briangru
Copy link
Author

BTW, I apologize for not testing symbolic links properly in System.IO when we added FileInfo around 2004. This was my bad.

@stephentoub
Copy link
Member

we hit a similar bug in MSVCRT's fstat command in all builds prior to VC 2010

stat is explicitly documented to follow symlinks. FileInfo hasn't been following symlinks since its introduction over a decade ago. I think there's a difference. FileInfo maps more closely to lstat.

Was there a call for the size to be 0? What do you think about just the size matching the target?

Not specifically, but rather that the FileInfo should generally represent the symlink rather than the target, as it does on Windows. Seems like it'd be inconsistent if we change the size to represent the target but keep the attributes representing the symlink, e.g. a symlink to a readonly file would show the target file's length but not readonly?

Changing FileInfo to follow symlinks would be a significant departure in observable behavior. And it's not clear to me that the behavior is wrong, just a choice. It's not "early" in that this behavior, while not documented to my knowledge, has been around for a long time. And I expect any change here would need to be made to desktop as well and quirked.

Adding some more folks that might be interested in the discussion:
@ianhays, @mjrousos, @bartonjs, @joshfree, @palladia, @andschwa, @AlexGhiondea

@joshfree joshfree assigned ianhays and unassigned JeremyKuhne Apr 15, 2016
@ianhays
Copy link
Contributor

ianhays commented Apr 18, 2016

I'm personally happy with where we landed in dotnet/corefx#6591 to match unix behavior to the windows behavior and I was happy with the Windows behavior before that. We did flip-flop on it a few times and it could definitely go either way, but in the end the back-compat benefits push the existing behavior to be a bit better imo.

@stephentoub
Copy link
Member

Pulling this back to 1.0.0-rtm. We should either fix it or close it by then.

@ianhays
Copy link
Contributor

ianhays commented May 3, 2016

I'm going to close this. The RTM deadline is coming up and there hasn't been any recent discussion in here that would justify changing the current behavior.

@ianhays ianhays closed this as completed May 3, 2016
@jhudsoncedaron
Copy link

@ianhays : It's pretty obvious you needed a new API anyway. Both behaviors should be reachable.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO design-discussion Ongoing discussion about design without consensus
Projects
None yet
Development

No branches or pull requests

6 participants