koajs / koa

Expressive middleware for node.js using ES2017 async functions
https://koajs.com
MIT License
35.22k stars 3.23k forks source link

bug report! If koa and http2server are used together, the response with method "head" request will never end! #1547

Open masx200 opened 3 years ago

masx200 commented 3 years ago

bug report! If koa and http2server are used together, the response with method "head" request will never end!

import http2 from "http2";
import Koa from "koa";
import fs from "fs";
import { fileURLToPath } from "url";
import { join, dirname } from "path";
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
const app = new Koa();
const selfSignedKey = join(__dirname, "./certs/self-signed.key.pem");
const selfSignedCert = join(__dirname, "./certs/self-signed.cert.pem");
app.use((ctx) => {
    ctx.body = "Hello Koa";
});
const config = {
    sslKey: fs.readFileSync(selfSignedKey).toString(),
    sslCert: fs.readFileSync(selfSignedCert).toString(),
};
const serverHandler = app.callback();
const httpsopt = { cert: config.sslCert, key: config.sslKey };
const server = http2.createSecureServer(httpsopt, serverHandler);

server.on("listening", () => {
    console.log(`Server listening on ` + JSON.stringify(server.address()));
});
server.listen(3000);
{
    "type": "module",
    "dependencies": {
        "koa": "^2.13.1"
    }
}
node index.js

Initiate a "head" request in the browser.

Request URL: https://localhost:3000/
Referrer Policy: strict-origin-when-cross-origin
:authority: localhost:3000
:method: HEAD
:path: /
:scheme: https
accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
accept-encoding: gzip, deflate, br
accept-language: zh-CN,zh;q=0.9
referer: https://localhost:3000/
sec-ch-ua: " Not;A Brand";v="99", "Google Chrome";v="91", "Chromium";v="91"
sec-ch-ua-mobile: ?0
sec-fetch-dest: empty
sec-fetch-mode: cors
sec-fetch-site: same-origin
upgrade-insecure-requests: 1
user-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.106 Safari/537.36

the response with method "head" request will never end!

masx200 commented 3 years ago

My solution is to modify it as follows and everything works fine!


import http2 from "http2";
import Koa from "koa";
import fs from "fs";
import { fileURLToPath } from "url";
import { join, dirname } from "path";
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
const app = new Koa();
const selfSignedKey = join(__dirname, "./certs/self-signed.key.pem");
const selfSignedCert = join(__dirname, "./certs/self-signed.cert.pem");

//
app.use(async (ctx, next) => {
    await next();
    if (ctx.method === "HEAD") {
        ctx.res.end();
    }
    return;
});
//
app.use((ctx) => {
    ctx.body = "Hello Koa";
});
const config = {
    sslKey: fs.readFileSync(selfSignedKey).toString(),
    sslCert: fs.readFileSync(selfSignedCert).toString(),
};
const serverHandler = app.callback();
const httpsopt = { cert: config.sslCert, key: config.sslKey };
const server = http2.createSecureServer(httpsopt, serverHandler);

server.on("listening", () => {
    console.log(`Server listening on ` + JSON.stringify(server.address()));
});
server.listen(3000);
masx200 commented 3 years ago

https://github.com/koajs/koa/blob/698ce0afbfac6480400625729a4b8fc4b4203fdc/lib/response.js#L535

I found the problem,

if it is the'http2' server and the method is 'head', the "socket.writable" is "false".

If it is the'http1' server, the "socket.writable" is "true".

masx200 commented 3 years ago

https://github.com/koajs/koa/pull/1548

Azlond commented 3 years ago

The same error also occurs when the client closes the connection before the server had a chance to respond, causing the socket to no longer be writable. This can cause pending connections, because ctx.res.end(); will never be called.

All other return-statements in the respond()-Function are calling res.end() (except for the koa-bypass), which makes me believe it should be called in that line as well - we have fixed it this way in our fork and haven't had any problems with pending connections since.

Instead of checking for

ctx.method === "HEAD"

as was done in #1548 I'd suggest replacing this line with the following:

if (!ctx.writable) {
    return res.end();
}

Would you like a separate Pull request for this @jonathanong ?

masx200 commented 1 year ago

Why has this problem not been solved after two years?