prismicio-community / php-kit

Community maintained development kit for Prismic and the PHP language
https://prismic.io
Other
108 stars 82 forks source link

PHP automatically decodes Cookies values, no need to use urlencoded char #96

Closed mteneur closed 9 years ago

tremby commented 9 years ago

I just noticed the same thing and opened a PR. (My one didn't fix the tests, though.)

Might I suggest using explode rather than preg_split, since it's not actually using any regex features? I'd also suggest using a literal space rather than the octal representation, since " " is a lot easier to understand at a glance than "\040".

tremby commented 9 years ago

@erwan and co: I'm surprised this one has sat untouched since February. Today is the first time I've tried to use experiments and I found this bug, and it's a blocker.

The relevant part of my code, with a workaround, is the following, and it's just the patched code from the kit written out longhand:

        // If a experiment token is in a cookie, use that
        // Most of the following is a temporary workaround before
        // https://github.com/prismicio/php-kit/pull/96 is fixed. Once that is
        // fixed the commented-out line can be used instead of everything which
        // precedes it.
        $ref = null;
        if ($cookie = self::getExperimentRefCookie()) {
            $parts = explode(' ', trim($cookie));
            if (count($parts) >= 2) {
                $experiments = self::getApi()->getExperiments();
                $running = $experiments->getRunning();
                $experiment = null;
                foreach ($running as $exp) {
                    if ($exp->getGoogleId() == $parts[0]) {
                        $experiment = $exp;
                        break;
                    }
                }
                if ($experiment != null) {
                    $variations = $experiment->getVariations();
                    $varIndex = (int) $parts[1];
                    if ($varIndex >= -1 && $varIndex < count($variations)) {
                        $ref = $variations[$varIndex]->getRef();
                    }
                }
            }
        }
        if ($ref) {
        /* if ($ref = self::getApi()->getExperiments()->refFromCookie(self::getExperimentRefCookie())) { */
            return $ref;
        }
erwan commented 9 years ago

@tremby sorry for the delay.

Would this pull request fix your problem?

tremby commented 9 years ago

Yes, I believe it would, though I would make the two small changes I mentioned in my first comment in this PR. One is a miniscule performance boost, the other addresses readability of the code.

erwan commented 9 years ago

OK, I'm merging this and I will apply @tremby 's suggestions manually.