Added support for sending and reading Tracker Cookies #28
Conversation
| @@ -1583,6 +1591,8 @@ protected function sendRequest($url, $method = 'GET', $data = null, $force = fal | |||
| if (!empty($response)) { | |||
| list($header, $content) = explode("\r\n\r\n", $response, $limitCount = 2); | |||
| } | |||
|
|
|||
| $this->parseIncomingCookies(explode("\r\n", $header)); | |||
There was a problem hiding this comment.
When I tested the PR I had a case where $header was not defined because I received an empty response. I would suggest to check whether $header is set by wrapping the call in if (!empty($header)) { ... }
There was a problem hiding this comment.
I just realize we will then also need an else { $this->incomingTrackerCookies = array(); }
There was a problem hiding this comment.
@MichaelHeerklotz I think here is still a
if (!empty($header)) {
$this->parseIncomingCookies(explode("\r\n", $header));
} else {
$this->incomingTrackerCookies = array();
}
missing but we can patch this also after merging.
There was a problem hiding this comment.
I have added a $header = ''; , so it should be fine, or am I missing something?
There was a problem hiding this comment.
Sorry I did not see that. Stupid me, that's a much better solution! Cheers :)
PiwikTracker.php
Outdated
| if (strpos(strtolower($header), 'set-cookie:') !== 0) { | ||
| continue; | ||
| } | ||
| $cookies = trim(substr($header, 11)); |
There was a problem hiding this comment.
It's a small thing but I would recommend to use eg strlen('set-cookie:') instead of hardcoded 11 as it is a bit better to read/understand (although it is kind of obvious in this case and not so much needed).
PiwikTracker.php
Outdated
| /** | ||
| * Reads incoming tracking server cookies. | ||
| * | ||
| * @param $cookies Array with HTTP response headers as values |
There was a problem hiding this comment.
I think $cookies is supposed to be $headers
|
Looks good to me. Left a few minor comment especially about |
|
Thank you for your excellent code review. I have updated the PR accordingly. |
These functions are needed by some new tests I wrote for the QueuedTracking plugin. With them it is possible to send and read the third party cookie set by Piwik.