Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UUIDの生成を、UUIDv4からUUIDv7にする #2466

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

TwoSquirrels
Copy link
Member

@TwoSquirrels TwoSquirrels commented Jun 25, 2024

Issue: #2424

とりあえず全ての uuid.NewV4uuid.NewV7 に置き換えてみました。
uuid.NewV3 はいじっていません。

セキュリティ上の問題がないかが少し心配です。
というのも、仮にトークン的な役割で UUIDv4 を使っている箇所がある場合、そこを v7 にしたことでランダム性が落ちてしまうと考えられるからです。

@TwoSquirrels TwoSquirrels requested a review from Takeno-hito June 25, 2024 08:04
@TwoSquirrels TwoSquirrels changed the title 全ての UUIDv4 の生成を UUIDv7 に置き換え UUIDの生成を、UUIDv4からUUIDv7にする Jun 25, 2024
@Takeno-hito
Copy link
Member

comment: テストのところも uuid v7 に書き換えたいが、v7 の知識をもとにプログラムが書かれても困るので(現状の運用的に)v4 / v7 がどっちも振ってくるテストを書くと良い…?

@Takeno-hito
Copy link
Member

  • 動作確認

@Takeno-hito Takeno-hito marked this pull request as ready for review June 26, 2024 05:23
@Takeno-hito Takeno-hito requested a review from pikachu0310 June 26, 2024 05:23
Copy link
Member

@Takeno-hito Takeno-hito left a comment

Choose a reason for hiding this comment

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

僕のレビューがついてなかった LGTM ですが traQ メンバーのレビューを貰ってください~

Copy link
Member

@kyosu-1 kyosu-1 left a comment

Choose a reason for hiding this comment

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

良さそうです 👍 (データ型としての互換性はあると思うので)

@TwoSquirrels TwoSquirrels merged commit 4915ff4 into master Jun 27, 2024
6 checks passed
@TwoSquirrels TwoSquirrels deleted the replace-uuidv4-with-v7 branch June 27, 2024 23:46
@Takeno-hito Takeno-hito restored the replace-uuidv4-with-v7 branch June 28, 2024 03:06
@Takeno-hito Takeno-hito deleted the replace-uuidv4-with-v7 branch June 28, 2024 03:07
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.

3 participants