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

絵文字の名前に@や:が使用できる #9964

Merged
merged 6 commits into from
Mar 20, 2023

Conversation

nenohi
Copy link
Contributor

@nenohi nenohi commented Feb 16, 2023

What

絵文字追加後のUpdateにて@や:、Unicode絵文字等をカスタム絵文字名に追加できないようにした

下記正規表現
/^[a-z0-9_]+$/
絵文字名の被りが無いかチェックも追加

Why

絵文字名にて予想外の文字が含まれていた場合に正常に読み込めなくなる為
(#9918)

Additional info (optional)

ローカル環境にてテスト済み
下記確認
・絵文字に英数字アンダーバーのみを登録できること
・日本語のみでエラー
・日本語含む英数字でエラー
・ユニコード絵文字のみでエラー
・同じ絵文字名が合った場合エラー

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Feb 16, 2023
@codecov
Copy link

codecov bot commented Feb 16, 2023

Codecov Report

Merging #9964 (f682f73) into develop (4db787c) will increase coverage by 50.14%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##           develop    #9964       +/-   ##
============================================
+ Coverage    23.19%   73.33%   +50.14%     
============================================
  Files          698      809      +111     
  Lines        64883    77524    +12641     
  Branches      1984     5392     +3408     
============================================
+ Hits         15050    56854    +41804     
+ Misses       49833    20670    -29163     
Impacted Files Coverage Δ
...end/src/server/api/endpoints/admin/emoji/update.ts 68.81% <75.00%> (+68.81%) ⬆️

... and 659 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ekkekuru2
Copy link
Contributor

OSSに関わる経験が浅いのでコメントの仕方とかが間違ってたらすみません

ちょうど同じissueに取り組んでいていくつか試行錯誤していました。
Additional infoで言及されていますが、条件の範囲を逆にした (/^[a-z0-9_]+$/) 等の正規表現の方が良いと思いました。

背景

  • 新規絵文字追加の際には、以下のように使用できる文字かどうかで判定がされていること

const name = driveFile.name.split('.')[0].match(/^[a-z0-9_]+$/) ? driveFile.name.split('.')[0] : `_${rndstr('a-z0-9', 8)}_`;

  • unicode絵文字、@等を含む文字列以外でも正しく表示されない場合があること
    ローカル環境で試したところ、こちらのPRで修正していただいている場合以外の文字列でも正しく表示されない場合がありました。
    例 ) カスタム絵文字の名前を試しに「にほんご」に設定すると、👍と表示される
    image

@nenohi
Copy link
Contributor Author

nenohi commented Feb 16, 2023

ありがとうございます、
やっぱり逆のほうが良さそうですね

ただ、絵文字に使える文字でどこまで許容(どの文字まで使えるように)するかが問題になってきそうですね

@ekkekuru2
Copy link
Contributor

さっそく変更ありがとうございます。

ただ、絵文字に使える文字でどこまで許容(どの文字まで使えるように)するかが問題になってきそうですね

たしかにその問題は出てきますね...
個人的には、今変更していただいた (/^[a-z0-9_]+$/) で、新規絵文字追加とも同じになって良いと思いました。

@tamaina
Copy link
Contributor

tamaina commented Mar 9, 2023

https://json-schema.org/understanding-json-schema/reference/regular_expressions.html

JSON Schema(paramDef)のpatternにできませんか?

@nenohi
Copy link
Contributor Author

nenohi commented Mar 9, 2023

大丈夫そう

@syuilo syuilo merged commit e152047 into misskey-dev:develop Mar 20, 2023
@syuilo
Copy link
Member

syuilo commented Mar 20, 2023

🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants