Skip to content

Conversation

@apapirovski
Copy link
Contributor

Some assorted changes to mapToHeaders behaviour, adjusted tests and also an expanded error message.

  • No longer check whether key is a symbol as Object.keys does not return symbols (or, well, anything other than strings)
  • No longer convert key to string as it is always a string (^ see above)
  • Validate that only one value is passed for each pseudo-header
  • Extend illegal connection header error message to include the name of the problematic header
  • Extend tests to cover this behaviour
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

http2, test

No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Expand tests to cover this behaviour. Expand illegal connection header message to include the name of the problematic header.
@apapirovskiapapirovski added the http2 Issues or PRs related to the http2 subsystem. label Oct 28, 2017
@nodejs-github-botnodejs-github-bot added dont-land-on-v4.x errors Issues and PRs related to JavaScript errors originated in Node.js core. http2 Issues or PRs related to the http2 subsystem. labels Oct 28, 2017
@apapirovski
Copy link
ContributorAuthor

@jasnell
Copy link
Member

I'm +1 on fast tracking landing on this so it can get in for 9.0.0

@apapirovski
Copy link
ContributorAuthor

Landing this right now.

@apapirovski
Copy link
ContributorAuthor

Landed in 980ebd2

@apapirovskiapapirovski deleted the patch-http2-map-to-headers branch October 29, 2017 17:07
apapirovski added a commit that referenced this pull request Oct 29, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: #16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: #16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: #16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
gibfahn pushed a commit that referenced this pull request Oct 31, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: #16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
@gibfahngibfahn mentioned this pull request Oct 31, 2017
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: nodejs/node#16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Qard pushed a commit to ayojs/ayo that referenced this pull request Nov 2, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: nodejs/node#16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: nodejs/node#16575 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

errorsIssues and PRs related to JavaScript errors originated in Node.js core.http2Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@apapirovski@jasnell@addaleax@cjihrig@nodejs-github-bot