This repository was archived by the owner on Jul 6, 2018. It is now read-only.
http2: fix on windows#156
Closed
jasnell wants to merge 64 commits intonodejs:masterfrom
Closed
Conversation
PR-URL: nodejs#3 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Allows TLS renegotiation to be disabled per `TLSSocket` instance. Per HTTP/2, TLS renegotiation is forbidden after the initial connection prefix is exchanged.
Squashed rollup of all the progress to this point
PR-URL: nodejs#58 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#65 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Just reading through the code and noticed that the stream event can take headers and flags. Just adding them for readability and asserting the values in the headers and flags. PR-URL: nodejs#71 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#78 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#64 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#64 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#83 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently when compiling the following error is displayed:
In file included from
/out/Release/obj/gen/node_javascript.cc:6:
../src/env-inl.h:201:7: warning: field 'fs_stats_field_array_' will be
initialized
after field 'http2_socket_buffer_' [-Wreorder]
fs_stats_field_array_(nullptr),
^
In file included from
/out/Debug/obj/gen/node_javascript.cc:6:
../src/env-inl.h:201:7: warning: field 'fs_stats_field_array_' will be
initialized
after field 'http2_socket_buffer_' [-Wreorder]
fs_stats_field_array_(nullptr),
^
1 warning generated.
This commit changes the order so that the members appear in the same
order in the initializer list.
PR-URL: nodejs#85
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This might be an incorrect change but the naming looks to be a little different for these files and wanted to bring it up. Feel free to close if this is indeed correct. PR-URL: nodejs#74 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#75 Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#81 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Currently there is a compiler warning which only showed up after applying the fix for nodejs#85: In file included from ../src/node-http2-core.cc:1: ../src/node-http2-core.h:242:1: warning: 'Nghttp2Stream' defined as a struct here but previously declared as a class [-Wmismatched-tags] struct Nghttp2Stream : public std::enable_shared_from_this<Nghttp2Stream> { ^ ../src/node-http2-core.h:19:1: note: did you mean struct here? class Nghttp2Stream; ^~~~~ struct In file included from ../src/node-http2-core.cc:1: This commit attempts to fix this warning. PR-URL: nodejs#86 Ref: nodejs#85 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#87 Reviewed-By: James M Snell <jasnell@gmail.com>
Currently the following compiler warning is displayed:
/out/Debug/obj.target/node/src/node_http2.o
../src/node_http2.cc
../src/node_http2.cc:../src/node_http2.cc838::8383::3 : warning:
warning: ignoring return value of function declared with
warn_unused_result ignoringattribute return[-Wunused-result]
value
of function declared with
warn_unused_result attribute [-Wunused-result]
holder->SetPrototype(context, v8::Null(isolate));
^~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
This commit adds a check to avoid this warning.
PR-URL: nodejs#89
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds a test for trailer headers. PR-URL: nodejs#90 Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#91 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I cannot find any usage of these macros and it build and all tests pass locally without them. PR-URL: nodejs#93 Reviewed-By: James M Snell <jasnell@gmail.com>
Remove unnecessary includes. Remove unnecesary `using` statements. PR-URL: nodejs#92 Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#96 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
PR-URL: nodejs#95 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-Url: nodejs#47 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Currently nghttp2_free_headers_list uses the cb pointer but does not check it for null first like other functions do. Wanted to bring this up just in case it was simply overlooked. PR-Url: nodejs#100 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#127 Reviewed-By: James M Snell <jasnell@gmail.com>
These are forbidden by HTTP/2. PR-URL: nodejs#128 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#129 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#120 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Support the socket/connection getter like require('http') does.
PR-URL: nodejs#130
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Support the upgrade path from https to http2. PR-URL: nodejs#125 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* rename functions in docs * add warning against using socket directly * document paddingStrategy / selectPadding * fill in missing details and examples PR-URL: nodejs#132 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#134 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Fixes: nodejs#135 PR-URL: nodejs#137 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* tighten up the Http2Stream/Http2Session lifecycle, destroy, and error handling. Some simplification, and more new tests * Eliminate queuing of internal callbacks. Queuing these to fire on the next event loop tick was throwing off timing. Now the js callbacks are called directly as they happen. This will require a bit more finesse on the javascript side (to ensure appropiate timing of destroy/shutdown actions) but that is handled from within the core api impl so users should not be affected. * fix debug output with nghttp2 PR-URL: nodejs#138 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Using `./configure --debug-http2` will enable verbose debug statements from node.js, Using `./configure --debug-nghttp2` will enable verbose debug statements from nghttp2. The two can be used together PR-URL: nodejs#138 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* Follow the model of the existing http code and pass the headers in to js as an array. Building the array on the native side then converting it to an object on the js side is generally faster than building the object on the native side. This also allows us to eliminate some duplicated checks. * Fill in additional doc details * Work on timeouts PR-URL: nodejs#148 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Emit `unknownProtocol` event or silently destroy the socket PR-URL: nodejs#144 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
9b6b067 broke the test PR-URL: nodejs#149 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
* unref the uv_prep_t handle * remove unused code * improve settings handling: add `'localSettings'` and `'remoteSettings'` events, improve handling of settings acknowledgements * improve options handling * ensure that max concurrent streams works correctly * update documentation * add return values in docs * throw on pushStream if push not enabled PR-URL: nodejs#150 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#151 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* send only when necessary * fix memory leak * additional perf/memory improvements * avoid use of deprecated V8 APIs * use ASCII header values (both for spec compliance and to avoid unnecessary copying / allocation PR-URL: nodejs#155 Fixes: nodejs#153 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Member
Author
|
|
Member
Author
|
oh sorry... lol missed that you already had!! it didn't show up until the page refreshed lol |
jasnell
added a commit
that referenced
this pull request
Jun 22, 2017
PR-URL: #156 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
jasnell
added a commit
to jasnell/http2-1
that referenced
this pull request
Jun 22, 2017
PR-URL: nodejs#156 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Member
Author
|
This landed |
jasnell
added a commit
to jasnell/http2-1
that referenced
this pull request
Jul 10, 2017
PR-URL: nodejs#156 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
jasnell
added a commit
to jasnell/http2-1
that referenced
this pull request
Jul 14, 2017
PR-URL: nodejs#156 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@sebdeckers @mcollina ... this ought to fix the busy loop and it should be working on windows now.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
http2