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

How to change response header ? #64

Closed
Runrioter opened this issue Dec 1, 2018 · 14 comments
Closed

How to change response header ? #64

Runrioter opened this issue Dec 1, 2018 · 14 comments
Labels

Comments

@Runrioter
Copy link

Hi buddies:

I set response header.

function version(r) {
    r.headersOut['Content-Type'] = 'text/html';
    r.return(200, njs.version);
}

But I got two Content-Type header (another comes from default_type).

I try to delete it, but it doesn't work either. (crash)

function version(r) {
    delete r.headersOut['Content-Type'];
    r.headersOut['Content-Type'] = 'text/html';
    r.return(200, njs.version);
}
@xeioex xeioex added the bug label Dec 1, 2018
@xeioex
Copy link
Contributor

xeioex commented Dec 1, 2018

Hi @Runrioter,

  1. The issue with 'Content-Type' (as well as other often used headers) is that it is treated in a special way by nginx (by a custom field)
    (https://github.com/nginx/nginx/blob/master/src/http/ngx_http_request.h#L277)

You can set contentType now by
r.contentType ='text/html'

In the future, I am planning to get rid of special setters (like r.contentType).
It will be possible to set Content-Type simply by
r.headersOut['Content-Type'] = 'text/html'

  1. the delete issue
    Thank you for reporting it. Will be fixed.

@xeioex
Copy link
Contributor

xeioex commented Dec 1, 2018

Is related to #43

@Runrioter
Copy link
Author

@xeioex Thanks for your quick reply. I like the njs module. Hope that it will become better.

@xeioex
Copy link
Contributor

xeioex commented Dec 3, 2018

Hi @Runrioter ,

Could you please report the njs version.

I am trying to reproduce the delete issue (using 0.2.6).

function version(r) {
    delete r.headersOut['Content-Type'];
    r.headersOut['Content-Type'] = 'text/html';
    r.return(200, njs.version);
}
2018/12/03 14:56:10 [error] 22597#22597: *2 js exception: TypeError: Cannot delete property 'Content-Type' of external
    at version (:82)
    at main (native)

@Runrioter
Copy link
Author

Yes. I use 0.2.6, too.

2018/12/03 13:12:52 [error] 91#91: *83 js exception: TypeError: Cannot delete property 'Content-Type' of external
    at version (:1)
    at main (native)

@xeioex
Copy link
Contributor

xeioex commented Dec 3, 2018

So, how can I reproduce the crash?

@Runrioter
Copy link
Author

Runrioter commented Dec 3, 2018

Sorry, I mean the crash is the js exception. Is this js exception TypeError reasonable ? Need I try catch this exception ?

external means what ?

delete operator can't throw TypeError. Refer to here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete

@xeioex
Copy link
Contributor

xeioex commented Dec 3, 2018

external means what ?

It is a special value type (a thin, zero-copy wrapper around nginx C-structures). externals is a way to introduce native nginx object into the engine.

For example r - nginx request is external.
Currently, externals behave like semi-Objects. You can do some js operations on them.

In C-code, you have to define some C level functions to support the operations (like iterations, property get, property set..) (https://github.com/nginx/njs/blob/master/nginx/ngx_http_js_module.c#L478). In the future, I hope to get rid of the externals, but it is not easy.

Currently, there is no C-level function which implements delete operation for r.headersOut object.

Is this js exception TypeError reasonable ?

njs aims to implement strict mode only. In the strict mode, you get TypeError exception (according to ES5.1 https://www.ecma-international.org/ecma-262/5.1/#sec-11.4.1) if delete operator fails (njs treats absence of C-level delete function as a configure:false).

@Runrioter
Copy link
Author

@xeioex Thank you. I learnt a lot. You can close this issue at any time.

@mfaerevaag
Copy link

mfaerevaag commented Jan 17, 2019

Hi! I'm having the problem when trying to change the content type.

Using r.headersOut['Content-Type'] = 'text/plain', the header is not changed, but added in addition to the original:

HTTP/1.1 200 OK
Server: nginx/1.15.8
Date: Thu, 17 Jan 2019 07:47:24 GMT
Content-Type: application/octet-stream
Content-Length: 0
Connection: close
Content-Type: text/plain     <-- here

When using the proposed r.contentType = 'text/plain', I get the following error:

2019/01/17 07:38:21 [error] 2263#2263: *376 js exception: TypeError: Cannot assign to read-only property 'contentType' of external
    at jsLogBasicAuth (:17)
    at main (native)
, client: 1.2.3.4, server: foo.bar, request: "GET /test/basic-auth/ HTTP/1.1", host: "foo.bar"

Any solution on this? I would prefer if the content type 'text/plain' was default, which would make more sense IMO when doing a simple r.return(200, "Hello, World\n").

Awesome project btw! Very helpful for doing security testing.

EDIT: Seems that the following code works:

r.status = 200;
r.headersOut['Content-Type'] = 'text/plain';
r.sendHeader();
r.finish();

Which returns the following correct response:

HTTP/1.1 200 OK
Server: nginx/1.15.8
Date: Thu, 17 Jan 2019 08:06:55 GMT
Connection: close
Content-Type: text/plain
Content-Length: 0

@xeioex
Copy link
Contributor

xeioex commented Jan 17, 2019

Hi @mfaerevaag,

The issue with some special headers still persists (the reason for that is explaned here).

As a workaround:
You can set special headers using nginx directives:
default_type for Content-Type header. add_header for other headers.

or the following code (r.response object is deprecated, and will be removed in the future):

r.response.contentType = 'text/plain';  // <- modifies resulting content_type header in-place.                                                                              
r.return(200, "Hello world\n");

@mfaerevaag
Copy link

Alright, thanks!

@nishant-dani
Copy link

When I try to set
r.response.contentType = 'application/json';
req.return (reply.status, reply.responseBody)

nginx.1 | 2020/01/08 08:10:27 [error] 59#59: *138 js exception: TypeError: property set on primitive undefined type

@xeioex
Copy link
Contributor

xeioex commented Jan 8, 2020

Hi @nishant-dani

r.response property was removed after 0.2.8 (26 Feb 2019) version.

To change Content-Type header:

r.headersOut['Content-Type'] = 'application/json';

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

No branches or pull requests

4 participants