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

deps: Update nghttp2 to v1.45.1 #40206

Closed
wants to merge 1 commit into from
Closed

deps: Update nghttp2 to v1.45.1 #40206

wants to merge 1 commit into from

Conversation

thunder-coding
Copy link
Contributor

Fixes #39449

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Sep 24, 2021
@targos
Copy link
Member

targos commented Sep 24, 2021

@nodejs/http2

@thunder-coding thunder-coding marked this pull request as draft September 24, 2021 11:29
@thunder-coding
Copy link
Contributor Author

thunder-coding commented Sep 24, 2021 •
edited
Loading

Seems like I got why the tests are failing 🤔,

Since nghttp2/nghttp2#1613, nghttp2 checks for allowed characters in :method and :path.

Looking at the tests:

https://github.com/thunder-coding/node/blob/2058eba38a715e1059a1bcf9e68ac2a04f25585b/test/parallel/test-http2-methods.js#L11

We have undefined as one of the methods, which is probably causing the issue

And for 2nd test

https://github.com/thunder-coding/node/blob/2058eba38a715e1059a1bcf9e68ac2a04f25585b/test/parallel/test-http2-multiplex.js#L32

We have an extra space which is even included in unallowed characters https://github.com/thunder-coding/node/blob/2058eba38a715e1059a1bcf9e68ac2a04f25585b/deps/nghttp2/lib/nghttp2_helper.c#L511

@mcollina
Copy link
Member

Could you fix the tests?

@thunder-coding
Copy link
Contributor Author

thunder-coding commented Sep 24, 2021 •
edited
Loading

Could you fix the tests?

I didn't run the tests on GitPod (currently where I am building this PR on), but it seems like I need to test on GitPod itself as the CI refused to build due to force push. I will update as soon as the test completes

@thunder-coding thunder-coding marked this pull request as ready for review September 24, 2021 16:32
@thunder-coding
Copy link
Contributor Author

I was able to run the tests on GitPod, should pass on CI too.

Ready for review process

mcollina
mcollina approved these changes Sep 24, 2021
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 24, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@thunder-coding
Copy link
Contributor Author

thunder-coding commented Sep 25, 2021 •
edited
Loading

The current build errors for node-test-commit-linuxone seems to be related to nodejs/build#1847 which was closed due to inactivity. The issue is still persistent and needs to reopened and fixed. nodejs/build#2563. Same errors occured for FreeBSD

@nodejs-github-bot
Copy link
Collaborator

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

@thunder-coding
Copy link
Contributor Author

The earlier test error for LinuxONE was due to some falky tests as said in the PR mentioned in the issue I mentioned in the earlier comments. And current errors are due to timeouts in tests (https://ci.nodejs.org/job/node-test-commit-linux/nodes=centos7-64-gcc8/43075/testReport/(root)/test/sequential_test_net_connect_econnrefused/), both can be safely skipped 😅

@nodejs-github-bot
Copy link
Collaborator

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

@mcollina mcollina added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Sep 25, 2021
@mcollina
Copy link
Member

everything is in order, we'll land it after 48 hours have passed

This was referenced Sep 26, 2021
Open
Open
@gengjiawen gengjiawen added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 27, 2021
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 27, 2021
@gengjiawen gengjiawen added the notable-change PRs with changes that should be highlighted in changelogs. label Sep 27, 2021
@thunder-coding thunder-coding deleted the nghttp2-v1.45.1 branch September 27, 2021 04:42
This was referenced Sep 28, 2021
Open
Open
Open
Open
Open
Open
Open
targos pushed a commit that referenced this pull request Oct 4, 2021
@thunder-coding @targos
deps: update nghttp2 to v1.45.1
685c7d4 
PR-URL: #40206
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
danielleadams added a commit that referenced this pull request Oct 4, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
b57aea9 
Notable changes:

* crypto:
  * update root certificates (Richard Lau) #40280
* tools:
  * update certdata.txt (Richard Lau) #40280
* deps:
  * update `nghttp2` to v1.45.1 (thunder-coding) #40206
* deps:
  * restore minimum ICU version to 68 (Michaël Zasso) #39470
danielleadams added a commit that referenced this pull request Oct 4, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
68d046d 
Notable changes:

* crypto:
  * update root certificates (Richard Lau) #40280
* tools:
  * update certdata.txt (Richard Lau) #40280
* deps:
  * update `nghttp2` to v1.45.1 (thunder-coding) #40206
* deps:
  * restore minimum ICU version to 68 (Michaël Zasso) #39470
@danielleadams danielleadams mentioned this pull request Oct 4, 2021
Merged
danielleadams added a commit that referenced this pull request Oct 4, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
b717d16 
Notable changes:

* crypto:
  * update root certificates (Richard Lau) #40280
* tools:
  * update certdata.txt (Richard Lau) #40280
* deps:
  * update `nghttp2` to v1.45.1 (thunder-coding) #40206
* deps:
  * restore minimum ICU version to 68 (Michaël Zasso) #39470

PR-URL: #40319
@github-actions github-actions bot mentioned this pull request Oct 5, 2021
Open
21 tasks
danielleadams added a commit that referenced this pull request Oct 5, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
6257926 
Notable changes:

* crypto:
  * update root certificates (Richard Lau) #40280
* tools:
  * update certdata.txt (Richard Lau) #40280
* deps:
  * update `nghttp2` to v1.45.1 (thunder-coding) #40206
  * restore minimum ICU version to 68 (Michaël Zasso) #39470

PR-URL: #40319
danielleadams added a commit that referenced this pull request Oct 5, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
0374c66 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * tools:
      * update certdata.txt (Richard Lau) #40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * restore minimum ICU version to 68 (Michaël Zasso) #39470

    PR-URL: #40319
danielleadams added a commit that referenced this pull request Oct 5, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
94667a7 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * tools:
      * update certdata.txt (Richard Lau) #40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * restore minimum ICU version to 68 (Michaël Zasso) #39470

    PR-URL: #40319
danielleadams added a commit that referenced this pull request Oct 5, 2021
@danielleadams
2021-10-05, Version 16.11.0 (Current)
1c47b70 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * tools:
      * update certdata.txt (Richard Lau) #40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * update V8 to 9.4.146.19 (Michaël Zasso) #40285

    PR-URL: #40319
danielleadams added a commit that referenced this pull request Oct 6, 2021
@danielleadams
2021-10-06, Version 16.11.0 (Current)
8bb231b 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * tools:
      * update certdata.txt (Richard Lau) #40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * update V8 to 9.4.146.19 (Michaël Zasso) #40285

    PR-URL: #40319
danielleadams added a commit to danielleadams/node that referenced this pull request Oct 7, 2021
@danielleadams
2021-10-06, Version 16.11.0 (Current)
0b53a6f 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) nodejs#40280
    * tools:
      * update certdata.txt (Richard Lau) nodejs#40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) nodejs#40206
      * update V8 to 9.4.146.19 (Michaël Zasso) nodejs#40285

    PR-URL: nodejs#40319
danielleadams added a commit to danielleadams/node that referenced this pull request Oct 7, 2021
@danielleadams
2021-10-07, Version 16.11.0 (Current)
91c95b4 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) nodejs#40280
    * tools:
      * update certdata.txt (Richard Lau) nodejs#40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) nodejs#40206
      * update V8 to 9.4.146.19 (Michaël Zasso) nodejs#40285

    PR-URL: nodejs#40319
danielleadams added a commit to danielleadams/node that referenced this pull request Oct 7, 2021
@danielleadams
2021-10-07, Version 16.11.0 (Current)
a728f34 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) nodejs#40280
    * tools:
      * update certdata.txt (Richard Lau) nodejs#40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) nodejs#40206
      * update V8 to 9.4.146.19 (Michaël Zasso) nodejs#40285

    PR-URL: nodejs#40319
danielleadams added a commit that referenced this pull request Oct 7, 2021
@danielleadams
2021-10-07, Version 16.11.0 (Current)
7521628 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * tools:
      * update certdata.txt (Richard Lau) #40280
    * deps:
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * update V8 to 9.4.146.19 (Michaël Zasso) #40285

    PR-URL: #40319
danielleadams added a commit to danielleadams/node that referenced this pull request Oct 8, 2021
@danielleadams
2021-10-07, Version 16.11.0 (Current)
80c5b9d 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) nodejs#40280
    * deps:
      * upgrade npm to 8.0.0 (npm team) nodejs#40369
      * update `nghttp2` to v1.45.1 (thunder-coding) nodejs#40206
      * update V8 to 9.4.146.19 (Michaël Zasso) nodejs#40285
    * tools:
      * update certdata.txt (Richard Lau) nodejs#40280

    PR-URL: nodejs#40319
danielleadams added a commit that referenced this pull request Oct 8, 2021
@danielleadams
2021-10-08, Version 16.11.0 (Current)
f1ae6fb 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * deps:
      * upgrade npm to 8.0.0 (npm team) #40369
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * update V8 to 9.4.146.19 (Michaël Zasso) #40285
    * tools:
      * update certdata.txt (Richard Lau) #40280

    PR-URL: #40319
danielleadams added a commit that referenced this pull request Oct 8, 2021
@danielleadams
2021-10-08, Version 16.11.0 (Current)
879ff77 
    Notable changes:

    * crypto:
      * update root certificates (Richard Lau) #40280
    * deps:
      * upgrade npm to 8.0.0 (npm team) #40369
      * update `nghttp2` to v1.45.1 (thunder-coding) #40206
      * update V8 to 9.4.146.19 (Michaël Zasso) #40285
    * tools:
      * update certdata.txt (Richard Lau) #40280

    PR-URL: #40319
codebytere added a commit to electron/electron that referenced this pull request Oct 11, 2021
@codebytere
deps: Update nghttp2 to v1.45.1
1712db8 
nodejs/node#40206
codebytere added a commit to electron/electron that referenced this pull request Oct 11, 2021
@codebytere
deps: Update nghttp2 to v1.45.1
0182c3a 
nodejs/node#40206
codebytere added a commit to electron/electron that referenced this pull request Oct 13, 2021
@codebytere
deps: Update nghttp2 to v1.45.1
9c88d3e 
nodejs/node#40206
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gengjiawen gengjiawen gengjiawen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. http2 Issues or PRs related to the http2 subsystem. notable-change PRs with changes that should be highlighted in changelogs. 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.

update nghttp2 to v1.45.1
7 participants
@thunder-coding @targos @mcollina @nodejs-github-bot @jasnell @cjihrig @gengjiawen