Skip to content
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

Add a failure test where / is appended once it move to next middleware #25

Merged
merged 1 commit into from
Jan 20, 2015
Merged

Conversation

harikt
Copy link
Contributor

@harikt harikt commented Jan 20, 2015

No description provided.

});
$request = new Request([], [], 'http://local.example.com/admin', 'GET', 'php://memory');
$result = $this->middleware->__invoke($request, $this->response);
$body = (string) $this->response->getBody();
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be pulling the body from the $result.

Copy link
Contributor

Choose a reason for hiding this comment

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

But the test is still valid, as the body of the request is mutable (as it's a stream). And travis does indeed show it failing -- will get a solution in place today -- thanks for the test case!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Do I need to change it ?

@harikt
Copy link
Contributor Author

harikt commented Jan 20, 2015

If you like to try a gist https://gist.github.com/harikt/9d5546623dfc8bcee711

weierophinney added a commit to weierophinney/conduit that referenced this pull request Jan 20, 2015
Add a failure test where / is appended once it move to next middleware
weierophinney added a commit to weierophinney/conduit that referenced this pull request Jan 20, 2015
Added another test case to test for the inverse condition -- that a
trailing slash **is** present if it was present in the original route.

The fix requires ensuring that the original route is retained, but that
matching is done without trailing slashes. At the same time, the
original route must be retained so that the path is reset correctly --
without double slashes.
@weierophinney weierophinney merged commit 3706f4c into phly:master Jan 20, 2015
weierophinney added a commit that referenced this pull request Jan 20, 2015
@harikt harikt deleted the failure branch January 21, 2015 01:02
weierophinney added a commit to weierophinney/conduit that referenced this pull request Jan 26, 2015
This patch properly fixes phly#25 and phly#27, bringing in v0.8.4, and properly
detecting situations when the matched route appends a "/".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants