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: reword message.headers #3814

Closed
wants to merge 1 commit into from

Conversation

tflanagan
Copy link
Contributor

message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Issue: #3146


'use strict';

const http = require('http');

http.createServer(function(req, res){
    req.headers.host = 'anotherhost';

    res.writeHead(200, {'Content-Type': 'text/plain'});
    res.end(req.headers.host);
}).listen(8124);

console.log('Server running at http://127.0.0.1:8124/');

// "anotherhost"

message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Issue: nodejs#3146
@mscdex mscdex added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Nov 13, 2015
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

LGTM.
This brings up a good point tho about whether these are supposed to be read only but aren't (a code bug) or whether it's just that the doc doesn't match the implementation. @indutny @mikeal

@indutny
Copy link
Member

indutny commented Nov 13, 2015

@jasnell they are kind of read-only (ideally), but in fact this is expensive to enforce. So the key-value pairs seems to be precise definition.

@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Ok, then it's a doc bug. 👍

@r-52
Copy link
Contributor

r-52 commented Nov 13, 2015

LGTM

jasnell pushed a commit that referenced this pull request Nov 13, 2015
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Landed in 934149a

@jasnell jasnell closed this Nov 13, 2015
@tflanagan tflanagan deleted the doc-http-headers-kvp branch November 13, 2015 20:16
Fishrock123 pushed a commit that referenced this pull request Nov 17, 2015
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2015
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

landed in v4.x-staging in fc629c2

rvagg pushed a commit that referenced this pull request Dec 4, 2015
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
message.headers states that the headers are read-only, when in fact they
are not. This change rewords the docs to a more appropriate description,
while not promoting this kind of behavior.

Fixes: #3146
PR-URL: #3814
Reviewed-By: Roman Klauke <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

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. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants