Skip to content

Conversation

@haoxins
Copy link

@haoxins haoxins commented Mar 27, 2015

No description provided.

@bnoordhuis
Copy link
Member

The current behavior is intentional and what you're proposing would be significantly backwards incompatible. What's the motivation for the change?

@bnoordhuis bnoordhuis added http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 27, 2015
@haoxins
Copy link
Author

haoxins commented Mar 27, 2015

use setHeader before(or not) writeHead will produce different results

res.setHeader('x', 'y');
res.writeHead(200, {
  'aa': 'a1',
  'aA': 'a2'
});

get

aA: a2
x: y

And

res.writeHead(200, {
  'aa': 'a1',
  'aA': 'a2'
});

will get

aA: a2
aa: a1

@mscdex mscdex added confirmed-bug Issues with confirmed bugs. investigate labels Mar 27, 2015
@brendanashworth
Copy link
Contributor

I'd say if the user wants to try and send the same-case headers (a and A are equivalent in HTTP), it isn't worth it to stop them at the expense of the extra CPU runs and perf cost.

@Fishrock123
Copy link
Contributor

I wonder if the opposite wouldn't be more useful? It'd be speedier for sure, and it is also the more common case. (i.e. something set headers before writeHead.)

Ultimately HTTP still needs the eventual re-write isaacs was once planning to do.

@brendanashworth brendanashworth removed the confirmed-bug Issues with confirmed bugs. label Apr 21, 2015
@brendanashworth
Copy link
Contributor

If we're changing functionality, this should error:

res.writeHead(200, {
  'X-BAR': 'foo',
  'x-bar': 'baz'
});

After all, objects are unordered and we can't decide whether to send foo or baz in this case. Therefor, though I don't advocate throwing an error in this case, I think it'd be better than this change.

I'll close this unless someone disagrees.

@brendanashworth
Copy link
Contributor

Closing - if you feel that this wasn't discussed fully, this can always be re-opened.

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

Labels

http Issues or PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants