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

Explicitly enable access to web cron script #266

Merged
merged 3 commits into from Apr 28, 2014

Conversation

piotr-cz
Copy link
Contributor

When somehow parent folder receives .htaccess file with Deny from all rules.
See issue [http://forum.piwik.org/read.php?2,109382]

@robocoder
Copy link
Contributor

It shouldn't be publicly accessible by default as it could lead to potential DoS attack.

@piotr-cz
Copy link
Contributor Author

@robocoder
Copy link
Contributor

Ok, I see CronArchive checks token_auth. But the code says archive.* scripts are deprecated or not recommended. Maybe this use case should no longer be documented?

@piotr-cz
Copy link
Contributor Author

I don't know, I'm just an user. So there is another front controller for web cron?

By the way, one travis job failed, but it doesn't seem to be related to this issue.

@mattab
Copy link
Member

mattab commented Apr 28, 2014

the archive.php is still used for webcron (the warning is not shown in the webcron).
is there a reason for the updatetoken.php added there as well?

@piotr-cz
Copy link
Contributor Author

@mattab Only reason is that the updatetoken.php code it looks to me like another front controller. I can exclude it from the directive if I'm wrong.

@mattab
Copy link
Member

mattab commented Apr 28, 2014

yes please exclude it as it is not needed to access this file from HTTP

@piotr-cz
Copy link
Contributor Author

@mattab Done. Not sure what's happening with tests though.

mattab pushed a commit that referenced this pull request Apr 28, 2014
Explicitly enable access to web cron script
@mattab mattab merged commit 7dbe126 into matomo-org:master Apr 28, 2014
@mattab
Copy link
Member

mattab commented Apr 28, 2014

The failures are un-related to your change, I tried to fix one here: 5e3a47a
and contacted Travis CI about the other! cheers

@piotr-cz
Copy link
Contributor Author

Yeah, as for [https://travis-ci.org/piwik/piwik/jobs/23912449] it seems that test failed one because of 2 microseconds difference in assertion, the other one might be travis issue

@piotr-cz piotr-cz deleted the patch-1 branch April 29, 2014 08:07
@piotr-cz
Copy link
Contributor Author

piotr-cz commented May 7, 2014

Not sure if it helps, but recently Travis switched PHPUnit installation from PEAR to PHAR: travis-ci/travis-ci#2223

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants