Skip to content

fix(backend): ULIDを正しく処理できない問題を修正#17310

Merged
kakkokari-gtyih merged 1 commit intomisskey-dev:developfrom
chan-mai:fix/ulid-crockford-base32
Apr 15, 2026
Merged

fix(backend): ULIDを正しく処理できない問題を修正#17310
kakkokari-gtyih merged 1 commit intomisskey-dev:developfrom
chan-mai:fix/ulid-crockford-base32

Conversation

@chan-mai
Copy link
Copy Markdown
Contributor

What

parseUlidFullがULIDのランダム部のパースにparseBigInt32(内部でparseInt(chunk, 32)を使用)を呼んでいた実装を、Crockford Base32に対応したparseBigIntCrockfordに置き換える。

Why

parseInt(str, 32)が認識する文字は標準base-32の0–9, A–Vのみで、ULIDが採用するCrockford Base32固有のW, X, Y, Zを無効文字として扱う。

W/X/Y/Zがチャンク先頭に現れるとparseIntNaNを返して例外が発生し、NotificationService#toXListIdを経由して伝播した後、本番環境ではtrackPromiseがno-opであるためエラーログにも残らず通知が消滅していた。発生確率は訳23.4%

また、チャンク先頭以外にW/X/Y/Zが現れる場合もparseIntが途中で解析を停止して誤値を返すため、Redis StreamのIDが不正な値となりソート順が壊れる問題もあった。

Related: #17309

Additional info (optional)

parseBigIntCrockfordは既存のCHARS定数(Crockford Base32)を使ってCHARS.indexOfで各文字を変換する。タイムスタンプ部のparseBase32がすでに同方針で正しく実装されており、それと一貫した実装となっている

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 14, 2026
@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/backend:test labels Apr 14, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.66%. Comparing base (c9c6ef2) to head (e62001e).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #17310      +/-   ##
===========================================
- Coverage    63.67%   63.66%   -0.02%     
===========================================
  Files         1162     1162              
  Lines       116490   116497       +7     
  Branches      8470     8461       -9     
===========================================
- Hits         74175    74162      -13     
- Misses       40110    40130      +20     
  Partials      2205     2205              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

@github-actions
Copy link
Copy Markdown
Contributor

Backend memory usage comparison

Before GC

Metric base (MB) head (MB) Diff (MB) Diff (%)
VmRSS 315.86 MB 321.54 MB +5.68 MB +1.80%
VmHWM 315.86 MB 321.54 MB +5.68 MB +1.80%
VmSize 23110.44 MB 23114.08 MB +3.64 MB +0.01%
VmData 1381.31 MB 1385.16 MB +3.85 MB +0.27%

After GC

Metric base (MB) head (MB) Diff (MB) Diff (%)
VmRSS 315.87 MB 321.55 MB +5.68 MB +1.80%
VmHWM 315.87 MB 321.55 MB +5.68 MB +1.80%
VmSize 23110.60 MB 23114.08 MB +3.47 MB +0.01%
VmData 1381.47 MB 1385.16 MB +3.68 MB +0.26%

After Request

Metric base (MB) head (MB) Diff (MB) Diff (%)
VmRSS 316.29 MB 321.93 MB +5.64 MB +1.78%
VmHWM 316.29 MB 321.93 MB +5.64 MB +1.78%
VmSize 23110.69 MB 23114.25 MB +3.55 MB +0.01%
VmData 1381.56 MB 1385.33 MB +3.77 MB +0.27%

See workflow logs for details

@chan-mai
Copy link
Copy Markdown
Contributor Author

このCIの失敗はPRの変更と無関係のflaky testです
標準Nodeバージョンでは同テストがPassしています

@kakkokari-gtyih kakkokari-gtyih merged commit 5dc5083 into misskey-dev:develop Apr 15, 2026
38 of 39 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in [実験中] 管理用 Apr 15, 2026
kakkokari-gtyih added a commit that referenced this pull request Apr 15, 2026
@syuilo
Copy link
Copy Markdown
Member

syuilo commented Apr 15, 2026

🙏🏻

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

Labels

packages/backend:test packages/backend Server side specific issue/PR size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Development

Successfully merging this pull request may close these issues.

parseUlidFull silently drops ~23% of notifications due to Crockford Base32 mismatch

3 participants