spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

MAX_CONCURRENT is send as 0, when maxStreams is Infinity #180

Closed AndreasMadsen closed 10 years ago

AndreasMadsen commented 10 years ago

Hi @indutny

I have been informed that 139c432425700803a891b1d7ea329028d59c93db causes some issues. Discussion here: https://bugzilla.mozilla.org/show_bug.cgi?id=1072478

indutny commented 10 years ago

Oi, this is totally correct! Sorry for missing this during review. Does this patch fix the problem for you?

diff --git a/lib/spdy/protocol/framer.js b/lib/spdy/protocol/framer.js
index 256d0c9..ef39451 100644
--- a/lib/spdy/protocol/framer.js
+++ b/lib/spdy/protocol/framer.js
@@ -402,16 +402,13 @@ Framer.prototype.settingsFrame = function settingsFrame(options, callback) {

   if (!(settings = Framer.settingsCache[key])) {
     var params = [];
-    if (this.version === 2) {
-      params.push({
-        key: constants.settings.SETTINGS_MAX_CONCURRENT_STREAMS,
-        value: options.maxStreams
-      });
-    } else {
+    if (isFinite(options.maxStreams)) {
       params.push({
         key: constants.settings.SETTINGS_MAX_CONCURRENT_STREAMS,
         value: options.maxStreams
       });
+    }
+    if (this.version > 2) {
       params.push({
         key: constants.settings.SETTINGS_INITIAL_WINDOW_SIZE,
         value: options.windowSize
AndreasMadsen commented 10 years ago

Pre 139c432 there was issues too, so it won't solve the entire problem. But it seams like the correct fix, though I don't known the SPDY protocol that well yet.