FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: make maximum invalid frames and maximum rejected streams configurable by lundibundi · Pull Request #30534 · nodejs/node · GitHub

/ node Public

http2: make maximum invalid frames and maximum rejected streams configurable - #30534

Closed
lundibundi wants to merge 6 commits into
nodejs:masterfrom
lundibundi:http2-invalid-frames-i30505
ConversationCommits (6)ChecksFiles changed
Closed

http2: make maximum invalid frames and maximum rejected streams configurable#30534
lundibundi wants to merge 6 commits into
nodejs:masterfrom
lundibundi:http2-invalid-frames-i30505

Conversation

lundibundi commented Nov 18, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

http2: replace direct array usage with struct for js_fields_

http2: allow to configure maximum tolerated invalid frames

test: update and harden http2-reset-flood

  • use new maxSessionInvalidFrames to lower the needed frames
  • slow down requests to generate less redundant after-session-close
    requests

http2: make maximum tolerated rejected streams configurable

Closes: #30505
This should also decrease the flakiness of test-http2-reset-flood.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Also, do we have a dedicated test for argument validation of createServer? I'm not sure where to put them.

/cc @nodejs/http2

lundibundi requested a review from addaleax November 18, 2019 20:20
nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Nov 18, 2019

nodejs-github-bot commented Nov 18, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26715/

ZYSzys reviewed Nov 19, 2019
View reviewed changes
Comment thread doc/api/http2.md Outdated
Comment thread doc/api/http2.md Outdated

ZYSzys commented Nov 19, 2019

Copy link
Copy Markdown
Member

Also, do we have a dedicated test for argument validation of createServer? I'm not sure where to put them.

Seem like we actually don't have a dedicated test for argument validation of createServer, but we test things like this by adding a new test file called test-http2-too-many-*.js.

Maybe add a new test and name it as test-http2-too-many-invalid-frame.js is suitable here.

ZYSzys added the http2 Issues or PRs related to the http2 subsystem. label Nov 19, 2019
addaleax approved these changes Nov 19, 2019
View reviewed changes
Comment thread lib/internal/http2/core.js Outdated
addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 19, 2019
lundibundi force-pushed the http2-invalid-frames-i30505 branch from 53d5a16 to f132068 Compare November 19, 2019 21:56

lundibundi commented Nov 19, 2019

Copy link
Copy Markdown
Member Author

Added dedicated test for maxSessionInvalidFrames that should be more reliable.
Added simple options validation testing for createServer/createSecureServer.
Fixed missing changelog for maxSessionRejectedStreams option.

nodejs-github-bot commented Nov 19, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26765/

addaleax approved these changes Nov 19, 2019
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Still LGTM

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 19, 2019
lundibundi force-pushed the http2-invalid-frames-i30505 branch from f132068 to b9c679c Compare November 22, 2019 13:52

nodejs-github-bot commented Nov 22, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26847/

lundibundi commented Nov 22, 2019

Copy link
Copy Markdown
Member Author

(nothing changed, force pushed to resolve conflicts)

vsemozhetbyt reviewed Nov 23, 2019
View reviewed changes

vsemozhetbyt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Some doc nits.

Comment thread doc/api/http2.md Outdated
Comment thread doc/api/http2.md Outdated
Comment thread doc/api/http2.md Outdated
Comment thread doc/api/http2.md Outdated

lundibundi commented Nov 23, 2019

Copy link
Copy Markdown
Member Author

@vsemozhetbyt done, thanks.

nodejs-github-bot commented Nov 23, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26884/

nodejs-github-bot commented Nov 26, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27059/

nodejs-github-bot commented Nov 27, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27083/

lundibundi mentioned this pull request Nov 27, 2019
Closed
3 tasks
BridgeAR mentioned this pull request Dec 3, 2019
Merged
BridgeAR added a commit that referenced this pull request Dec 3, 2019
2019-12-03, Version 13.3.0 (Current)
8906c52 
Notable changes:

* http:
  * Make maximum header size configurable per-stream or per-server
    (Anna Henningsen) #30570
* http2:
  * Make maximum tolerated rejected streams configurable (Denys
    Otrishko) #30534
  * Allow to configure maximum tolerated invalid frames (Denys
    Otrishko) #30534
* wasi:
  * Introduce initial WASI support (cjihrig)
    #30258

PR-URL: #30774
BridgeAR added a commit that referenced this pull request Dec 3, 2019
2019-12-03, Version 13.3.0 (Current)
f5fe384 
Notable changes:

* fs:
  * Reworked experimental recursive `rmdir()`  (cjihrig)
    #30644
    * The `maxBusyTries` option is renamed to `maxRetries`, and its
      default is set to 0. The `emfileWait` option has been removed,
      and `EMFILE` errors use the same retry logic as other errors.
      The `retryDelay` option is now supported. `ENFILE` errors are
      now retried.
* http:
  * Make maximum header size configurable per-stream or per-server
    (Anna Henningsen) #30570
* http2:
  * Make maximum tolerated rejected streams configurable (Denys
    Otrishko) #30534
  * Allow to configure maximum tolerated invalid frames (Denys
    Otrishko) #30534
* wasi:
  * Introduce initial WASI support (cjihrig)
    #30258

PR-URL: #30774
BridgeAR added a commit that referenced this pull request Dec 3, 2019
2019-12-03, Version 13.3.0 (Current)
4091ea9 
Notable changes:

* fs:
  * Reworked experimental recursive `rmdir()`  (cjihrig)
    #30644
    * The `maxBusyTries` option is renamed to `maxRetries`, and its
      default is set to 0. The `emfileWait` option has been removed,
      and `EMFILE` errors use the same retry logic as other errors.
      The `retryDelay` option is now supported. `ENFILE` errors are
      now retried.
* http:
  * Make maximum header size configurable per-stream or per-server
    (Anna Henningsen) #30570
* http2:
  * Make maximum tolerated rejected streams configurable (Denys
    Otrishko) #30534
  * Allow to configure maximum tolerated invalid frames (Denys
    Otrishko) #30534
* wasi:
  * Introduce initial WASI support (cjihrig)
    #30258

PR-URL: #30774
targos pushed a commit that referenced this pull request Jan 13, 2020
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 pull request Jan 13, 2020
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 pull request Jan 13, 2020
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 pull request Jan 13, 2020
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>
targos mentioned this pull request Jan 15, 2020
Closed
sxa mentioned this pull request Jan 21, 2020
Closed
2 tasks
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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 pull request Feb 6, 2020
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 pull request Feb 6, 2020
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 pull request Feb 6, 2020
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>
MylesBorins mentioned this pull request Feb 8, 2020
Merged
puzpuzpuz mentioned this pull request Mar 8, 2020
Closed
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ZYSzys ZYSzys left review comments
jasnell jasnell approved these changes
addaleax addaleax approved these changes
+2 more reviewers
vsemozhetbyt vsemozhetbyt left review comments
devnexen devnexen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make Http2Session maximum invalid frame count configurable

7 participants

Back | FazBrowse Home | New Git URL