Zend Framework

I prefer not to put too much effort on the Zend_Http_Client as it is going to be replaced by the incubator Zend_Http_Client which is a complete rewrite.

Do you think this issue concers the new Http_Client as well? This one has easier access to GET parameters and you could either set them individually using $client->setParameterGet('foo', 'baz'), or directly access the query string by doing this:

$client->getUri()->setQuery('foo=baz');

I'm not sure if any further changes are needed. Let me know what you think.

Oh, I didn't know there was a newer version of Zend_Http_Client coming. Naturally, any updates to the old one would be a waste of time.

However, the changes to Zend_Uri_Http (which are the bulk of my suggestion) would still apply. I would prefer having the choice of using an array or string, and I'm sure others would as well.

The changes to Zend_Http_Client also use the native PHP function http_build_query(), which renders Zend_Http_Client::_getParametersRecursive() redundant.

I've only taken a couple of minutes to look at the updated Client, but I'm a bit confused why setting GET parameters is handled in two different places-why there is even that option. It seems to me that GET should be in one place. The most obvious choice is in the URI, since Zend_Uri_Http is or will be used in places other than Zend_Http. In that case, you could simply have setParameterGet() should call $this>uri->setQuery().

What do you think?

Incidentally, a single hyphen on either side is probably a poor choice of syntax for "strikethrough"...

I see your point now, and of course using $this->uri->setQuery() will be better, as it uses the native http_build_query().

However, I would still like to store the GET parameters in an internal array of the client, and only when the request is sent "attach" them to the URI object - and this is important because I want people to be able to set parameters before they set the URI for example, or even set several parameters, then set a URI with it's own parameters, and when the request is sent, not to lose any of them.

Does this makes sense to you? If so, I'll fix up my _prepare_headers() method to use $this->uri->setQuery().

Shahar.

I think that's a good idea, Shahar.

By the way, I've reconsidered the setQuery() function a little. It should probably look more like this:

// Zend_Uri_Http

    public function setQuery($query)
    {
        $this->_query = $this->_parseQuery($query);
    }

    protected function _parseQuery($query)
    {
        if (empty($query)) {
            return "";
        }

        $queryArray = array();
        if (is_array($query)) {
            $queryArray = $query;
        } else if (is_string($query)) {
            parse_str($query, $queryArray);
        }
        if (count($queryArray) < 1) {
            throw new Zend_Uri_Exception('No query parameters found');
        }
        $query = http_build_query($queryArray, '', '&amp;');
        if (!$this->validateQuery($query)) {
            throw new Zend_Uri_Exception('Invalid query string');
        }
        return $query;
    }

I commited Zend_Uri and Zend_Uri_Http revision 1064 which includes your suggestions, although I modified your code a bit.

Please take a look at it and let me know what you think (if you want to).

Do not mark this bug as fixed - I'll later proceed to change Zend_Http_Client as well.

Thanks for making the update. I like the fact that you return the old query in setQuery now.

As far as the changes in _parseQuery, I debated whether or not to include the parse_str part or not. The main purpose there was an additional check for string validity. Which, of course, does not belong in _parseQuery, but validateQuery.

However, the regular expression in that method (validateQuery) doesn't seem to adequately cover query string validity. Would something involving parse_str (perhaps comparing the array it returns to an array generated from manually parsing) be good to add there?

Brain lapse. Ignore the parenthetical aside in the last sentence.

I think the recent changes to Zend_Http_Client and Zend_Uri_Http solve all issues described here.
I don't think using parse_str is right because just for validating the string, it's a bit overkill (building up an array and all). The query string can be validated easily with a regexp, as long as you follow the RFC.

Bookkeeping. Closing old issues and assigning them to the person who ultimately resolved the issue.