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

Fixes downloading files in chunks when not using curl #16582

Merged
merged 2 commits into from Oct 21, 2020
Merged

Fixes downloading files in chunks when not using curl #16582

merged 2 commits into from Oct 21, 2020

Conversation

stevenseeley
Copy link
Contributor

@stevenseeley stevenseeley commented Oct 16, 2020

Hi Guys,

I was having some issues when downloading files as the response headers were not set when php-curl isn’t installed.

Instead of opening an issue, I decided to create a simple patch that fixes the file download bug and set the response headers correctly.

Kind regards,

Steve

@Findus23 Findus23 added the Needs Review PRs that need a code review label Oct 17, 2020
core/Http.php Outdated Show resolved Hide resolved
update the http headers for the whole `fopen` method branch
@sgiehl
Copy link
Member

sgiehl commented Oct 20, 2020

@stevenseeley Would you mind explaining your issue a bit more? Which file did you try downloading?
I would like to reproduce that locally in order to check if the changes are solving it

@stevenseeley
Copy link
Contributor Author

stevenseeley commented Oct 20, 2020

@stevenseeley Would you mind explaining your issue a bit more? Which file did you try downloading?
I would like to reproduce that locally in order to check if the changes are solving it

Hi @sgiehl, yes when using the Http::downloadChunk static method which is used by several of your API's (such as downloadFreeGeoIPDB) there is a check for content-length and if it's 0 it will thrown an exception see here

It's only set when using php-curl, for example when falling back to fopen this header (and all the others) is not set. I hope that makes sense!

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @stevenseeley. Was now able to reproduce the problem locally and can confirm your changes solves the issue.

@sgiehl sgiehl added this to the 4.0.0-RC milestone Oct 21, 2020
@sgiehl sgiehl changed the title Update Http.php Fixes downloading files in chunks when not using curl Oct 21, 2020
@sgiehl sgiehl merged commit 4a419be into matomo-org:4.x-dev Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants