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

urldecode errors? #949

Closed
anonymous-matomo-user opened this issue Aug 25, 2009 · 12 comments
Closed

urldecode errors? #949

anonymous-matomo-user opened this issue Aug 25, 2009 · 12 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Milestone

Comments

@anonymous-matomo-user
Copy link

http://forum.piwik.org/index.php?showtopic=1438

@anonymous-matomo-user
Copy link
Author

Attachment: replace html_entity_decode with urldecode in core/Tracker/Visit.php
urldecode.patch

@anonymous-matomo-user
Copy link
Author

Attachment: phpinfo on affected server
phpinfo.txt

@anonymous-matomo-user
Copy link
Author

Attachment: some lines from the log_visit table. The last three lines are after applying the patch
log_visit_excerpt.txt

@robocoder
Copy link
Contributor

In [1439], fix #949 - replace html_entity_decode($url) with urldecode(Piwik_Common::unsanitizeInputValue($url))

@robocoder
Copy link
Contributor

According to php.net/urldecode:

The superglobals $_GET and $_REQUEST are already decoded. Using urldecode() on an element in $_GET or $_REQUEST could have unexpected and dangerous results.

That would explain why urldecode() wasn't previously required and why this problem hasn't been reported before.

@robocoder
Copy link
Contributor

In [1441], partially revert #949 pending further investigation

@robocoder
Copy link
Contributor

Unable to reproduce with:

PHP Version => 5.2.6-3ubuntu4.2

System => Linux gemini 2.6.28-15-generic #49-Ubuntu SMP Tue Aug 18 18:40:08 UTC 2009 i686
...

and magic_quotes_gpc on. Other than a few additional modules/extensions (APC, xdebug, sqlite, memcache), the other difference is 32-bit vs 64-bit.

@anonymous-matomo-user
Copy link
Author

I've set magic_quotes_gpc to Off, but even then I need my patch. Is it expected that the referer URLs are stored urlencoded in the database?
Like:

http%3A%2F%2Fdomain.com%2F

@robocoder
Copy link
Contributor

No, URLs are not stored urlencoded in my database.

As the PHP documentation states, superglobals like $_GET should already be decoded. This isn't happening in your setup for some reason. So, your patch only addresses a symptom.

Can you create a test script like:

<?php
var_dump($_GET);

and call the script with an encoded URL. Test it locally and remotely to see if there's a difference.

test.php?url=http%3a%2f%2fexample.com

@anonymous-matomo-user
Copy link
Author

Result of var_dump running on my server in the same directory as Piwik:

array(1) { ["url"]=>  string(18) "http://example.com" }

I have the bad feeling that my integration is faulty. I'll report more, when I find something.

@anonymous-matomo-user
Copy link
Author

I found the problem. In short: My fault, Piwik was correct.

Long version:
The server on which Piwik is running redirects all http requests to the https page. Since the website I'm tracking is not running under https the auto detection code of Piwik tries to establish a http connection, gets redirected and only then accesses the https protected piwik instance.
It the second request the url parameters are urlencoded again, so that piwik sees false data. I don't know why this happens.

I'm now running fine without my patch, after enforcing https in the tracked website.

@robocoder
Copy link
Contributor

Thanks fo clearing that up.

@anonymous-matomo-user anonymous-matomo-user added this to the Piwik 0.4.4 milestone Jul 8, 2014
@mattab mattab added the wontfix label Aug 3, 2014
This issue was closed.
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. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

No branches or pull requests

3 participants