docs: add contribution note for #65674

contributions/65674.md

@@ -0,0 +1,232 @@
+---
+pr-url: https://github.com/nodejs/node/pull/65674
+---
+
+# http: prevent reuse after incomplete request destruction
+
+## 문제 내용
+
+HTTP server에서 `IncomingMessage`를 async iterator로 읽다가 request body를 끝까지 소비하지 않은 상태에서 iteration이 종료되면, request stream은 destroy되지만 underlying socket은 살아 있을 수 있었다.
+
+이 상태에서 keep-alive connection이 다음 request에 재사용되면, 아직 남아 있는 request body 데이터가 영향을 주어 이후 request가 stall될 수 있는 문제가 있었다.
+
+관련 Issue: #49429
+
+## 원인 분석
+
+문제를 재현하면서 request와 socket 상태를 확인했다.
+
+request는 destroy된 상태였지만 body는 완전히 소비되지 않았고, socket은 여전히 살아 있는 경우가 있었다.
+
+```text
+req.destroyed = true
+req.complete = false
+req.readableEnded = false
+
+socket.destroyed = false
+```
+
+즉 다음과 같은 상태였다.
+
+```text
+Request stream 종료
+→ Request body는 완전히 소비되지 않음
+→ Socket은 살아 있음
+→ Keep-Alive connection으로 재사용 가능
+```
+
+이 때문에 불완전한 request가 남아 있는 connection이 다음 request에 다시 사용될 수 있었다.
+
+## 초기 해결 방향
+
+처음에는 문제를 Stream destroy 경로에서 해결하려고 했다.
+
+request가 완전히 소비되지 않은 상태에서 destroy될 경우, 해당 socket을 다음 request에서 재사용하지 못하도록 처리하는 방향으로 수정했다.
+
+하지만 PR 리뷰에서 `ronag`에게 다음과 같은 피드백을 받았다.
+
+```text
+The socket should not be re-used if destroyed without being fully consumed.
+```
+
+이 리뷰를 통해 단순히 Stream이 destroy되는 것보다, 불완전한 HTTP request가 끝난 뒤 connection 자체를 재사용해도 되는지가 핵심이라는 점을 다시 확인했다.
+
+## 해결 방향 수정
+
+이후 수정 방향을 다음과 같이 정리했다.
+
+```text
+Request body가 완전히 소비되지 않음
+→ Request destroy
+→ 해당 HTTP connection은 재사용하지 않음
+```
+
+이때 response header가 이미 전송되었는지 여부에 따라 두 가지 경우로 나누었다.
+
+### Response Header가 아직 전송되지 않은 경우
+
+아직 header가 전송되지 않았다면 현재 response는 정상적으로 완료할 수 있다.
+
+대신 keep-alive를 비활성화한다.
+
+```js
+response.shouldKeepAlive = false;
+response._last = true;
+```
+
+이 경우 클라이언트에는 `Connection: close`가 전달되고, response가 끝난 뒤 socket은 재사용되지 않는다.
+
+### Response Header가 이미 전송된 경우
+
+header가 이미 전송된 뒤에는 keep-alive 여부가 이미 결정된 상태일 수 있다.
+
+이 경우 response를 destroy하여 connection 자체가 재사용되지 않도록 한다.
+
+```js
+response.destroy();
+```
+
+## 리뷰를 통해 수정 위치 변경
+
+이후 `mcollina`에게 다음과 같은 피드백을 받았다.
+
+```text
+My understanding is that the fix for this needs to happen in http, not in streams.
+```
+
+처음에는 request가 Stream이기 때문에 Stream implementation에서 수정하려고 했다.
+
+하지만 실제 문제는 generic Stream 동작이 아니라 HTTP keep-alive connection의 재사용 정책과 관련된 문제였다.
+
+그래서 수정 위치를 Stream implementation에서 HTTP layer로 이동했다.
+
+최종적으로 `lib/_http_incoming.js`의 `IncomingMessage._destroy()`에서 처리하도록 변경했다.
+
+핵심 로직은 다음과 같다.
+
+```js
+IncomingMessage.prototype._destroy = function _destroy(err, cb) {
+  if (this.socket === null) {
+    const response = this.client?._httpMessage;
+
+    if (response?.req === this) {
+      if (response.headersSent) {
+        response.destroy();
+      } else {
+        response.shouldKeepAlive = false;
+        response._last = true;
+      }
+    }
+  }
+
+  // existing destroy handling
+};
+```
+
+## 테스트 및 검증
+
+두 가지 상황을 각각 회귀 테스트로 추가했다.
+
+### Headers가 아직 전송되지 않은 경우
+
+첫 번째 response가 정상적으로 끝나더라도 connection이 close되어야 한다.
+
+```js
+assert.strictEqual(res.headers.connection, 'close');
+```
+
+두 번째 request에서는 기존 socket을 재사용하지 않는지 확인했다.
+
+```js
+assert.strictEqual(second.reusedSocket, false);
+```
+
+### Headers가 이미 전송된 경우
+
+response 일부를 먼저 전송한 뒤 request body 처리를 중간에 종료하는 상황을 만들었다.
+
+```js
+res.write('partial');
+```
+
+이 경우 response/socket을 destroy하고, 클라이언트 측에서 `ECONNRESET`이 발생하는 것을 확인했다.
+
+```js
+res.on('error', common.expectsError({
+  code: 'ECONNRESET',
+  message: 'aborted',
+}));
+```
+
+이후 두 번째 request가 새로운 socket을 사용하는 것도 확인했다.
+
+```js
+assert.strictEqual(second.reusedSocket, false);
+```
+
+## 로컬 테스트
+
+새로 추가한 회귀 테스트를 실행했다.
+
+```bash
+python3 tools/test.py --mode=release \
+  test/parallel/test-http-server-for-await-keepalive.js \
+  test/parallel/test-http-server-for-await-keepalive-headers-sent.js
+```
+
+기존 관련 테스트도 함께 실행했다.
+
+```bash
+python3 tools/test.py --mode=release \
+  test/parallel/test-stream-destroy.js \
+  test/parallel/test-http-server-incomingmessage-destroy.js \
+  test/parallel/test-http-incoming-pipelined-socket-destroy.js
+```
+
+그리고 JavaScript lint도 확인했다.
+
+```bash
+make lint-js
+```
+
+관련 테스트와 lint 모두 로컬에서 통과했다.
+
+## CI 및 Follow-up
+
+CI 과정에서 일부 실패가 있었지만, `x86_64-darwin` 환경의 `test-tick-processor-arguments.js` timeout으로 확인되었고 이번 HTTP 변경과 직접적인 관련은 없어 보였다.
+
+이후 수정 방향과 테스트 결과를 정리해 reviewer에게 다시 확인을 요청했다.
+
+또한 최신 `main`을 기준으로 rebase한 뒤 다시 리뷰를 요청했다.
+
+## 현재 상태
+
+현재 PR #65674는 아직 Open 상태다.
+
+진행 과정은 다음과 같다.
+
+```text
+Issue #49429 분석
+→ 문제 재현
+→ 불완전한 request body + keep-alive 재사용 문제 확인
+→ 초기 수정
+→ ronag Changes Requested
+→ connection 재사용 방지 방향으로 수정
+→ headers 전송 전/후 회귀 테스트 추가
+→ mcollina 피드백
+→ 수정 위치를 Stream에서 HTTP로 이동
+→ IncomingMessage._destroy() 수정
+→ 관련 테스트 및 lint 통과
+→ 최신 main으로 rebase
+→ 재리뷰 요청
+```
+
+현재는 maintainer의 추가 리뷰를 기다리고 있다.
+
+## 배운 점
+
+이번 기여를 통해 Stream lifecycle과 Socket lifecycle, HTTP connection lifecycle을 각각 구분해서 봐야 한다는 점을 배웠다.
+
+특히 request stream이 destroy됐다고 해서 underlying socket까지 자동으로 종료되는 것은 아니며, keep-alive connection의 재사용 여부는 HTTP layer에서 판단해야 한다는 점을 이해하게 됐다.
+
+또한 Core 코드에서는 단순히 문제가 발생한 위치를 수정하는 것이 아니라, 해당 동작을 어느 계층이 책임져야 하는지 판단하는 것이 중요하다는 점을 경험했다.