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

HTTP header are not case insensitive #14339

Closed
Findus23 opened this issue Apr 11, 2019 · 2 comments · Fixed by #14449
Closed

HTTP header are not case insensitive #14339

Findus23 opened this issue Apr 11, 2019 · 2 comments · Fixed by #14449
Assignees
@katebutler
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone
3.11.0

Comments

@Findus23
Copy link
Member

I am currently setting up a test-instance for my workshop and I try to be pedantic and not skip over potential bugs when doing so.

Currently I am really confused by the GeoIP downloader (I think I saw the same bug for the initial download on my real instance, but the updater works):

When I try to initially download the free geoip2 database, I just get the generic General_DownloadFail_HttpRequestFail error.

Adding a print_r($expectedFileSizeResult); shows the source of the issue:

matomo/core/Http.php

Lines 737 to 745 in 07adf0a

$expectedFileSize = 0;
if (isset($expectedFileSizeResult['headers']['Content-Length'])) {
$expectedFileSize = (int)$expectedFileSizeResult['headers']['Content-Length'];
}
if ($expectedFileSize == 0) {
Log::info("HEAD request for '%s' failed, got following: %s", $url, print_r($expectedFileSizeResult, true));
throw new Exception(Piwik::translate('General_DownloadFail_HttpRequestFail'));
} 

Array
(
    [status] => 200
    [headers] => Array
        (
            [date] => Thu, 11 Apr 2019 16:29:34 GMT
            [content-type] => application/gzip
            [content-length] => 29444833
            [set-cookie] => __cfduid=d5e7c2e5b339a937a45385e60f1a69d7c1555000174; expires=Fri, 10-Apr-20 16:29:34 GMT; path=/; domain=.geolite.maxmind.com; HttpOnly
            [content-disposition] => attachment; filename=GeoLite2-City_20190409.tar.gz
            [last-modified] => Mon, 08 Apr 2019 12:07:23 GMT
            [cf-cache-status] => HIT
            [expires] => Thu, 11 Apr 2019 20:29:34 GMT
            [cache-control] => public, max-age=14400
            [accept-ranges] => bytes
            [expect-ct] => max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
            [server] => cloudflare
            [cf-ray] => 4c5e57111a0dbef8-FRA
        )

    [data] => 
)

Cloudflare only returns lowercase HTTP headers, but Matomo only checks for 'Content-Length' and therefore the download fails.

Interestingly fixing this, the download still doesn't work as I just get an unhelpful Failed sending HTTP request here:

matomo/core/Http.php

Lines 581 to 586 in 07adf0a

} elseif ($response === false) {
$errstr = curl_error($ch);
if ($errstr != '') {
throw new Exception('curl_exec: ' . $errstr
. '. Hostname requested was: ' . UrlHelper::getHostFromUrl($aUrl));
}

I'm also a bit clueless why I am the first one to come across this issue. Am I missing something?
@Findus23
Copy link
Member Author

Findus23 commented Apr 11, 2019 •
edited

I think I have found the potential answer to why I am the only one to report this bug:

I am using a fairly modern curl version (7.64.0 in the php module) and for a while curl now uses HTTP/2 by default.

When running curl -v --http1.0 https://geolite.maxmind.com/download/geoip/database/GeoLite2-City.tar.gz cloudflare responds with uppercase headers.

That also explains why the download still fails as many parts of Http.php have been written without newer HTTP versions in mind like e.g.

matomo/core/Http.php

Line 317 in 07adf0a

if (!preg_match('~^HTTP/(\d\.\d)\s+(\d+)(\s*.*)?~', $line, $m)) {

@tsteur tsteur added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Apr 11, 2019
@tsteur tsteur added this to the 3.12.0 milestone Apr 11, 2019
@tsteur
Copy link
Member

tsteur commented Apr 11, 2019

Moving it to 3.12 as it might be a somewhat easy fix (but will take a while to get new curl version installed) and could affect eventually quite a few people

@katebutler katebutler self-assigned this May 12, 2019
@katebutler katebutler mentioned this issue May 15, 2019
Merged
@mattab mattab modified the milestones: 3.12.0, 3.11.0 Jun 16, 2019
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@katebutler katebutler

Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
3.11.0
Development

Successfully merging a pull request may close this issue.

Fix download issues via cURL with HTTP/2 matomo-org/matomo
4 participants
@tsteur @mattab @katebutler @Findus23