Microsoft-ui-xaml: Question: Why the file dragged into a UWP app is read-only?

Created on 10 May 2020  路  17Comments  路  Source: microsoft/microsoft-ui-xaml

Hey folks,

I have a question bothers me for quite a while: "If user drag a storage file into a UWP app, it will be read-only by default. If you try to save changes on top of this file, you will get UnauthorizedAccessException, but why?"

For example, this is how you get the file(s) from DragEventArgs:

var storageItems = await args.DataView.GetStorageItemsAsync();

And this will be true for the files you dragged in:

public static bool IsFileReadOnly(StorageFile file)
{
    return (file.Attributes & Windows.Storage.FileAttributes.ReadOnly) != 0;
}

And you will see exception if you do this:

using (var stream = await file.OpenStreamForWriteAsync()) { }

This does not make any sense to me, because if user drag a file into an app, he or she is fully aware of the operation and should just give the app same permission just like what the file open picker does. Another approach would be asking for broadSystemAccess, but it makes little sense to me as well for just enabling write to a dragged file? (+ @soumyamahunt for visibility)

Last year, when I started my notepads app. I did some research online and found out that not only me, but also thousands of developers voted for this change on MS Voice (I am not sure if it is this forum) but never received official response from MSFT. Not only that, the website is now deprecated so I cannot find and paste the link here.

Now here comes the interesting part: I recently knows that from @yaichenbaum that if you use PathIO API to write, you can workaround this limitation. Then we have a community member (@Maickonn) sent out a PR for this workaround, let me paste the interesting workaround logic here:

if (IsFileReadOnly(file) || !await FileIsWritable(file))
{
    // For file(s) dragged into Notepads, they are read-only
    // StorageFile API won't work but can be overwritten by Win32 PathIO API (exploit?)
    // In case the file is actually read-only, WriteBytesAsync will throw UnauthorizedAccessException exception
    var content = encoding.GetBytes(text);
    var result = encoding.GetPreamble().Concat(content).ToArray();
    await PathIO.WriteBytesAsync(file.Path, result);
}
else // Use StorageFile API to save 
{
    using (var stream = await file.OpenStreamForWriteAsync())
    using (var writer = new StreamWriter(stream, encoding))
    {
        stream.Position = 0;
        await writer.WriteAsync(text);
        await writer.FlushAsync();
        // Truncate
        stream.SetLength(stream.Position);
    }
}

So basically PathIO.WriteBytesAsync can write to the file although it is marked as read-only for the StorageFile item. This looks like an exploit to me and I am very confused.

So, here are my questions:

  1. Why the file dragged into a UWP app is read-only, what is the reason behind it? Why we never change this behavior for so many years? This literally stops any notepad like or VSCode like document editing UWP app from working properly with dragged file. And further making them weaker comparing to their win32 counterparts.
  2. Why PathIO.WriteBytesAsync can write to a read-only StorageFile? I know I am using the "file.Path" here instead of the runtime object, but it should be caught by the file security model, isn't it? Or is this intended?
  3. Is there any plan for changing this behavior in the future or is there any plan to address it as part of WinUI 3.0 or post WinUI 3.0?

Note: If the file is already "read-only" before dragging or becoming "read-only" after dragging, none of these API will work anyway. So we are safe here. There is no bugs or concerns regarding to that topic.

Thanks,
Jackie

question

Most helpful comment

I think this has to do with DataPackage and DataPackageView classes especially how DataPackage.SetStorageItems method is implemented. The default behavior of SetStorageItems is to make the provided storage items readonly unless the app explicitly provides readonly attribute as false when drag of an item is starting from a given app. MS has to change this default behavior or make changes to set the readonly attribute as false in file explorer code behind.

All 17 comments

@michael-hawker

Regarding whether files that are dragged into the app should be read only or not, I would say that they shouldn't be. The reason for having read only access for files is so that an app can't start making changes without the user giving permission. However actions such as using the file picker, launching an app from a file or dragging a file into the app are all actions initiated by the user and should give write access capabilities.
If the behavior of PathIO is correct, StorageFile should use it as well. If it was an oversight and PathIO is not meant to be able to write to files that were dragged into the app, then this should be changed.

Yeah, this is a royal pain and I can't see any logic to it. I figure it must be a mistake.

Please can we get this fixed.

I think this has to do with DataPackage and DataPackageView classes especially how DataPackage.SetStorageItems method is implemented. The default behavior of SetStorageItems is to make the provided storage items readonly unless the app explicitly provides readonly attribute as false when drag of an item is starting from a given app. MS has to change this default behavior or make changes to set the readonly attribute as false in file explorer code behind.

Yes, this is extremely annoying. My users expect to be able to open files by drag and drop and then edit them. In my case I use a workaround if the file happens to be in the Pictures library because I have the Pictures library capability, so when the files are dropped I check if they are read only, and if so I try to load them from the file path. This is very cumbersome and only works if the files are in the Pictures library.

The read-only attribute isn't adding much security because it's possible to complete the drag-drop operation as a move operation (and not actually move the files anywhere), in which case the original files are deleted even though they were marked as read-only.

PS I imagine that the reason PathIO.WriteBytesAsync works for the file path is because you have access to that file path by some other means, e.g. Pictures library capability.

This definitely seems like a bug in drag-n-drop, the developer should have access to a file if a user drops it into the app. This seems no different than opening the file via activation arguments, which works fine right?

Also, can't the developer still take the StorageFile from the drag-n-drop arguments and add it to the app's Future Access List and still open/access it later for both read & write?

This definitely seems like a bug in drag-n-drop, the developer should have access to a file if a user drops it into the app. This seems no different than opening the file via activation arguments, which works fine right?

Also, can't the developer still take the StorageFile from the drag-n-drop arguments and add it to the app's Future Access List and still open/access it later for both read & write?

You can if you add it to the future access list but it won't take effect for the current run tho.

@JasonStein even if you close the reference from the DataView and just re-open the file directly from the access list?

@JasonStein even if you close the reference from the DataView and just re-open the file directly from the access list?

I have not tried that way but I would assume no, not for the current run. @yaichenbaum I think you tried that, did you?

I tried adding the storage item to the future access list but it didn't help, I didn't try closing the reference though so it could be that would help.

I tried adding the storage item to the future access list but it didn't help, I didn't try closing the reference though so it could be that would help.

Even if that works, I would still prefer to use PathIO since it is more straight forward anyway :)

I tried adding the storage item to the future access list but it didn't help, I didn't try closing the reference though so it could be that would help.

Even if that works, I would still prefer to use PathIO since it is more straight forward anyway :)

That is why I ended up going with PathIO as well.

@chrisglein @Austin-Lamb can you route this bug ? With win32 support perhaps some of these restrictions can be relaxed.

Btw, same applies to StorageFile.RenameAsync API. But there is literally no alternative way to rename a dragged file.

I've resorted to warning the users that they need to enable BroadFileSystemAccess just to accept dragged file properly (in apps where it needs to save over/rename, etc)

frustrating and not a good experience for dev or user.

I've resorted to warning the users that they need to enable BroadFileSystemAccess just to accept dragged file properly (in apps where it needs to save over/rename, etc)

frustrating and not a good experience for dev or user.

Exactly, and it makes no sense to let user give BroadFileSystemAccess to a simple notepad app at all.

@StephenLPeters @jevansaks Would it make sense to move this issue over to the project reunion repository as this is related to the app model?

Was this page helpful?
0 / 5 - 0 ratings