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.

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

close event is called before finish #327

Open
Dagrut opened this issue Sep 21, 2017 · 3 comments
Open

close event is called before finish #327

Dagrut opened this issue Sep 21, 2017 · 3 comments

Comments

@Dagrut
Copy link

Dagrut commented Sep 21, 2017

I had a problem using https://github.com/binarysec/gate, so I made a shorter version of this bug below. The bug is that the close event of the response is called even if the connection ended correctly, and so the finish event is never called (close means that there were still data to be sent to the client, but the connection closed before the .end()). It seems to only happen in HTTP/2, not HTTP/1.1. Also,
this bug happens with bot the latest v4 and v6 of nodejs (debian repo version). It may be a nodejs bug or a spdy bug, so I thought I would ask it here first. To reproduce it, here are the steps :

Test case

This test focuses on request forwarding, but you may make a shorter version too.

Create ssl certs

openssl req -x509 -nodes -days 365 -newkey rsa:2048 -keyout server.key -out server.crt

Install dependancies

npm install spdy

Set host

Add this to your system hosts file :

127.0.0.1 www.materiel.net

server.js

Put this in a file called server.js :

const http = require('http');
const https = require('https');
const spdy = require('spdy');
const crypto = require('crypto');
const fs = require("fs");
const url = require('url');

var srvOptions = {
	key: fs.readFileSync('server.key').toString('utf8'),
	cert: fs.readFileSync('server.crt').toString('utf8')
};

var onReq = function (req, res) {
	console.log('Received :', req.url);

	var options = url.parse(req.url);
	options.headers = req.headers;
	options.method = req.method;
	options.host = '87.98.146.101';

	/* Test using static response */
	res.writeHeader(200, { 'Content-Length': 6 });
	res.write('Hello!');
	res.end();

	/* Test using a pipe */
	// var connector = https.request(options, function(serverResponse) {
	// 	res.writeHeader(serverResponse.statusCode, serverResponse.headers);
	// 	serverResponse.pipe(res);
	// });
	// req.pipe(connector);

	res.on('close', function() {
		console.log('Had close... :-(')
	});

	res.on('finish', function() {
		console.log('Had finish! :-)')
	});
};

var server = spdy.createServer(srvOptions, onReq);
server.listen(443);

Run server

sudo node server.js

Test

Go to https://www.materiel.net/ (using firefox in my case, but any browser supporting HTTP/2 should do).

@ivan-kleshnin
Copy link

ivan-kleshnin commented Aug 17, 2018

@Dagrut did you manage to find a fix or a workaround? I also think it's a Spdy bug.

@wsjwa
Copy link

wsjwa commented Apr 5, 2019

Can comfirm it. Is there s solution?

mykiimike added a commit to mykiimike/node-spdy that referenced this issue May 2, 2019
@9still
Copy link

9still commented Apr 28, 2020

For anyone still looking for a solution, this (adapted slightly from the proposal above) seems to work without requiring a fork.

Thanks @mykiimike!

  server.on('request', (req, res) => {
    res.spdyStream.once('finish', () => res.emit('finish'));
  });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants