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

fix(shim): fix a minor issue with Get-Subsystem (and refactor) #5684

Merged
merged 7 commits into from
Oct 14, 2023

Conversation

spider2048
Copy link
Contributor

Description

Fixes minor bugs

Motivation and Context

The Get-Subsystem function doesn't catch errors when it fails to read a binary.
The Get-Subsystem and Change-Subsystem functions are refactored to Get-PESubsystem and Change-PESubsystem.
The code will only try to change subsystem to 2 (which indicates GUI)

Relates to #5559

How Has This Been Tested?

Credits to @HUMORCE (original).

Steps to reproduce the bug:

% touch empty.exe
% scoop shim add empty .\empty.exe
Adding local shim empty...
MethodInvocationException: D:\Sources\Scoop\lib\core.ps1:12
Line |
  12 |          $peOffset = $binaryReader.ReadInt32()
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling "ReadInt32" with "0" argument(s): "Unable to read beyond the end of the stream."

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@rashil2000
Copy link
Member

There should be a catch block in the Change-PESubsystem function too, right, just in case?

@r15ch13
Copy link
Member

r15ch13 commented Oct 11, 2023

And use a approved PowerShell verb by changing Change-PESubsystem to Set-PESubsystem

@spider2048
Copy link
Contributor Author

I've refactored Change-PESubsystem to Set-PESubsystem and added an additional catch block as requested.
I think it is good to go.

@rashil2000 rashil2000 merged commit 6cdcc75 into ScoopInstaller:develop Oct 14, 2023
2 checks passed
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.

4 participants