Skip to content

Proper URI rendering for Zend/Http #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
weierophinney opened this issue Dec 31, 2019 · 1 comment
Closed

Proper URI rendering for Zend/Http #15

weierophinney opened this issue Dec 31, 2019 · 1 comment

Comments

@weierophinney
Copy link
Member

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7479
User: @afeder
Created On: 2015-05-03T15:26:42Z
Updated At: 2015-06-03T17:28:40Z
Body
Fixes #7472.


Comment

User: @Martin-P
Created On: 2015-05-03T17:32:43Z
Updated At: 2015-05-03T17:32:43Z
Body
This PR needs unittests.


Comment

User: @afeder
Created On: 2015-05-03T17:36:07Z
Updated At: 2015-05-03T17:39:57Z
Body
Does any guidance exist as to what kinds of unit tests would be needed?


Comment

User: @samsonasik
Created On: 2015-05-03T18:48:36Z
Updated At: 2015-05-03T18:48:36Z
Body
need rebase


Comment

User: @afeder
Created On: 2015-05-03T18:59:30Z
Updated At: 2015-05-03T18:59:30Z
Body
samsonasik: Indeed. Done.


Comment

User: @afeder
Created On: 2015-05-03T19:24:02Z
Updated At: 2015-05-03T19:24:02Z
Body
I added a test for the issue described in #7472.


Comment

User: @samsonasik
Created On: 2015-05-08T23:46:11Z
Updated At: 2015-05-08T23:46:11Z
Body
need rebase again


Comment

User: @afeder
Created On: 2015-05-09T00:53:00Z
Updated At: 2015-05-09T00:53:00Z
Body
Rebase done.


Comment

User: @Martin-P
Created On: 2015-05-09T12:05:19Z
Updated At: 2015-05-09T12:05:19Z
Body
IMO this needs PR needs many more tests.

There are multiple new methods introduced in Zend\Http\Request which are not tested at all:

  • setOptions
  • setArgSeparator
  • getArgSeparator
  • renderQueryString
  • renderUri

A big chunk of code in Zend\Http\Client has been removed and new code is added, but no tests have been added for this.

The only test which has been added is a very basic query string example. What happens with more advanced query strings? Does that work too or will it fail?


Comment

User: @afeder
Created On: 2015-05-09T15:52:06Z
Updated At: 2015-05-09T16:03:00Z
Body
If you share with me what tests you are looking for I shall add them right away.

The big chunks of code being removed or added is little more than migrating the URI building logic from the Client class into the Request class.

I've found two existing tests pertaining to this code which I've now copied over to the tests for the Request class. I've also expanded the query test to cover the case of multiple query parameters being set, and duplicated this test for the renderUri() and renderQueryString() functions.


Comment

User: @afeder
Created On: 2015-05-12T02:13:32Z
Updated At: 2015-05-12T18:21:12Z
Body
I fixed the exception in the setOptions() function per sasezaki's comment. As for testing the function, none exist for the original code in Client.php, but I added a basic one anyway.



Originally posted by @GeeH at zendframework/zend-http#68

@weierophinney
Copy link
Member Author

This package is considered feature-complete, and is now in security-only maintenance mode, following a decision by the Technical Steering Committee.
If you have a security issue, please follow our security reporting guidelines.
If you wish to take on the role of maintainer, please nominate yourself

You can continue using laminas/laminas-http safely.
Its successor will be PSR-7 in a later revision of laminas/laminas-mvc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant