Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Added missing constructor parameter #3287

Merged
merged 3 commits into from Jan 3, 2013

Conversation

iwalz
Copy link
Contributor

@iwalz iwalz commented Dec 24, 2012

Is someone working on the cookie stuff? If I understand it correct, Zend\Http\Cookie and Zend\Http\Client\Cookie should handle Header\Cookie and Header\SetCookie - but it looks like there are a few copy & paste issues here.

E.g. this will always fail:

if ($cookie instanceof Cookie) {
        $domain = $cookie->getDomain();
        $path = $cookie->getPath();
        if (!isset($this->cookies[$domain])) {
            $this->cookies[$domain] = array();
        }
        if (!isset($this->cookies[$domain][$path])) {
            $this->cookies[$domain][$path] = array();
        }
        $this->cookies[$domain][$path][$cookie->getName()] = $cookie;
        $this->rawCookies[] = $cookie;
} else {
        throw new Exception\InvalidArgumentException('Supplient argument is not a valid cookie string or object');
}

Cause Cookie does not have the method getDomain (same with getPath) - but SetCookie has that method. From the internal architecture I guess Cookie needs this method.

@iwalz
Copy link
Contributor Author

iwalz commented Dec 24, 2012

Okay, the problem is somewhere else. Zend\Http\Client\Cookies::fromResponse() has to look for SetCookie - the Cookie Header can't occur in a response. RFC2109:

Servers may return a Set-Cookie response headers with any response.
User agents should send Cookie request headers, subject to other
rules detailed below, with every request.

And Wikipedia.

@@ -103,7 +104,7 @@ public function addCookie($cookie, $ref_uri = null)
$cookie = Cookie::fromString($cookie, $ref_uri);
Copy link
Member

Choose a reason for hiding this comment

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

Do you still intend for this to use Cookie (vs. SetCookie)? If so, you need to re-add the import; if not, this should be updated.

@ghost ghost assigned weierophinney Jan 3, 2013
weierophinney added a commit that referenced this pull request Jan 3, 2013
- Changed a reference from Cookie to SetCookie
- Removed empty constructors
- Typehint against Traversable instead of ArrayIterator
@weierophinney weierophinney merged commit 7f470d7 into zendframework:develop Jan 3, 2013
@weierophinney
Copy link
Member

I made the requested changes when merging.

weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
…ctors

- Changed a reference from Cookie to SetCookie
- Removed empty constructors
- Typehint against Traversable instead of ArrayIterator
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants