Skip to content

Conversation

@whqtker
Copy link
Member

@whqtker whqtker commented Nov 27, 2025

관련 이슈

작업 내용

모든 요청에 대해 Connection 헤더의 값이 upgrade로 설정되는 문제가 있었습니다.
Upgrade 헤더 유무에 따라 Connection 헤더의 값을 동적으로 설정하도록 수정하였습니다.

특이 사항

AS-IS

WebSocket 핸드셰이크

  • 클라이언트는 Nginx에 Connection 헤더에 Upgrade, Upgrade 헤더에 websocket 을 포함하여 전송한다.
  • Nginx는 이를 수신 후, Connection 헤더의 값을 Upgrade, Upgrade 헤더에 받은 요청의 Upgrade 헤더 값으로 설정하여 서버로 전송한다.
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection "upgrade";
  • 아래는 실제 Nginx로부터 받은 요청입니다.
  • 수정: IP 주소와 같은 민감 정보는 지웠습니다.
GET /connect/123/randomsession/websocket?token=YOUR_ACCESS_TOKEN HTTP/1.1
Host: api.stage.solid-connection.com
X-Real-IP: XXX.XX.XXX.XX
X-Forwarded-For: XXX.XX.XXX.XX
X-Forwarded-Proto: https
Upgrade: websocket
Connection: upgrade
User-Agent: curl/7.81.0
Accept: */*
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==

일반 HTTP 요청

  • 클라이언트는 Nginx에 Connection 헤더에 keep-alive, Upgrade 헤더는 값을 포함하지 않은 채 전송한다.
  • Nginx는 이를 수신 후, Connection 헤더의 값을 Upgrade로 설정하고, Upgrade 헤더의 값을 null로 설정하여 서버로 전송한다.
  • 요청을 받은 서버는 Upgrade 요청이나 정작 Upgrade 헤더의 값이 비어있는 것을 보고 일반 HTTP로 처리한다.
  • 아래는 실제 Nginx로부터 받은 요청입니다.
GET / HTTP/1.1
Host: api.stage.solid-connection.com
X-Real-IP: 1XXX.XX.XXX.XX
X-Forwarded-For: XXX.XX.XXX.XX
X-Forwarded-Proto: https
Connection: upgrade
User-Agent: curl/7.81.0
Accept: */*

리뷰 요구사항 (선택)

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

  1. 변경 1 — 각 환경 설정 파일에 $connection_upgrade를 계산하는 map 블록을 추가했습니다.
  2. 변경 2 — HTTPS 서버 블록에서 proxy_set_header Connection "upgrade"proxy_set_header Connection $connection_upgrade로 변경했습니다.
  3. 변경 3 — 서브모듈 src/main/resources/secret의 커밋 참조를 업데이트했습니다.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10분

  • 동일한 패턴의 변경이 두 개의 Nginx 구성 파일에 반복 적용되어 검토 부담이 낮습니다.
  • 검토 시 주의할 파일/영역:
    • docs/infra-config/nginx.dev.conf (map 문법과 삽입 위치 확인)
    • docs/infra-config/nginx.prod.conf (map 문법과 삽입 위치 확인)
    • src/main/resources/secret (서브모듈 커밋 참조 정합성 확인)

Suggested reviewers

  • wibaek
  • Hexeong
  • JAEHEE25
  • lsy1307

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 Upgrade 헤더 유무에 따라 Connection 헤더를 동적으로 설정한다는 변경사항의 핵심을 명확하게 요약하고 있습니다.
Description check ✅ Passed PR 설명은 관련 이슈, 작업 내용, AS-IS 상황을 포함한 템플릿 요구사항을 충족하고 있습니다.
Linked Issues check ✅ Passed 변경사항이 #578의 요구사항을 충족합니다: nginx.conf에서 map을 통해 Connection 헤더를 동적으로 설정하여 일반 HTTP 요청에는 keep-alive를, WebSocket 요청에는 upgrade를 설정합니다.
Out of Scope Changes check ✅ Passed 모든 변경사항(nginx.dev.conf, nginx.prod.conf, 서브모듈 참조 업데이트)이 #578 해결과 직접 관련이 있습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa0bcef and 02be55c.

📒 Files selected for processing (3)
  • docs/infra-config/nginx.dev.conf (2 hunks)
  • docs/infra-config/nginx.prod.conf (2 hunks)
  • src/main/resources/secret (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@sukangpunch sukangpunch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! 제가 프로토콜에 대해 이해도가 낮아 간단하게 궁금한 내용이 있어서 여쭈어 봅니다.

만약 Connection 헤더가 upgrade 인 상황에선 어떤 문제가 발생하나요?
간단하게 알아 본 결과 요청 및 응답 결과에 대한 문제는 없지만,

  1. 프로토콜 표준 위반
  2. 리소스 비효율
    • 서버가 프로토콜 전환을 기대하며 대기 할 수 있다.(연결이 길어짐)
  3. 의미없는 리소스 점유
    • 실제로는 일반 http 요청인데 websocket 대기 상태처럼 처리

개선 내용

  1. 모든 요청에 Connection: upgrade로 가는 상황에서, Upgrade 헤더 기반으로 upgrade, keep-alive 의 값이 들어가게 함
    • 즉, websocket 연결이면 upgrade로 프로토콜을 변경하고, http 연결이면 Connection을 keep-alive로 유지하게 한다

이 정도의 문제로 이해하고 있는데 맞는지 궁금합니다!!

@whqtker
Copy link
Member Author

whqtker commented Nov 28, 2025

고생하셨습니다! 제가 프로토콜에 대해 이해도가 낮아 간단하게 궁금한 내용이 있어서 여쭈어 봅니다.

만약 Connection 헤더가 upgrade 인 상황에선 어떤 문제가 발생하나요? 간단하게 알아 본 결과 요청 및 응답 결과에 대한 문제는 없지만,

  1. 프로토콜 표준 위반

  2. 리소스 비효율

    • 서버가 프로토콜 전환을 기대하며 대기 할 수 있다.(연결이 길어짐)
  3. 의미없는 리소스 점유

    • 실제로는 일반 http 요청인데 websocket 대기 상태처럼 처리

서버가 Connection 헤더의 값이 upgrade로 설정되었으나 정작 Upgrade 헤더가 없는 응답을 받고, 따로 요청을 거절하지 않고 Upgrade 동작만 무시한 채 일반 HTTP 요청처럼 간주하는 것으로 알고 있습니다. 즉, 알아보신 바와 같이 동작에는 문제가 없습니다.
다만, 어쨌든 의도하지 않는 동작이므로 수정했습니다 !

개선 내용

  1. 모든 요청에 Connection: upgrade로 가는 상황에서, Upgrade 헤더 기반으로 upgrade, keep-alive 의 값이 들어가게 함

    • 즉, websocket 연결이면 upgrade로 프로토콜을 변경하고, http 연결이면 Connection을 keep-alive로 유지하게 한다

이 정도의 문제로 이해하고 있는데 맞는지 궁금합니다!!

이해하신 것이 맞습니다 🙆‍♂️

Copy link
Contributor

@sukangpunch sukangpunch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!

Copy link
Contributor

@Gyuhyeok99 Gyuhyeok99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오! 이런 건 또 처음알았네요! 공유해주셔서 감사합니다~

- Upgrade 헤더가 존재하면(e.g. WebSocket) upgrade로 설정
- Upgrade 헤더가 존재하지 않으면 keep-alive로 설정
@whqtker whqtker force-pushed the fix/578-request-header-caused-by-nginx branch from fa0bcef to 02be55c Compare December 21, 2025 10:48
@whqtker whqtker merged commit 1f12fea into solid-connection:develop Dec 21, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: HTTP 요청에 Connection 헤더가 upgrade로 설정되는 문제 해결

3 participants