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

Jump to bottom

Error when calling isSupportedWithReason from CliMulti on Windows #18605

Closed
brainfoolong opened this issue Jan 11, 2022 · 1 comment
Closed

Error when calling isSupportedWithReason from CliMulti on Windows #18605

brainfoolong opened this issue Jan 11, 2022 · 1 comment
Assignees
@sgiehl
Labels
Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone
4.7.0

Comments

@brainfoolong
Copy link
Contributor

Another one :)

I've seen really strange error messages since a while in our Apache log which have no timestamp or what so ever.

  The system cannot find the path specified.
  The system cannot find the path specified.
  The system cannot find the path specified.
  The system cannot find the path specified.
  'groups' is not recognized as an internal or external command,
  operable program or batch file.

After checking every application i finally found the problem in Matomo (Current version).

The call stack from

matomo/core/CliMulti/Process.php

Line 206 in ec3e220

public static function isSupportedWithReason()

with the stack

matomo/core/CliMulti/Process.php

Line 243 in ec3e220

if (!self::awkExistsAndRunsCorrectly()) {

matomo/core/CliMulti/Process.php

Line 255 in ec3e220

private static function awkExistsAndRunsCorrectly()

and the stack

matomo/core/CliMulti/Process.php

Line 234 in ec3e220

if (!self::psExistsAndRunsCorrectly()) {

matomo/core/CliMulti/Process.php

Line 250 in ec3e220

private static function psExistsAndRunsCorrectly()

both result in commands executed on windows ps x and awk... that not exist on windows.

There are even more commands, that are all suppressed with @shell_exec that do not throw a Matomo error, but still log errors on the windows apache, because commands are tried to execute, which apache then logs as errors without useful information.

A general check for commands should be implemented, to check for windows or not. Some shell_exec are already pre-cechekd with isWindows checks, but not all.

More examples:

matomo/core/CliMulti/Process.php

Line 263 in ec3e220

$uname = @shell_exec('uname -a 2> /dev/null');

Maybe a fix for the whole canse of isSupportedWithReason could be, when on windows, just skip all other checks. As the isWindows check already return that is not possible on windows so further checks are useless, aren't they?
@brainfoolong brainfoolong added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Jan 11, 2022
@tsteur tsteur added this to the 4.7.0 milestone Jan 11, 2022
@tsteur tsteur added Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Jan 11, 2022
@tsteur
Copy link
Member

tsteur commented Jan 11, 2022

Makes sense 👍 if isWindows() === true we want to return immediately and not perform any of the other checks. Thanks for reporting this @brainfoolong

As in

diff --git a/core/CliMulti/Process.php b/core/CliMulti/Process.php
index e3a6bd968d..6016f72918 100644
--- a/core/CliMulti/Process.php
+++ b/core/CliMulti/Process.php
@@ -215,6 +215,7 @@ class Process
 
         if (SettingsServer::isWindows()) {
             $reasons[] = 'not supported on windows';
+            return $reasons;
         }
 
         if (self::isMethodDisabled('shell_exec')) {

@sgiehl sgiehl self-assigned this Jan 17, 2022
@sgiehl sgiehl mentioned this issue Jan 17, 2022
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sgiehl sgiehl

Labels
Bug For errors / faults / flaws / inconsistencies etc. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Milestone
4.7.0
Development

No branches or pull requests
4 participants
@tsteur @justinvelluppillai @sgiehl @brainfoolong