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.

Sign up for GitHub

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

Jump to bottom

Make Http2Session maximum invalid frame count configurable #30505

Closed
lundibundi opened this issue Nov 16, 2019 · 1 comment
Closed

Make Http2Session maximum invalid frame count configurable #30505

lundibundi opened this issue Nov 16, 2019 · 1 comment
Labels
discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js. http2 Issues or PRs related to the http2 subsystem.

Comments

@lundibundi
Copy link
Member

Is your feature request related to a problem? Please describe.
We have flaky #29802 that doesn't hit the needed amount of invalid frames and crashes with OOM which may be solved by decreasing the amount of maximum invalid frames for that test.
It may prove useful for people to be able to configure how many invalid frames they want to tolerate though I don't think it'll be a popular feature.
At the same time, it shouldn't add too much of a maintenance burden to us.

Describe the solution you'd like
Probably option of maxSessionInvalidFrames (comepletely open to better names) to http2.createServer()/http2.createSecureServer() and possibly http2.connect(). I don't think this needs to be changed on the go and can be set upon server creation, though dynamic change may be useful.

Though I'm not sure how to pass that to our C++-land Http2Session? (via js_fields_; property on Http2Session accessible from JS; constructor param?)

Describe alternatives you've considered
None probably, it's hardcoded so it can stay that way.

/cc @addaleax as the one who added invalid frame counting.
/cc @nodejs/http2
@lundibundi lundibundi added discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js. http2 Issues or PRs related to the http2 subsystem. labels Nov 16, 2019
@addaleax
Copy link
Member

@lundibundi I think using js_fields_ to pass this would be fine, and generally this SGTM … do you want to open a PR?

@lundibundi lundibundi mentioned this issue Nov 18, 2019
Closed
4 tasks
addaleax pushed a commit that referenced this issue Nov 27, 2019
@lundibundi @addaleax
http2: allow to configure maximum tolerated invalid frames
9590f57 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 27, 2019
@lundibundi @addaleax
test: update and harden http2-reset-flood
74f6bc7 
* use new maxSessionInvalidFrames to lower the needed frames
* slow down requests to generate less redundant after-session-close
  requests

PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 27, 2019
@lundibundi @addaleax
http2: make maximum tolerated rejected streams configurable
988034b 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
@lundibundi @addaleax
http2: replace direct array usage with struct for js_fields_
e92afd9 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
@lundibundi @addaleax
http2: allow to configure maximum tolerated invalid frames
092a3c2 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
@lundibundi @addaleax
test: update and harden http2-reset-flood
5ecfd94 
* use new maxSessionInvalidFrames to lower the needed frames
* slow down requests to generate less redundant after-session-close
  requests

PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Nov 30, 2019
@lundibundi @addaleax
http2: make maximum tolerated rejected streams configurable
7d37bce 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
@lundibundi @targos
http2: replace direct array usage with struct for js_fields_
46cb0da 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
@lundibundi @targos
http2: allow to configure maximum tolerated invalid frames
3bed1fa 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
@lundibundi @targos
test: update and harden http2-reset-flood
518fd8f 
* use new maxSessionInvalidFrames to lower the needed frames
* slow down requests to generate less redundant after-session-close
  requests

PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
@lundibundi @targos
http2: make maximum tolerated rejected streams configurable
fb4f71b 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
@lundibundi @BethGriggs
http2: replace direct array usage with struct for js_fields_
7998fbb 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
@lundibundi @BethGriggs
http2: allow to configure maximum tolerated invalid frames
7b26606 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
@lundibundi @BethGriggs
test: update and harden http2-reset-flood
8601394 
* use new maxSessionInvalidFrames to lower the needed frames
* slow down requests to generate less redundant after-session-close
  requests

PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
@lundibundi @BethGriggs
http2: make maximum tolerated rejected streams configurable
a4ae272 
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ronag ronag mentioned this issue Mar 1, 2020
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discuss Issues opened for discussions and feedbacks. feature request Issues that request new features to be added to Node.js. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http2: make maximum invalid frames and maximum rejected streams configurable lundibundi/node
2 participants
@addaleax @lundibundi