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

Test RedirectResponse problem report #1486

Merged
merged 4 commits into from
Nov 16, 2018

Conversation

jim-parry
Copy link
Contributor

Added tests for response->redirect() and redirect(), and both seem ok.
Closes #1393

@jim-parry
Copy link
Contributor Author

@natanfelles Made an adjustment to Common::redirect() that looks like it has fixed the problem.

@natanfelles
Copy link
Contributor

Not solved. But I see a light:

$this->sendHeaders();

Headers are sent before the fluent method ->setCookie() is called

@natanfelles
Copy link
Contributor

And is called again in send():

public function send()
{
// If we're enforcing a Content Security Policy,
// we need to give it a chance to build out it's headers.
if ($this->CSPEnabled === true)
{
$this->CSP->finalize($this);
}
else
{
$this->body = str_replace(['{csp-style-nonce}', '{csp-script-nonce}'], '', $this->body);
}
$this->sendHeaders();
$this->sendBody();
$this->sendCookies();
return $this;
}

Maybe because of this works with the Response but not with RedirectResponse.

Firefox Raw Headers repeat some headers: #1393 (comment) - because of the headers sent in redirect() and send(), again.

Something tells me this is there in bootstrap.php

@jim-parry
Copy link
Contributor Author

sendHeader() in redirect() looks out of place, I agree.
I eliminated it, and that does not affect the unit tests - they still pass.
This could very well be something not-testable with unit testing ... headers are a pain at the best of times with phpunit.
In any case, I pushed the change.

@jim-parry
Copy link
Contributor Author

The travis-ci failure is from Redis tests, not related. This is good to test.

@jim-parry jim-parry merged commit fc8424f into codeigniter4:develop Nov 16, 2018
@jim-parry jim-parry deleted the testing8/http branch November 16, 2018 10:11
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

Successfully merging this pull request may close these issues.

2 participants