(chocolatey-core.extension) regex pattern vs wildcard pattern usage in Get-AppInstallLocation

[michaelray]

(chocolatey-core.extension) The first parameter for the Get-AppInstallLocation function, $AppNamePattern, is documented/commented to be a "Regular expression pattern" and the function itself uses it as a regex pattern when using the -match operator in its body, BUT Get-AppInstallLocation also calls Get-UninstallRegistryKey, which does NOT use a regex pattern but instead a wildcard pattern. Regex patterns and wildcard patterns aren't interchangeable like that, however (ex. a regex pattern "Notepad\+\+.*" versus wildcard pattern "Notepad++*").

Expected Behavior

Since Get-AppInstallLocation takes a regular expression pattern (used with -match operator) as its parameter, it shouldn't use other functions or operators that treat that same parameter value as a wildcard pattern (used with -like operator). Get-UninstallRegistryKey (called by Get-AppInstallLocation) takes a wildcard pattern. Because of this interchangeable use, unexpected matches or lack of matches may occur when looking for the path. Errors occur ("ERROR: Bad argument to operator '-match': parsing "Notepad++*" - Nested quantifier +..") when passing in a wildcard pattern into Get-AppInstallLocation because of the use of -match (ex. "Notepad++*").

When passing a regex pattern into the Get-AppInstallLocation method, if a regex is pattern is provided, the calls -like operation used in Get-UninstallRegistryKey will be unexpectedly unmatched.

Possible Solution

Change Get-AppInstallLocation to not call any functions or operators that use wildcard pattern matching, since the parameter in this function is a regex pattern.

I would imagine this would mean maybe adjusting Get-UninstallRegistryKey to have an alternative parameter to pass in a regex pattern instead of a wildcard pattern, then have Get-AppInstallLocation call Get-UninstallRegistryKey with the new regex pattern parameter instead of the default (current) wildcard pattern. This would allow Get-UninstallRegistryKey to continue to function with existing calls while also supporting an alternative of using regex pattern matching instead of wildcard patterns so that other functions using wildcards could still use the same functions/logic.

Steps to Reproduce (for bugs)

1. choco install notepadplusplus -y

I'm not sure what exactly is triggering the error I receive (see gist below), but when I attempt to install "notepadplusplus" (which installs "notepadplusplus.install"), I get a bad argument error due to a wildcard pattern being used. I discovered this issue in Get-AppInstallLocation in investigating that (chocolatey-community/chocolatey-packages#782 was submitted in regards to adjusting the notepadplusplus.install package as a seperate fix & PR chocolatey-community/chocolatey-packages#783 has also been submitted as a fix localized to the "notepadplusplus.install" package).

Context

This occurred when attempting to install the "notepadplusplus.install" package on a Windows 7 Pro 32 bit machine.

Your Environment

• Package Version used: 1.3.1
• Operating System and version: Windows 7 Pro (Windows v6.1.7601.65536) (32 bit)
• Chocolatey version: v0.10.7
• Install/uninstall gist: https://gist.github.com/michaelray/fa3ad48d81026988810923226d86e7bf

Related issue:
chocolatey-community/chocolatey-packages#782

[RedBaron2]

Attention!
@ferventcoder or @gep13 could have a more thorough idea about what might not be working correctly.

The feature useRememberedArgumentsForUpgrades could be playing a part in this issue, and could be related to choco issue(s) #797 and #1390

I had a similar problem awhile back, but thinking it was due to being on the windows insider program. I am sorry to report that I didn't think it a issue. My solution on my system was to just disable the feature. Since it "... is considered in preview ..." Then I ran the command cinst notepadplusplus --x86 -y
Since this has been seen by someone else. I will speculate that this could be possibly due to notepadplusplus being a meta-package.

[majkinetor]

Simple solution would be to espace reg ex chars in Get-AppUnininstallLocation for Get-UninstallRegistryKey call via RegEx.Escape method.

[majkinetor]

It would be better tho to make all functions work with either globs or regex for consistency.

[michaelray]

@RedBaron2 This doesn't seem to be related to the meta-package or the "useRememberedArgumentsForUpgrades" feature since I wasn't using that feature when the issue appeared (gist shows that option set to 'False').

@majkinetor I don't think just escaping the regex chars alone (in Get-AppInstallLocation when calling Get-UninstallRegistryKey) is an effective solution because the issue still remains with calling functions/operations that are using the pattern as a wildcard when it is actually a regex. If a regex is actually being provided to Get-AppInstallLocation as specified, it will still fail to match expected keys when calling Get-UninstallRegistryKey since the use of -like in Get-UninstallRegistryKey will actually be using it to wildcard match the name of the registry key (for example, if "Software Name.*" is provided in the call to Get-AppInstallLocation (which is a valid regex), the -like wildcard match (in Get-UninstallRegistryKey) will fail to find registry keys that have names like "Software Name (v1.2)" because there is no period after "Name"). Using Regex.Escape would still cause registry key mismatches (and make it match even less than it currently does) because it would add \'s to escape regex, but the wildcard matching would look for those \'s in the actual string value.

Like you said, the fix and ultimate goal would be to make all of the functions work with either globs/wildcard or regex for consistency (probably regex would be better since it is more powerful). Since just changing Get-UninstallRegistryKey to use a regex alone would not be backward compatible (for packages that are using Get-UninstallRegistryKey directly already, that would be a breaking change), it would be more ideal (to move toward the use of regex without breaking existing calls) to maybe add a new, separate argument to Get-UninstallRegistryKey that accepts a regex pattern and just have Get-UninstallRegistryKey support both regex (new) and wildcard (as it currently does) as a shim until the next major version of chocolatey-core.extension is released, at which point the wildcard matching argument for Get-UninstallRegistryKey can be deprecated. In the meantime, Get-AppInstallLocation could then be updated to use the new regex-based parameter when calling Get-UninstallRegistryKey so that it properly matches registry key names when using a regex.

It looks like Get-UninstallRegistryKey is getting keys from certain registry locations, then looping through them applying the -like wildcard check against them, so I would think it would be simple enough for it to support both for now by adding a new optional regex argument, which, when used, will override the current wildcard pattern argument and apply a regex -match instead. Again, once the next major version of chocolatey-core.extension is released, support for wildcard/glob patterns could be deprecated in Get-UninstallRegistryKey.

Doing it that way could seemingly achieve fixing this in a backward compatible way where existing calls using these functions would not break or have behavior changed, while also moving toward using consistent patterns (regex vs wildcard/glob) as extension function parameters.

[ferventcoder]

Chocolatey side - chocolatey/choco#1342

[majkinetor]

After some thinking I believe globs are probably better to use here even tho regex is more powerful .

Maybe I am wrong but my reasoning is:

1. Get-UninstallRegistryKey is used way longer then Get-AppInstallLocation
2. I am even not sure Get-AppInstallLocation is used a lot outside of the core team repo and if it is, its probably always used without regex but with simple search pattern so it would probably not cause any serious problems switching it to globs.
3. Occasions in which Get-AppInstallLocation progresses later then call to Get-UninstallRegistryKey do not for majority.

[ferventcoder]

https://github.com/chocolatey/chocolatey-coreteampackages/issues/784#issuecomment-313296467

@majkinetor this is probably the best solution here. I thought there was a fix that needed to be done Chocolatey side, but just realized it was in Get-AppInstallLocation.

[majkinetor]

OK @ferventcoder , will do that.

[AdmiringWorm]

I know this is an old issue, but something seemed off to me.

As mentioned before, Get-AppInstallLocation expects a regex input value, but the Get-UninstallRegistryKey expects a wildcard pattern.

Now, if the value passed to Get-UninstallRegistyKey was regex escaped, wouldn't this pretty much mean that nothing would match it at all.

More appropriately IMO, would probably be to 'Unescape' the pattern when calling 'Get-UninstallRegistryKey'.
Basically, transforming Notepad\+\+* to Notepad++* (I know, regex would actually need a dot (.) before *, but it seems that isn't true in powershell).
Or changing Get-AppInstallLocation to use the -like operator instead when traversing the directories.

[majkinetor]

Perhaps creating new version of function, Get-AppInstallLocationEx should be done and keep this current one as it is. I am not sure this one can be fixed so current packages are not affected.

There are other potential improvements that could be done here, for instance adding some limitations (#1168) such as limiting architecture, desired binary size/metadata etc.

[majkinetor]

I think we should close this and related PR chocolatey-community/chocolatey-packages#1210 based on my previous comment. It looks like this isn't produce much problems anyway so its tolerable.

Any comments ?

[michaelray]

Ideally, I think it would be better to have expected and consistent behavior in these helper methods/APIs, at least as a long term goal.

I'm not as involved in the Chocolatey community as you, though, and if you believe this is not causing any problems and prefer to close this issue, I'm fine with that.

The original issue came up when installing the Notepad++ package, but that issue has been resolved on that end via PR chocolatey-community/chocolatey-packages#783 . This isn't actively causing me any issues now, but I just wanted to make sure this was reported.

[AdmiringWorm]

This issue has been re-opened and moved to the repository where future development of chocolatey-core.extension will be happening.

[midrare]

Would it be acceptable to try both? i.e. Every string is compiled both as a glob and as regex. If it's invalid as a glob or regex it's simply ignored and the one that compiled correctly is used. If its valid as either one then both are used.

[AdmiringWorm]

My opinion is that we should regex escape whatever is passed to the Get-UninstallRegistryKey in that cmdlet.

However, I do not have an opinion on whether we should have a fallback or not, at least not until I can see what the possible implementation would be.