Skip to content

Various fixes for application compatibility#132

Merged
dhoehna merged 9 commits intomicrosoft:developfrom
TimMangan:master
Mar 11, 2020
Merged

Various fixes for application compatibility#132
dhoehna merged 9 commits intomicrosoft:developfrom
TimMangan:master

Conversation

@TimMangan
Copy link
Copy Markdown
Contributor

@TimMangan TimMangan commented Mar 10, 2020

This PR has three major areas of changes. Most are addressing issues found in specific applications that were recently tested.
A) Changes to PsfLauncher:

  • Directory Iteration bug. There was a bug in the launcher affecting certain apps that resulted in the launcher crashing.

  • Arguments. Support for target command arguments involving filepaths that are part of the package. We introduce a new pseudo-variable %MsixPackageRoot% that you can use in the config.json in the arguments field. The launcher will resolve this and start the command with the mounted path of the package (which could be different on different systems).

  • Shell Launches. Last year I added support for shortcuts to files that were not exe files by using a Shell launch. Unfortunately these shell launches run outside of the container, which is OK in some cases but not in others. The Shell launches will now attempt a technique recommended by Microsoft to launch these inside the container. These changes are now in place, but ultimately the MSIX Runtime still launches it externally. The changes do no harm so they are going in hoping that we can get a future change to the runtime to accommodate. Microsoft has not committed to anything on this yet, but see the scripting issue below.

  • Scripting Changes. NOTE: THIS CHANGE IS NOT INCLUDED IN THIS PR. IT WILL BE IN A FUTURE PR:

There were issues with the script launching if you did not provide a reference to the file relative to the package, such as a script file on a network share. This is now resolved, and you may reference a script file either inside or outside the package. The reference to the script file inside the package may be a relative path to the file (from the root folder of the package) as before, or it may also use the %MsixPackageRoot% pseudo variable.
Also, file references to the script file that included spaces were also not resolvable in the original code. These are now detected and appropriately escaped using the PowerShell back-tic.
Be aware that, similar to the issue for Shell launches, the scripting implementation jumps out of the container even if you ask it not to. The launcher uses PowerShell to run your script (technically to run a PSF provided script called StartingScriptWrapper.ps1 which must be added to the package and that script calls your script). As PowerShell.exe is not part of the package, even through the code tries to launch it inside the container if you ask it to, it currently always runs external to the container. We will need Microsoft to resolve this (probably in the OS itself) , but we don’t have any commitments on it at this time.

B) Changes to FileRedirectionFixup:

  • FindFirstFile and friends. Previously these intercepts supported a two layer strategy of finding files in the redirection area layered upon the path as requested by the app. Now it supports a third layer to cover files in the package VFS paths not found by the MSIX Runtime. Additionally, a few edge cases (network shares and the root of the C: drive) needed fixes.
  • GetFileAttributes and friends. Edge case fixes were needed to resolve several issues involving VFS pathing inside the package seen in applications needing the FRF.
  • CopyFile/MoveFile and Directories and Friends. Resolved more edge case issues involving VFS pathing inside the package.
  • Crashes. There were instances when adding tracing led to crashes due to the tracing code. Generally these were cases where the application was making a call with incorrect parameters. All intercepts are now protected, and if an exception occurs the issueEvent GUID. A fix for PsfTracing to use the correct GUID for PsfMonitor was made. This was broken when Microsoft introduced the customer improvement tracing to the PSF.
  • API Call ID. A general change to improve the tracing by including a API call instance identifier was added. I found this was needed when tracing multi-threaded applications where similar calls were overlapping with each other and using the debug output method of viewing the results.
  • Crashes. There were instances when adding tracing led to crashes due to the tracing code. Generally these were cases where the application was making a call with incorrect parameters. All intercepts are now protected, and if an exception occurs the issue will be logged and the original requested API call attempted so that the app receives the same result as before. will be logged and the original requested API call attempted so that the app receives the same result as before.

C) Changes to Tracing/Logging:

  • Event GUID. A fix for PsfTracing to use the correct GUID for PsfMonitor was made. This was broken when Microsoft introduced the customer improvement tracing to the PSF.
  • API Call ID. A general change to improve the tracing by including a API call instance identifier was added. I found this was needed when tracing multi-threaded applications where similar calls were overlapping with each other and using the debug output method of viewing the results.
  • Crashes. There were instances when adding tracing led to crashes due to the tracing code. Generally these were cases where the application was making a call with incorrect parameters. All intercepts are now protected, and if an exception occurs the issue will be logged and the original requested API call attempted so that the app receives the same result as before.

sout << name << "=";
sout << "0x" << std::uppercase << std::setfill('0') << std::setw(8) << std::hex << value;
return sout.str();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: new-line

sout << "0x" << std::uppercase << std::setfill('0') << std::setw(8) << std::hex << value;
return sout.str();
}
inline std::string InterpretAsHex(const char* name, INT value)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: new-line

@dhoehna
Copy link
Copy Markdown
Contributor

dhoehna commented Mar 10, 2020

Yo. About scripts. It came to my attention that the scripts run outside of the container when PSF is built using release mode. I modified the code to make the attribute list closer to CreateProcess. Now Powershell should always launch inside the container.

You mention changes to help with scripts but I don't see it in your changes. Are there any code changes related to scripts?

@TimMangan
Copy link
Copy Markdown
Contributor Author

Yo. About scripts. It came to my attention that the scripts run outside of the container when PSF is built using release mode. I modified the code to make the attribute list closer to CreateProcess. Now Powershell should always launch inside the container.
You mention changes to help with scripts but I don't see it in your changes. Are there any code changes related to scripts?

I did not attempt to solve the out of container issue. Unfortunately I included the script item in the PR description, but this is not included until the next PR. That PR will add the pseudo-environment variables to allow the script to reference files inside of the package under a %MsixPackageRoot% pseudo-variable.

@dhoehna
Copy link
Copy Markdown
Contributor

dhoehna commented Mar 10, 2020

I see. So, this PR has everything in it?

@TimMangan
Copy link
Copy Markdown
Contributor Author

I see. So, this PR has everything in it?

Everything in the PR description above except for the note about scripting is in this PR. THe scripting change, plus another 4 or 5 edge case fixes for certain API calls, will be in a follow-on PR once this one clears.

@dhoehna dhoehna merged commit f7445d4 into microsoft:develop Mar 11, 2020
dhoehna added a commit that referenced this pull request Apr 10, 2020
* Moving breakout code to closer to CreateProcess call (#129)

* Changing sing location for Psfmonitor

* Adding output for PsfMonitor to PsfMonitor directory

* Moving PsfMonitor stuff to it's own folder

* Adding new files ot nuspec

* Changing sing location for Psfmonitor

* Adding output for PsfMonitor to PsfMonitor directory

* Moving PsfMonitor stuff to it's own folder

* Fixing issues

* Using correct WIL macro

* Moving code so atrribute list is in scopr to CreateProcess

Co-authored-by: Darren Hoehna <dahoehna@microsoft.com>

* Full PrivateProfile support (#124)

* Complete PrivateProfile intercepts for FRF

* Full PrivateProfile support

* Full PrivateProfile support

* Added PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* Various fixes for application compatibility (#132)

* Complete PrivateProfile intercepts for FRF

* Full PrivateProfile support

* Full PrivateProfile support

* Added PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* Replacement Post350 (#137)

* Complete PrivateProfile intercepts for FRF

* Full PrivateProfile support

* Full PrivateProfile support

* Added PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* Changes for script flexibility: Can be in any folder inside or outside package, may use quotation marks, may use new pseudovariables to reference the package root or user redirection area.

* Updates to documentation on scripting.

* Redo of Post350 Changes

* Update for naming conventions

* Added optional executionpolicy options to script configs.

* Correction to scripting Bypass

* Scripting documentation for ExecutionPolicy

* Fix issue with sending files to StartingScriptWrapper.

* Put wait back in for Shell Launch

* Requested cleanups to PR137

* Document UrlDecode method.

* Cleanup StripFileColonSlash

* wrong merge fix for calling StartProcess

Co-authored-by: Darren Hoehna <dhoehna@yahoo.com>
Co-authored-by: Darren Hoehna <dahoehna@microsoft.com>

* Target and Script pseudo-variables and cleaup of PR137 manual merge issues. (#138)

* Making sure breakout does not occur with PS scripts

Co-authored-by: Darren Hoehna <dahoehna@microsoft.com>
Co-authored-by: Tim Mangan (MVP) <TIm@tmurgent.onmicrosoft.com>
dhoehna added a commit that referenced this pull request Jun 16, 2020
* Moving breakout code to closer to CreateProcess call (#129)

* Changing sing location for Psfmonitor

* Adding output for PsfMonitor to PsfMonitor directory

* Moving PsfMonitor stuff to it's own folder

* Adding new files ot nuspec

* Changing sing location for Psfmonitor

* Adding output for PsfMonitor to PsfMonitor directory

* Moving PsfMonitor stuff to it's own folder

* Fixing issues

* Using correct WIL macro

* Moving code so atrribute list is in scopr to CreateProcess

Co-authored-by: Darren Hoehna <dahoehna@microsoft.com>

* Full PrivateProfile support (#124)

* Complete PrivateProfile intercepts for FRF

* Full PrivateProfile support

* Full PrivateProfile support

* Added PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* Various fixes for application compatibility (#132)

* Complete PrivateProfile intercepts for FRF

* Full PrivateProfile support

* Full PrivateProfile support

* Added PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* Replacement Post350 (#137)

* Complete PrivateProfile intercepts for FRF

* Full PrivateProfile support

* Full PrivateProfile support

* Added PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* PrivateProfile Tests

* Changes for script flexibility: Can be in any folder inside or outside package, may use quotation marks, may use new pseudovariables to reference the package root or user redirection area.

* Updates to documentation on scripting.

* Redo of Post350 Changes

* Update for naming conventions

* Added optional executionpolicy options to script configs.

* Correction to scripting Bypass

* Scripting documentation for ExecutionPolicy

* Fix issue with sending files to StartingScriptWrapper.

* Put wait back in for Shell Launch

* Requested cleanups to PR137

* Document UrlDecode method.

* Cleanup StripFileColonSlash

* wrong merge fix for calling StartProcess

Co-authored-by: Darren Hoehna <dhoehna@yahoo.com>
Co-authored-by: Darren Hoehna <dahoehna@microsoft.com>

* Target and Script pseudo-variables and cleaup of PR137 manual merge issues. (#138)

* Making sure breakout does not occur with PS scripts

* Trm 20200529 (#145)

* Fix bug when workingDirectory=""

* Add missing files to master version that are present in the per-build version.

* Added additional output to aid in debugging tests.

* Add new fixup RegLegacyFixups

* Added Test rigging for RegLegacyTest

* Protect against calls with null strings, especially for the filename.

* Add fix for local device path format ("\\.\") in filepath for CreateFile calls

* constexpr wchar_t root_local_device_prefix_dot[] = LR"(\\.\)";

* Fix for Root Virtual Device paths

* Fix for dos local paths

* Add support to modify access to "Maximum_Allowed"

* braces on if

* remove logging files from the source.

* removed commented out code.

* consolidate if _Debugs

* Make debug builds happy again.

* Fix documentation json example

* Brackets 2

* Update config.json

Co-authored-by: Darren Hoehna <dahoehna@microsoft.com>
Co-authored-by: Tim Mangan (MVP) <TIm@tmurgent.onmicrosoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants