GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Enable recursion into OneDrive by SteveL-MSFT · Pull Request #9509 · PowerShell/PowerShell

Conversation

@SteveL-MSFT

@SteveL-MSFT Steve Lee (SteveL-MSFT) commented May 1, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

PR Summary

The current logic of Get-ChildItem -Recurse for the FileSystemProvider is that if it's a symlink, we don't recurse into it unless -FollowSymLink is specified. However, OneDrive reparse points aren't symlinks (in that they don't link to the local filesystem). There is another flag (on Windows) to indicate if the reparse point is a named surrogate which means it IS a symlink. So the fix is to check for this flag and if it's not a named surrogate (aka symlink), it is ok to recurse into it.

Manually verified with OneDrive.

PR Context

Fix #9461

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • Documentation needed
      • Issue filed:
  • Testing - New and feature
    • N/A or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
cprivitere reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@SteveL-MSFT
Enable recursion into OneDrive
d014114
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested review from Bruce Payette (BrucePay) and Andrew (anmenaga) as code owners May 1, 2019 18:33
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested a review from Aditya Patwardhan (adityapatwardhan) May 1, 2019 18:34
@SteveL-MSFT Steve Lee (SteveL-MSFT) assigned Aditya Patwardhan (adityapatwardhan) May 1, 2019
iSazonov
Ilya (iSazonov) reviewed May 2, 2019
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label May 2, 2019
@iSazonov Ilya (iSazonov) added this to the 7.0.0-preview.1 milestone May 2, 2019
@SteveL-MSFT
address Ilya's feedback
1df6183
iSazonov
Ilya (iSazonov) reviewed May 2, 2019
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT
removed unused method
5032769
iSazonov
Ilya (iSazonov) approved these changes May 3, 2019
View reviewed changes
@sba923

Stéphane BARIZIEN (sba923) commented May 3, 2019

Copy link
Copy Markdown
Contributor

Impressive fix... relying on very undocumented stuff if you ask me... unless "named surrogates" and "data.dwReserved0 & 0x20000000" is documented somewhere. Steve Lee (@SteveL-MSFT)?

  • my understanding is that there won't be a 6.3 before 7.0, correct?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented May 3, 2019

Copy link
Copy Markdown
Member Author

Stéphane BARIZIEN (@sba923) correct, next version is 7.0-preview.1. As noted in the comments in the change, the use of dwReserved0 & 0x20000000 is documented via the macro published in the Windows SDK header. I agree that it would have been better as an API (or even better just another FileAttribute), but that's what we have to work with right now.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sba923

Stéphane BARIZIEN (sba923) commented May 3, 2019

Copy link
Copy Markdown
Contributor

Steve Lee (@SteveL-MSFT) Thanks for the confirmation about 7.0. I had missed the comment with the reference to the IsReparseTagNameSurrogate macro, sorry for that. Reparse point tags are documented here, this documents the N (Name surrogate) bit with value 0x20000000, and links to another documentation for the macro.

The documentation for WIN32_FIND_DATAA documents that symlinks use a reparse point tag of IO_REPARSE_TAG_SYMLINK (0xA000000C) that has the N bit set. To satisfy my curiosity, I will check what reparse point tag (with the N bit not set) is used by OneDrive (and request that it be added to the documentation).
SteveL-MSFT and ChuckOp reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov
Ilya (iSazonov) reviewed May 4, 2019
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
{
#if !UNIX
var data = new WIN32_FIND_DATA();
using (SafeFileHandle handle = FindFirstFileEx(filePath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0))

@iSazonov Ilya (iSazonov) May 4, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could use method with fewer parameters
https://github.com/dotnet/corefx/blob/87089ef4559605f8a34ca8959a0ef9c71aa1e02e/src/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs#L241

I do very wonder that CoreFX uses IsNameSurrogateReparsePoint() method only for recurse directory remove but not for recurse directory read.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) May 4, 2019

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CoreFx uses FindFirstFileEx() as well, they just name their wrapper FindFirstFile()

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@SteveL-MSFT Steve Lee (SteveL-MSFT) May 4, 2019

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the second point, I agree it might be an oversight that they don't have symmetric behavior.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
adityapatwardhan
Aditya Patwardhan (adityapatwardhan) approved these changes May 7, 2019
View reviewed changes
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit 1d28144 into PowerShell:master May 7, 2019
@sba923

Stéphane BARIZIEN (sba923) commented May 12, 2019

Copy link
Copy Markdown
Contributor

I've collected my findings about the reparse tags used by OneDrive in this spreadsheet.

https://docs.microsoft.com/en-us/windows/desktop/w8cookbook/placeholder-files is definitely outdated, as Windows 10 OneDrive doesn't use IO_REPARSE_TAG_FILE_PLACEHOLDER anymore. Instead, it uses a number of tags from IO_REPARSE_TAG_CLOUD_1 to IO_REPARSE_TAG_CLOUD_F (on my system I found only 3 values being used), with a IO_REPARSE_TAG_CLOUD_MASK that apparently can be used to compute (tag & IO_REPARSE_TAG_CLOUD_MASK) >> 12 and get some value between 1 and 15 that the OneDrive sync client probably uses as information about the item...

I'm not sure where to post the request to document all this. Steve Lee (@SteveL-MSFT): can you help?
wangfu91 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sba923

Stéphane BARIZIEN (sba923) commented May 12, 2019

Copy link
Copy Markdown
Contributor

For now I've opened MicrosoftDocs/feedback#1484 and MicrosoftDocs/feedback#1483
iSazonov, SteveL-MSFT, and ChuckOp reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT
Steve Lee (SteveL-MSFT) deleted the ondrive-recurse branch May 12, 2019 20:04
@SteveL-MSFT Steve Lee (SteveL-MSFT) mentioned this pull request Jun 13, 2019
Fix auto-downloading of files in OneDrive when doing dir #9895
Merged
7 of 12 tasks
KalleOlaviNiemitalo
Kalle Olavi Niemitalo (KalleOlaviNiemitalo) reviewed Aug 21, 2019
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs
IntPtr hTemplateFile);

[DllImport(PinvokeDllNames.FindFirstFileDllName, EntryPoint = "FindFirstFileExW", SetLastError = true, CharSet = CharSet.Unicode)]
private static extern SafeFileHandle FindFirstFileEx(string lpFileName, FINDEX_INFO_LEVELS fInfoLevelId, ref WIN32_FIND_DATA lpFindFileData, FINDEX_SEARCH_OPS fSearchOp, IntPtr lpSearchFilter, int dwAdditionalFlags);

@KalleOlaviNiemitalo Kalle Olavi Niemitalo (KalleOlaviNiemitalo) Aug 21, 2019

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns a SafeFileHandle, which eventually calls CloseHandle.
According to the FindFirstFileExW documentation, the handle should be closed with FindClose, not CloseHandle.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 21, 2019

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Will submit a new PR to address.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 21, 2019

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#10405

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT Steve Lee (SteveL-MSFT) mentioned this pull request Aug 21, 2019
Minor fix for recursion into OneDrive - Change FindFirstFileEx() to use SafeFindHandle type #10405
Merged
8 of 14 tasks
@iSazonov Ilya (iSazonov) mentioned this pull request Aug 31, 2021
Add internal AppExecLink support to link APIs dotnet/runtime#58233
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@SteveL-MSFT @adityapatwardhan
Enable recursion into OneDrive (PowerShell#9509)
2c2912c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
@BrucePay Bruce Payette (BrucePay) Awaiting requested review from BrucePay
+1 more reviewer
@KalleOlaviNiemitalo Kalle Olavi Niemitalo (KalleOlaviNiemitalo) KalleOlaviNiemitalo left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.0.0-preview.1

Development

Successfully merging this pull request may close these issues.

Get-ChildItem -Recurse doesn't work on the OneDrive sync folder

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@SteveL-MSFT @sba923 @adityapatwardhan @iSazonov @KalleOlaviNiemitalo
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.