Skip to content
New issue

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

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

Already on GitHub? # to your account

4.x - Fix RouteCollectorProxy::redirect() Bug #2816

Merged
merged 1 commit into from
Aug 20, 2019

Conversation

l0gicgate
Copy link
Member

@l0gicgate l0gicgate commented Aug 20, 2019

Fix for #2815

When a container is present the callable gets bound to the container when it's resolved. So the $this context becomes the container and not RouteCollectorProxy anymore.

$handler = function () use ($to, $status) {
    $response = $this->responseFactory->createResponse($status);
    return $response->withHeader('Location', (string) $to);
};

We're using $this->responseFactory() when we should just pass it in via use()

@l0gicgate l0gicgate added this to the 4.2.0 milestone Aug 20, 2019
@l0gicgate l0gicgate requested a review from adriansuter August 20, 2019 15:13
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5128833 on l0gicgate:FixRedirectBug into 806adf2 on slimphp:4.x.

@adriansuter
Copy link
Contributor

Is there a reason for the container prophecy in the test case? CallableResolver can be used without.

Other than that, everything looks fine. Can't officially approve as my phone app (OctoDroid) has a bug.

@adriansuter
Copy link
Contributor

Ah, should have read the issue. Everything is good!

@l0gicgate l0gicgate merged commit 89f8076 into slimphp:4.x Aug 20, 2019
@l0gicgate l0gicgate deleted the FixRedirectBug branch August 20, 2019 15:47
@l0gicgate l0gicgate mentioned this pull request Aug 20, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants