Http\Request behaviour

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Http\Request behaviour

DeNix
Hi ZF team,
I'm looking at Http\Request class and I see that Uri instance and
queryParams are not connected
so if you do something like:

$request = new Zend\Http\Request();
$request->setUri('http://www.example.com/some/path?param1=1&param2=2');

var_dump($request->query());
var_dump($request->uri()->getQuery());

result is:

object(Zend\Stdlib\Parameters)#4 (1) {
   ["storage":"ArrayObject":private]=>
   array(0) {
   }
}
string(17) "param1=1&param2=2"

Expected behaviour is when you setUri object that has query, queryParams
gets changed, and vice versa.

In general HTTP stack looks like work in progress, Http\Client in particular
Is someone working on it, or is it ok to submit PRs

Thanks
Denis
Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

weierophinney
Administrator
-- Denis Portnov <[hidden email]> wrote
(on Sunday, 03 June 2012, 02:52 AM +0400):

> I'm looking at Http\Request class and I see that Uri instance and
> queryParams are not connected
> so if you do something like:
>
> $request = new Zend\Http\Request();
> $request->setUri('http://www.example.com/some/path?param1=1&param2=2');
>
> var_dump($request->query());
> var_dump($request->uri()->getQuery());
>
> result is:
>
> object(Zend\Stdlib\Parameters)#4 (1) {
>   ["storage":"ArrayObject":private]=>
>   array(0) {
>   }
> }
> string(17) "param1=1&param2=2"
>
> Expected behaviour is when you setUri object that has query,
> queryParams gets changed, and vice versa.
>
> In general HTTP stack looks like work in progress, Http\Client in particular
> Is someone working on it, or is it ok to submit PRs

Feel free to submit PRs. I know that Paddy and Shahar were working on
request pool support, but I'm unaware of any other on-going initiatives
right now. The support currently works for the various use cases of the
framework, but I know there's a lot of detail work that could still be
done (particularly surrounding the various header types).

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

padraicb
I'll touch bases with Shahar when Escaper is out the proverbial door (kicked out with a vengeance!).

Best thing to do for issues like this are to create PRs (use issue tracker if necessary, but PRs with fixes are far better if you can contribute the time). The client is a work in progress - it's a bit of a jigsaw puzzle where the various pieces don't quite fit together perfectly and where edge cases will be inevitable. When I get around to it (next week is likely), I'll run it against a different test suite I can modify to find the weak spots (HTTP stuff is all the same everywhere - so this is the fastest way) - from there it can be refactored into whatever final design is needed. Hopefully, Shahar (at least one other had an interest too) will have some available time coming up to help firm this up.

Paddy
 
Pádraic Brady

http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team


From: Matthew Weier O'Phinney <[hidden email]>
To: [hidden email]
Sent: Tuesday, June 5, 2012 12:52 PM
Subject: Re: [zf-contributors] Http\Request behaviour

-- Denis Portnov <[hidden email]> wrote
(on Sunday, 03 June 2012, 02:52 AM +0400):

> I'm looking at Http\Request class and I see that Uri instance and
> queryParams are not connected
> so if you do something like:
>
> $request = new Zend\Http\Request();
> $request->setUri('http://www.example.com/some/path?param1=1¶m2=2');
>
> var_dump($request->query());
> var_dump($request->uri()->getQuery());
>
> result is:
>
> object(Zend\Stdlib\Parameters)#4 (1) {
>  ["storage":"ArrayObject":private]=>
>  array(0) {
>  }
> }
> string(17) "param1=1&param2=2"
>
> Expected behaviour is when you setUri object that has query,
> queryParams gets changed, and vice versa.
>
> In general HTTP stack looks like work in progress, Http\Client in particular
> Is someone working on it, or is it ok to submit PRs

Feel free to submit PRs. I know that Paddy and Shahar were working on
request pool support, but I'm unaware of any other on-going initiatives
right now. The support currently works for the various use cases of the
framework, but I know there's a lot of detail work that could still be
done (particularly surrounding the various header types).

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc


Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

DeNix
The problem with this particular issue is that it requires design changes,
Http\Request exposes both Uri object and query as Params object, those are not connected,  so updating query in one doesn't change the other
there are several ways to fix, but all require to change Http\Request design a bit
So it's not just simple bugfix PR, that what my inital question was about.

On 05.06.2012 21:51, Pádraic Brady wrote:
I'll touch bases with Shahar when Escaper is out the proverbial door (kicked out with a vengeance!).

Best thing to do for issues like this are to create PRs (use issue tracker if necessary, but PRs with fixes are far better if you can contribute the time). The client is a work in progress - it's a bit of a jigsaw puzzle where the various pieces don't quite fit together perfectly and where edge cases will be inevitable. When I get around to it (next week is likely), I'll run it against a different test suite I can modify to find the weak spots (HTTP stuff is all the same everywhere - so this is the fastest way) - from there it can be refactored into whatever final design is needed. Hopefully, Shahar (at least one other had an interest too) will have some available time coming up to help firm this up.

Paddy
 
Pádraic Brady

http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team


From: Matthew Weier O'Phinney [hidden email]
To: [hidden email]
Sent: Tuesday, June 5, 2012 12:52 PM
Subject: Re: [zf-contributors] Http\Request behaviour

-- Denis Portnov <[hidden email]> wrote
(on Sunday, 03 June 2012, 02:52 AM +0400):
> I'm looking at Http\Request class and I see that Uri instance and
> queryParams are not connected
> so if you do something like:
>
> $request = new Zend\Http\Request();
> $request->setUri('http://www.example.com/some/path?param1=1¶m2=2');
>
> var_dump($request->query());
> var_dump($request->uri()->getQuery());
>
> result is:
>
> object(Zend\Stdlib\Parameters)#4 (1) {
>  ["storage":"ArrayObject":private]=>
>  array(0) {
>  }
> }
> string(17) "param1=1&param2=2"
>
> Expected behaviour is when you setUri object that has query,
> queryParams gets changed, and vice versa.
>

Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

weierophinney
Administrator
-- Denis Portnov <[hidden email]> wrote
(on Tuesday, 05 June 2012, 10:51 PM +0400):
> The problem with this particular issue is that it requires design changes,
> Http\Request exposes both Uri object and query as Params object, those are not
> connected,  so updating query in one doesn't change the other
> there are several ways to fix, but all require to change Http\Request design a
> bit
> So it's not just simple bugfix PR, that what my inital question was about.

Basically, you'll need to bridge the two. While it's a design change,
it's not really an API change, and will help make it more internally
consistent -- so go for it.

> On 05.06.2012 21:51, Pádraic Brady wrote:
>
>     I'll touch bases with Shahar when Escaper is out the proverbial door
>     (kicked out with a vengeance!).
>
>     Best thing to do for issues like this are to create PRs (use issue tracker
>     if necessary, but PRs with fixes are far better if you can contribute the
>     time). The client is a work in progress - it's a bit of a jigsaw puzzle
>     where the various pieces don't quite fit together perfectly and where edge
>     cases will be inevitable. When I get around to it (next week is likely),
>     I'll run it against a different test suite I can modify to find the weak
>     spots (HTTP stuff is all the same everywhere - so this is the fastest way)
>     - from there it can be refactored into whatever final design is needed.
>     Hopefully, Shahar (at least one other had an interest too) will have some
>     available time coming up to help firm this up.
>
>     Paddy
>      
>     Pádraic Brady
>
>     http://blog.astrumfutura.com
>     http://www.survivethedeepend.com
>     Zend Framework Community Review Team
>    
>
>         ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
>         From: Matthew Weier O'Phinney <[hidden email]>
>         To: [hidden email]
>         Sent: Tuesday, June 5, 2012 12:52 PM
>         Subject: Re: [zf-contributors] Http\Request behaviour
>
>         -- Denis Portnov <[hidden email]> wrote
>         (on Sunday, 03 June 2012, 02:52 AM +0400):
>         > I'm looking at Http\Request class and I see that Uri instance and
>         > queryParams are not connected
>         > so if you do something like:
>         >
>         > $request = new Zend\Http\Request();
>         > $request->setUri('http://www.example.com/some/path?param1=1¶m2=2');
>         >
>         > var_dump($request->query());
>         > var_dump($request->uri()->getQuery());
>         >
>         > result is:
>         >
>         > object(Zend\Stdlib\Parameters)#4 (1) {
>         >  ["storage":"ArrayObject":private]=>
>         >  array(0) {
>         >  }
>         > }
>         > string(17) "param1=1&param2=2"
>         >
>         > Expected behaviour is when you setUri object that has query,
>         > queryParams gets changed, and vice versa.
>         >
>
>

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

Shahar Evron-2
In reply to this post by weierophinney
Hi,

I'm actually not working on request pools - I am working on a complete (more or less) rewrite of the HTTP client stack (improving the API, refactoring the transport adapters and adding abstract request / response entity support), although changes to Request / Response are minimal. This has been quite slow going but is getting close to being fairly usable - you can see the work in https://github.com/shevron/zf2/tree/feature/http-client-rewrite (and I would be very happy with feedback and comments).

I am not sure I have a fix for the issue you've mentioned. It's somewhat tricky to obtain full synchronization between parameters passed in the URI and parameters added to ->query(). We may be able to build something that makes sense in 90% of the cases and document the rest so behavior (precedence of one over the other) is clear.
 
Shahar.

On Tue, Jun 5, 2012 at 2:52 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
-- Denis Portnov <[hidden email]> wrote
(on Sunday, 03 June 2012, 02:52 AM +0400):
> I'm looking at Http\Request class and I see that Uri instance and
> queryParams are not connected
> so if you do something like:
>
> $request = new Zend\Http\Request();
> $request->setUri('http://www.example.com/some/path?param1=1&param2=2');
>
> var_dump($request->query());
> var_dump($request->uri()->getQuery());
>
> result is:
>
> object(Zend\Stdlib\Parameters)#4 (1) {
>   ["storage":"ArrayObject":private]=>
>   array(0) {
>   }
> }
> string(17) "param1=1&param2=2"
>
> Expected behaviour is when you setUri object that has query,
> queryParams gets changed, and vice versa.
>
> In general HTTP stack looks like work in progress, Http\Client in particular
> Is someone working on it, or is it ok to submit PRs

Feel free to submit PRs. I know that Paddy and Shahar were working on
request pool support, but I'm unaware of any other on-going initiatives
right now. The support currently works for the various use cases of the
framework, but I know there's a lot of detail work that could still be
done (particularly surrounding the various header types).

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

DeNix
Hi Shahar,

It's not possible to have a full sync because according to RFC query is any arbitrary string, not just delimitered key/value pairs
so my plan is this:
-- extend Zend\Uri to sopport Stdlib\Params for query (queryParams)
-- bridge Http\Request::queryParams with Uri::queryParams
-- fix all dependencies

I like what you did with Http\Client, few questions though

- is injecting Logger into transport is by design? I think it's better to connect EventManager, so one could have any listener for Client (or Transport) events
- do you plan to work on Curl transport?

Thanks
Denis


On 06.06.2012 11:51, Shahar Evron wrote:
Hi,

I'm actually not working on request pools - I am working on a complete (more or less) rewrite of the HTTP client stack (improving the API, refactoring the transport adapters and adding abstract request / response entity support), although changes to Request / Response are minimal. This has been quite slow going but is getting close to being fairly usable - you can see the work in https://github.com/shevron/zf2/tree/feature/http-client-rewrite (and I would be very happy with feedback and comments).

I am not sure I have a fix for the issue you've mentioned. It's somewhat tricky to obtain full synchronization between parameters passed in the URI and parameters added to ->query(). We may be able to build something that makes sense in 90% of the cases and document the rest so behavior (precedence of one over the other) is clear.
 
Shahar.

On Tue, Jun 5, 2012 at 2:52 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
-- Denis Portnov <[hidden email]> wrote
(on Sunday, 03 June 2012, 02:52 AM +0400):
> I'm looking at Http\Request class and I see that Uri instance and
> queryParams are not connected
> so if you do something like:
>
> $request = new Zend\Http\Request();
> $request->setUri('http://www.example.com/some/path?param1=1&param2=2');
>
> var_dump($request->query());
> var_dump($request->uri()->getQuery());
>
> result is:
>
> object(Zend\Stdlib\Parameters)#4 (1) {
>   ["storage":"ArrayObject":private]=>
>   array(0) {
>   }
> }
> string(17) "param1=1&param2=2"
>
> Expected behaviour is when you setUri object that has query,
> queryParams gets changed, and vice versa.
>
> In general HTTP stack looks like work in progress, Http\Client in particular
> Is someone working on it, or is it ok to submit PRs

Feel free to submit PRs. I know that Paddy and Shahar were working on
request pool support, but I'm unaware of any other on-going initiatives
right now. The support currently works for the various use cases of the
framework, but I know there's a lot of detail work that could still be
done (particularly surrounding the various header types).

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc


Reply | Threaded
Open this post in threaded view
|

Re: Http\Request behaviour

Shahar Evron-2
Hi,

On Wed, Jun 6, 2012 at 6:15 PM, Denis Portnov <[hidden email]> wrote:
Hi Shahar,

It's not possible to have a full sync because according to RFC query is any arbitrary string, not just delimitered key/value pairs
so my plan is this:
-- extend Zend\Uri to sopport Stdlib\Params for query (queryParams)
-- bridge Http\Request::queryParams with Uri::queryParams

Sounds interesting - I'd be happy to review that if you want.

-- fix all dependencies

I like what you did with Http\Client, few questions though

- is injecting Logger into transport is by design? I think it's better to connect EventManager, so one could have any listener for Client (or Transport) events

The logging stuff in the transport is just old leftovers and should be ignored. It definately makes more sense to provide generic events so that different handlers could be connected using an EventManager.
 
- do you plan to work on Curl transport?

I would like to have a Curl transport implemented - I am not sure if I will do it or someone else will but I think it makes sense.

Shahar.
 

Thanks
Denis



On 06.06.2012 11:51, Shahar Evron wrote:
Hi,

I'm actually not working on request pools - I am working on a complete (more or less) rewrite of the HTTP client stack (improving the API, refactoring the transport adapters and adding abstract request / response entity support), although changes to Request / Response are minimal. This has been quite slow going but is getting close to being fairly usable - you can see the work in https://github.com/shevron/zf2/tree/feature/http-client-rewrite (and I would be very happy with feedback and comments).

I am not sure I have a fix for the issue you've mentioned. It's somewhat tricky to obtain full synchronization between parameters passed in the URI and parameters added to ->query(). We may be able to build something that makes sense in 90% of the cases and document the rest so behavior (precedence of one over the other) is clear.
 
Shahar.

On Tue, Jun 5, 2012 at 2:52 PM, Matthew Weier O'Phinney <[hidden email]> wrote:
-- Denis Portnov <[hidden email]> wrote
(on Sunday, 03 June 2012, 02:52 AM +0400):
> I'm looking at Http\Request class and I see that Uri instance and
> queryParams are not connected
> so if you do something like:
>
> $request = new Zend\Http\Request();
> $request->setUri('http://www.example.com/some/path?param1=1&param2=2');
>
> var_dump($request->query());
> var_dump($request->uri()->getQuery());
>
> result is:
>
> object(Zend\Stdlib\Parameters)#4 (1) {
>   ["storage":"ArrayObject":private]=>
>   array(0) {
>   }
> }
> string(17) "param1=1&param2=2"
>
> Expected behaviour is when you setUri object that has query,
> queryParams gets changed, and vice versa.
>
> In general HTTP stack looks like work in progress, Http\Client in particular
> Is someone working on it, or is it ok to submit PRs

Feel free to submit PRs. I know that Paddy and Shahar were working on
request pool support, but I'm unaware of any other on-going initiatives
right now. The support currently works for the various use cases of the
framework, but I know there's a lot of detail work that could still be
done (particularly surrounding the various header types).

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc