spdy-http2 / node-spdy

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

Gzip compression error #233

Closed lewispham closed 8 years ago

lewispham commented 8 years ago

I've tried testing spdy with gzipped content. Unfortunately, instead of ungzipping it, Chrome throws the error net::ERR_SPDY_COMPRESSION_ERROR.

spdy.createServer(options, function(req, res) {
  var stream = res.push('/main.js', {
    request: {
      accept: '*/*'
    },
    response: {
      'content-type': 'application/javascript',
      'content-encoding' : 'gzip',
    }
  });
  stream.on('error', function() {
  });
  stream.end(zlib.gzipSync('alert("hello from push stream!");'));

  res.end('<script src="/main.js"></script>');
}).listen(3000);
akyoto commented 8 years ago

Same here :+1:

Also: If the push stream itself does not use gzip but the actual response does I get the error as well.

indutny commented 8 years ago

What version of node-spdy are you using? May I ask you to run the script with DEBUG="spdy*" and gist the results?

I have just tried it in chrome and it appears to be working with the latest spdy.

lewispham commented 8 years ago

@indutny I'm using node-spdy v2.1.0 (checked via npm list). Could you give me a detailed instruction? I'm not really familiar with gist.

akyoto commented 8 years ago

spdy 2.1.0 output (single get request): https://gist.github.com/blitzprog/374be5e587b6a595ede2

Relevant test code:

if(response.push && request.url === '/') {
    let push = response.push('/images/backgrounds/keyboard.jpg', {
        request: {
            accept: '*/*'
        },
        response: {
            'Content-Type': 'image/jpeg'
        }
    })
    push.end(require('fs').readFileSync('/home/eduard/projects/blitzprog.org/images/backgrounds/keyboard.jpg'))
}

Note that I do not gzip the push itself. However the actual HTML response is gzipped. If I do gzip the push the error stays.

You need to emulate a higher network latency to even see the error message on localhost because it quickly disappears.

Chrome version is 48.0.2560.0 dev (64-bit) on Ubuntu 15.10 however I get the exact same error on v46 (stable).

indutny commented 8 years ago

@blitzprog is this gist from the error-ed case? I can't see anything abnormal or any kind of error in it.

akyoto commented 8 years ago

@indutny: It is, however the error appears in Chrome only and I don't know whose mistake it is. I looked up the error code (ERR_SPDY_COMPRESSION_ERROR) in Google and the threads were saying that this is an nginx issue (read: mistake of the webserver). Therefore I assume(d) it could be a bug in node-spdy.

I don't really know whose fault it is. I just want it to work :/

Do you need more details?

indutny commented 8 years ago

@blitzprog ok, if this is happening on the Chrome side - let's try to figure it out from that side! :)

May I ask you to open chrome://net-internals/ before the test, start spdy server, make a faulty request that results in compression error, and click export at chrome://net-internals/ tab?

If you want to send this data using private channel - my email is public on my github profile.

Thank you very much, Fedor.

akyoto commented 8 years ago

@indutny Sent you the exported data via mail, let me know if you need anything else.

indutny commented 8 years ago

Thank you, this is a relevant error from chrome's log:

t=746 [st=743]    HTTP2_SESSION_CLOSE
                  --> description = "Framer error: 5 (DECOMPRESS_FAILURE)."
                  --> net_error = -363 (ERR_SPDY_COMPRESSION_ERROR)

Going to dig into it.

indutny commented 8 years ago

Wow, 14 days. Sorry for delay. Still looking into it.

indutny commented 8 years ago

I think it should be fixed wit the latest spdy-transport@2.0.3 . Please run npm update and give it another try. Sorry again for the long wait!

jadbox commented 8 years ago

Can we get an example of spdy+gzip? (non-push)

indutny commented 8 years ago

@jadbox I think the code here is quite relevant to your question, but anyway I don't think that this question really belongs to this issue. Feel free to open another issue if you want to discuss this, though!

lewispham commented 8 years ago

I still got a gzip compression error (on Chrome) with the sample code taken from issue #237 . Despite the Content-Encoding: gzip header is already added, it seems that gizpped files transfered via PUSH_PROMISE are not ungzipped on Chrome. Here is the content of main.js. gzip-error Similar result with main.css. gzip-error2

indutny commented 8 years ago

@tresdin asking just in case, you are piping files straight to the push stream. Are the files themselves compressed with gzip?

lewispham commented 8 years ago

@indutny

you are piping files straight to the push stream

Yes. You can see it from the sample code.

Are the files themselves compressed with gzip?

Yes. These files are compressed by the built in node zlib gzip compression

lewispham commented 8 years ago

@indutny Should I open another issue for this bug?

indutny commented 8 years ago

@tresdin heya! Sorry for delaying. Going to look at this right now.

indutny commented 8 years ago

Sorry, but it looks like an issue in your code. First of all, you are using deflateSync and then advertising gzip compression. You should consider using deflate instead, or compress it according to the header.

The second thing is the way headers are pushed to the browser. It should be something like:

var stream = res.push({
  response: {
    'Content-Encoding': 'deflate'
  }
});

Hope this helps!

lewispham commented 8 years ago

@indutny It doesn't help me. I'm using zlib.gzip for compressions. And the sample code is actually taken from #237 .

indutny commented 8 years ago

@tresdin I'm afraid that that code is broken. May I ask you to apply suggest changes, and repost the code sample here?

lewispham commented 8 years ago

@indutny Retested with this sample code. And browser produced the same error.

spdy.createServer(options, function(req, res) {
   // main.js and main.css were recompressed with `zlib.deflate`
    ['/app/main.js','/app/main.css','/app/favicon-32x32.png'].forEach(function(url){
        console.log('fetching',url);
      var stream = res.push(url, {
          'content-type': `${MIME[url.substr(url.lastIndexOf('.')+1)]};charset=utf-8`,
          'content-encoding' : 'deflate',
        });
      stream.on('error', function(er) {
          //throw er;
      });
      var fstream = fs.createReadStream('/home'+url);
      fstream.pipe(stream);
    });

  res.end(`
    <!DOCTYPE html>
    <html>
        <head>
            <script src="/app/main.js" async></script>
            <link rel="stylesheet" href="/app/main.css" async>
            <link rel="icon" type="image/png" href="/app/favicon-32x32.png" sizes="32x32" async>
            <meta name="viewport" content="width=device-width, initial-scale=1">
        </head>
        <body></body>
    </html>`
  );
indutny commented 8 years ago

You need to put headers in a following way:

res.push(url, {
  response: {
    // your headers
  }
});

It should work after this change.

lewispham commented 8 years ago

@indutny You're right. It works now. I forgot that change. Thank you.