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

String function warming from isValidHost(null) called after update in Matomo 4.8.0-rc1 on PHP 8.1 #18885

Closed
GreenReaper opened this issue Mar 4, 2022 · 1 comment
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone

Comments

@GreenReaper
Copy link

Expected Behavior

There are no notice boxes on the Matomo dashboard.

Current Behavior

Two errors are reported as orange message boxes on the dashboard just after update:

WARNING: /core/Url.php(234): Deprecated - strlen(): Passing null to parameter #1 ($string) of type string is deprecated - Matomo 4.8.0-rc1 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) (Module: CoreHome, Action: index, In CLI mode: false)
WARNING: /core/Url.php(235): Deprecated - strcspn(): Passing null to parameter #1 ($string) of type string is deprecated - Matomo 4.8.0-rc1 - Please report this message in the Matomo forums: https://forum.matomo.org (please do a search first as it might have been reported already) (Module: CoreHome, Action: index, In CLI mode: false)

Possible Solution

The code in question is within public static function isValidHost($host = false):

         // Only punctuation we allow is '[', ']', ':', '.', '_' and '-'
>        $hostLength = strlen($host);
>        if ($hostLength !== strcspn($host, '`~!@#$%^&*()+={}\\|;"\'<>,?/ ')) {
             return false;
         }

It occurs to me that there is no code prior to this that would be triggered by 'null'. Passing null does not trigger use of the default value. As PHP type comparisons show, they are not the same - false is 'not true', null means it is not initialized.

Perhaps the code could guard against this by adding a test for null to the block with the condition if ($host === false) { earlier on, that tries self::getHostFromServerVariable() instead, and failing that returns true.

Steps to Reproduce (for Bugs)

  1. Update to 4.8.0-rc1 via the automatic upgrade
  2. Visit main page after update and see the warning
  3. The warning does not appear after visiting the admin plugin page and and going back to the dashboard.

I ran service php8.1-fpm restart but I cannot be sure whether I did that before or after seeing the warning.

In case it matters: this server is processing proxy_passed from another server.

On the recieving processing server, the nginx config is:

        location = /piwik/matomo.php {
                # xxx main server proxying
                set_real_ip_from xxx.xxx.xxx.xxx;
                real_ip_header X-Forwarded-For;

                add_header Strict-Transport-Security "max-age=63072000";
                try_files $fastcgi_script_name =404; # protects against CVE-2019-11043. If this line is already included in your snippets/fastcgi-php.conf you can comment it here.
                include /etc/nginx/fastcgi_params;
                fastcgi_pass unix:/var/run/php8.1-fpm-harmony-prod.sock;

                fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
                fastcgi_param HTTP_PROXY ""; # prohibit httpoxy: https://httpoxy.org/

                # Allow requests of up to half an hour
                fastcgi_read_timeout 1800;

                # Buffer response up to 128kb in 4kb chunks
                fastcgi_buffers 32 4k;
        }

On the sending server:

        location ~ /piwik[0-9]*/(matomo|piwik)\.php {

            # Proxy to yyy
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
            proxy_set_header X-Forwarded-Host $http_host;
            proxy_set_header X-Forwarded-Uri /piwik;
            proxy_http_version 1.1;
            proxy_socket_keepalive on;
            proxy_read_timeout 5s;
            #proxy_set_header Host xxx.net
            proxy_pass https://yyy.yy.yyy.net

        }

Your Environment

  • Matomo Version: 4.8.0-rc1
  • PHP Version: PHP 8.1.3 (fpm-fcgi) (built: Feb 23 2022 16:07:16)
  • Server Operating System: Debian bullseye x86_64 on Linux 5.10.84
  • Nginx version: nginx version: nginx/1.21.6
  • Additionally installed plugins: CustomVariables, DevicePixelRatio (v2.0.0)
  • Browser: Firefox Nightly 99.0a1 (2022-03-03) (64-bit)
  • Operating System: Windows 10 Home 21H2 19044.1566
@GreenReaper GreenReaper added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Mar 4, 2022
@sgiehl sgiehl added Bug For errors / faults / flaws / inconsistencies etc. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Mar 4, 2022
@sgiehl sgiehl added this to the 4.8.0 milestone Mar 4, 2022
@sgiehl
Copy link
Member

sgiehl commented Mar 4, 2022

Thanks @GreenReaper for the report. We will try to fix this in the final version of 4.8.0

@sgiehl sgiehl self-assigned this Mar 4, 2022
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

No branches or pull requests

3 participants