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

doc(http): res.write(data) followed by res.end() is not actually equivalent to res.end(data) #26005

Closed
peat-psuwit opened this issue Feb 8, 2019 · 4 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.

Comments

@peat-psuwit
Copy link

  • Version: v11.9.0
  • Platform: Linux (My hostname) 4.15.0-45-generic #48-Ubuntu SMP Tue Jan 29 16:28:13 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux (Xubuntu 18.04)
  • Subsystem: Documentation of HTTP module

In the document for method end([data][, encoding][, callback]) of class http.ServerResponse, there's a following text:

If data is specified, it is equivalent to calling response.write(data, encoding) followed by response.end(callback).

However, this is not actually true when there's only single response.write(data), demonstrable with the following code:

const http = require('http');

const HELLO = 'Hello!\n';

const server1 = http.createServer(function (req, res) {
  res.write(HELLO);
  res.end();
});

const server2 = http.createServer(function (req, res) {
  res.end(HELLO);
});

server1.listen(9001);
server2.listen(9002);

By using curl -v, this happens:

$ curl -v http://localhost:9001/ # response.write(data) follow by response.end()
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 9001 (#0)
> GET / HTTP/1.1
> Host: localhost:9001
> User-Agent: curl/7.58.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Fri, 08 Feb 2019 10:26:25 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
Hello!
* Connection #0 to host localhost left intact

$ curl -v http://localhost:9002/ # response.end(data)
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 9002 (#0)
> GET / HTTP/1.1
> Host: localhost:9002
> User-Agent: curl/7.58.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Fri, 08 Feb 2019 10:26:31 GMT
< Connection: keep-alive
< Content-Length: 7
< 
Hello!
* Connection #0 to host localhost left intact
$ 

So, it appears that response.write(data) follow by response.end() cause Node.js to use chunked encoding, while response.end(data) cause Node.js to not using chunked encoding and include Content-Length header. This small distinction is important with some client. For example, Windows 10's MDM enrollment system will not accept chunked response.

peat-psuwit added a commit to peat-psuwit/node-soap that referenced this issue Feb 8, 2019
Using res.end() to write result instead of res.write() followed by
res.end() will cause node not to use chunked encoding and include
Content-Length header. (See nodejs/node#26005) This small distinction is
important with some client. For example, Windows 10's MDM enrollment
system will not accept chunked response
(https://docs.microsoft.com/en-us/windows/client-management/mdm/on-premise-authentication-device-enrollment).

This might improve compatibility with some other clients, too. However,
there's a small chance that some client may expect the response to be
chunked.
peat-psuwit added a commit to peat-psuwit/node-soap that referenced this issue Feb 8, 2019
Using res.end() to write result instead of res.write() followed by
res.end() will cause node not to use chunked encoding and include
Content-Length header. (See nodejs/node#26005) This small distinction is
important with some client. For example, Windows 10's MDM enrollment
system will not accept chunked response
(https://docs.microsoft.com/en-us/windows/client-management/mdm/on-premise-authentication-device-enrollment).

This might improve compatibility with some other clients, too. However,
there's a small chance that some client may expect the response to be
chunked. So, I put this behind an option which defaults to enabled and
can be disabled if needed.
peat-psuwit added a commit to peat-psuwit/node-soap that referenced this issue Feb 8, 2019
Using res.end() to write result instead of res.write() followed by
res.end() will cause node not to use chunked encoding and include
Content-Length header. (See nodejs/node#26005) This small distinction is
important with some client. For example, Windows 10's MDM enrollment
system will not accept chunked response
(https://docs.microsoft.com/en-us/windows/client-management/mdm/on-premise-authentication-device-enrollment).

This might improve compatibility with some other clients, too. However,
there's a small chance that some client may expect the response to be
chunked. So, I put this behind an option which defaults to enabled and
can be disabled if needed.
@ChALkeR ChALkeR added doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem. labels Feb 8, 2019
jsdevel pushed a commit to vpulim/node-soap that referenced this issue Feb 11, 2019
Using res.end() to write result instead of res.write() followed by
res.end() will cause node not to use chunked encoding and include
Content-Length header. (See nodejs/node#26005) This small distinction is
important with some client. For example, Windows 10's MDM enrollment
system will not accept chunked response
(https://docs.microsoft.com/en-us/windows/client-management/mdm/on-premise-authentication-device-enrollment).

This might improve compatibility with some other clients, too. However,
there's a small chance that some client may expect the response to be
chunked. So, I put this behind an option which defaults to enabled and
can be disabled if needed.
@lpinca
Copy link
Member

lpinca commented Mar 4, 2019

@peat-psuwit are you interested in submitting a PR to fix the documentation?

@lpinca lpinca added the good first issue Issues that are suitable for first-time contributors. label Mar 4, 2019
@peat-psuwit
Copy link
Author

Hmm, I'm not sure how I should change the document. Also, after I read some comment in the source I start to wonder if the current behavior is not an intended behavior.

@lpinca
Copy link
Member

lpinca commented Mar 4, 2019

I think current behavior is intended, but the sentence you quoted is misleading. Perhaps remove it or reword it to match current behavior?

cc: @nodejs/http

@lpinca
Copy link
Member

lpinca commented Mar 4, 2019

Current behavior was defined in #1062. Before that both cases in your example returned Transfer-Encoding: chunked.

lpinca added a commit to lpinca/node that referenced this issue Mar 7, 2019
Calling `response.end(data)` is not 100% equivalent to calling
`response.write(data)` followed by `response.end()`.

Fixes: nodejs#26005
BridgeAR pushed a commit that referenced this issue Mar 14, 2019
Calling `response.end(data)` is not 100% equivalent to calling
`response.write(data)` followed by `response.end()`.

PR-URL: #26465
Fixes: #26005
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this issue Mar 14, 2019
Calling `response.end(data)` is not 100% equivalent to calling
`response.write(data)` followed by `response.end()`.

PR-URL: nodejs#26465
Fixes: nodejs#26005
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
BridgeAR pushed a commit that referenced this issue Mar 14, 2019
Calling `response.end(data)` is not 100% equivalent to calling
`response.write(data)` followed by `response.end()`.

PR-URL: #26465
Fixes: #26005
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
BethGriggs pushed a commit that referenced this issue Apr 16, 2019
Calling `response.end(data)` is not 100% equivalent to calling
`response.write(data)` followed by `response.end()`.

PR-URL: #26465
Fixes: #26005
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants