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

Response setHeaders overwrites setContentType #12836

Closed
linxlad opened this Issue May 6, 2017 · 3 comments

Comments

Projects
5 participants
@linxlad
Copy link
Member

linxlad commented May 6, 2017

Reponse setHeaders is scrubbing setContentType.

Expected and Actual Behavior

Expected:
I expected the reponse->setHeaders to merge with what was already in headers.

Actual :
I had response->setContentType('application/json') before response->setHeaders. The result meant I was losing the desired content type and it was reverting back to text/html.

Working:

$this->response->setHeaders($headers);
$this->response->setContentType('application/json', 'UTF-8');

Not working:

$this->response->setContentType('application/json', 'UTF-8');
$this->response->setHeaders($headers);

Details

  • Phalcon version: 3.1.2
  • PHP Version: 7.0.18
  • Operating System: Windows running a Doker environment for development.
  • Installation type: Docker box.
  • Zephir version (if any):
  • Server: Nginx
@dschissler

This comment has been minimized.

Copy link
Contributor

dschissler commented May 6, 2017

I recommend that this be added to the Phalcon 4 milestone with merging on by default. If it is added in Phalcon 3 then merging will need to be off by default. It can be currently worked around by extending the Response class and calling getHeaders in setHeaders.

@sergeyklay sergeyklay added this to the 4.0.0 milestone May 6, 2017

@sergeyklay sergeyklay added the Bug - Low label May 6, 2017

@Jurigag

This comment has been minimized.

Copy link
Member

Jurigag commented May 8, 2017

@linxlad

Well not sure what you exactly do further but $this->response->setContentType('application/json', 'UTF-8'); is most likely not needed just do:

$model = $model->toArray();
return $this->request->setJsonContent($model);

@stale stale bot added the stale label Apr 16, 2018

@sergeyklay sergeyklay closed this Apr 16, 2018

@sergeyklay sergeyklay reopened this May 2, 2018

@stale stale bot removed the stale label May 2, 2018

@niden niden added this to In progress in 4.0 Release Nov 28, 2018

@niden niden moved this from In progress to To do in 4.0 Release Nov 28, 2018

@phalcon phalcon deleted a comment from stale bot Dec 18, 2018

@niden niden moved this from To do to In progress in 4.0 Release Dec 18, 2018

niden added a commit to niden/cphalcon that referenced this issue Dec 18, 2018

niden added a commit to niden/cphalcon that referenced this issue Dec 18, 2018

@niden niden referenced this issue Dec 18, 2018

Merged

T12836 set headers merges the headers #13669

3 of 3 tasks complete

niden added a commit to niden/cphalcon that referenced this issue Dec 18, 2018

niden added a commit that referenced this issue Dec 18, 2018

Merge branch 'niden-T12836-set-headers' into 4.0.x
* niden-T12836-set-headers:
  [#12836] - Updated the changelog
  [#12836] - Merging headers using setHeaders; Added tests
  [#12836] - Reformatted the code

@niden niden moved this from In progress to Done in 4.0 Release Dec 18, 2018

@niden

This comment has been minimized.

Copy link
Member

niden commented Dec 22, 2018

Addressed in #13669

Thank you @linxlad

@niden niden closed this Dec 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment