Skip to content

fix: 쿼리 없는 동아리 상세 URL에 &를 붙여 clubId가 오염되던 것을 고친다 - #36

Open
seongwon030 wants to merge 1 commit into
mainfrom
mixpanel-identity-split-audit
Open

seongwon030 wants to merge 1 commit into
mainfrom
mixpanel-identity-split-audit

Conversation

@seongwon030

@seongwon030 seongwon030 commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

문제

sessionId가 빈 상태에서 구독 중인 동아리 상세를 열면 동아리 상세 화면이 뜨지 않는다.

원인

appendSessionId는 sessionId가 비면 URL을 그대로 돌려준다 (utils/webview.ts:11). 그 다음 줄에서 구독 여부를 붙이는 쪽이 "앞 단계에서 ?가 생겼다"고 가정하고 &를 쓴다.

?가 없는데 &를 붙이면 브라우저는 그 뒤를 쿼리로 승격시키지 않고 경로의 일부로 읽는다.

url      = https://moadong.com/webview/club/abc123&is_subscribed=true
pathname = /webview/club/abc123&is_subscribed=true   ← 전부 경로
search   = ""                                         ← 쿼리는 비어 있음

라우트가 /webview/club/:clubId (frontend routes/webviewRoutes.tsx:25)라서 clubId가 abc123&is_subscribed=true로 오염되고, 그 ID로는 동아리를 못 찾아 상세가 렌더되지 않는다.

sessionId가 있을 때는 appendSessionId가 ?를 만들어 주므로 &가 우연히 맞는 구분자가 된다. 그래서 지금까지 드러나지 않았다.

변경

appendSessionId가 쓰는 것과 같은 분기(utils/webview.ts:12)로 구분자를 고른다. 한 줄.

- url += `&is_subscribed=true`;
+ url += `${url.includes('?') ? '&' : '?'}is_subscribed=true`;

검증

sessionId 구독 결과 URL session_id is_subscribed
있음 O ...abc123?session_id=…&is_subscribed=true ✅ ✅
있음 X ...abc123?session_id=… ✅ —
빈값 O ...abc123?is_subscribed=true — ✅
빈값 X ...abc123 — —

3행이 이번에 고쳐진 경우. 고치기 전에는 clubId가 오염돼 둘 다 못 읽었다.

범위 밖 (후속)

  • sessionId가 빌 때 웹이 session_id를 못 받아 익명 신원이 되는 문제는 남아 있다. 이건 네이티브/웹뷰 Mixpanel 신원 분리 이슈에서 함께 다룬다.
  • club-detail / webview/[slug]에 홈(home-webview-screen.tsx:59-60)과 같은 세션 로딩 게이트를 넣는 건 의도적으로 제외했다. 외부 URL 웹뷰까지 Mixpanel 준비를 기다리게 만들어 범위가 넓어진다.

Summary by CodeRabbit

  • 버그 수정
    • 구독 상태에서 클럽 상세 WebView URL에 is_subscribed 정보가 올바르게 추가되도록 수정했습니다.
    • 기존 쿼리 문자열 유무에 따라 적절한 구분자를 사용해 WebView가 정상적으로 열립니다.

appendSessionId는 sessionId가 비면 URL을 그대로 돌려준다. 그 뒤 구독 여부를
붙이는 쪽이 앞 단계에서 ?가 생겼다고 가정하고 &를 쓰는데, ?가 없으면 브라우저는
& 이후를 쿼리로 승격시키지 않고 경로의 일부로 읽는다. 그러면 라우트
/webview/club/:clubId 의 clubId가 "<id>&is_subscribed=true" 가 되어 동아리
조회에 실패하고 상세 화면이 뜨지 않는다.

appendSessionId가 쓰는 것과 같은 분기로 구분자를 고른다. sessionId가 비어도
clubId는 온전하고 session_id만 빠진다.

sessionId가 비는 순간(부트스트랩 완료 전 딥링크 진입)은 코드상 도달 가능하다고
판단했을 뿐 재현하지는 못했다.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ba13dd93-a917-441d-bd8d-805dfc0d8df5

📥 Commits

Reviewing files that changed from the base of the PR and between 16ae7da and d93b500.

📒 Files selected for processing (1)
  • ui/club-detail/club-detail-screen.tsx

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

ClubWebViewScreen은 기존 쿼리 문자열의 유무에 따라 is_subscribed=true 파라미터의 구분자를 선택합니다.

Changes

클럽 WebView URL

Layer / File(s) Summary
구독 파라미터 구분자 수정
ui/club-detail/club-detail-screen.tsx
URL에 쿼리 문자열이 있으면 &를 사용하고, 없으면 ?를 사용합니다.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 쿼리 문자열이 없는 동아리 상세 URL에 잘못된 '&'를 사용해 발생한 clubId 오염 문제의 수정 내용을 정확히 설명합니다.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@seongwon030

Copy link
Copy Markdown
Member Author

검증 정정: 이 PR을 올릴 때 적은 tsc --noEmit 통과는 실제로는 검증되지 않은 상태였습니다. 워크트리에 node_modules가 없어 npx tsc가 typescript를 찾지 못했는데, 래퍼가 완료 메시지를 출력해서 통과로 잘못 읽었습니다.

의존성을 설치하고 이 브랜치에서 다시 돌렸습니다:

  • tsc --noEmit — 통과 (exit 0)
  • npm run lint — 0 errors, 5 warnings 모두 기존 import/no-named-as-default (변경과 무관)

결과적으로 원래 주장은 맞았지만, 그때는 근거가 없었습니다. 본문의 동작 검증(입출력 시뮬레이션)은 실제로 돌린 것이라 유효합니다.

🤖 Generated with Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant