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

ディレクトリ配下のファイルでもプレビューだけ表示できるように修正 #121

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

cm-wada-yusuke
Copy link
Member

@cm-wada-yusuke cm-wada-yusuke commented Jan 31, 2025

📑 Summary

  • プレビュー可否を判定するロジックで、URIのパターンを厳密な完全一致ではなく部分一致としました
  • これにより Cmd+Shift+P で実行できる 「プレビュー」機能で、ユーザーフォルダー配下のファイルもプレビューできるようになります
  • ツリービューには表示できません(ユーザーフォルダをどう切り替えるかや、全部まとめて表示するかなど別の検討が必要)

セキュリティ上のリスクについて

  • [^/]+パターンによりパストラバーサルを防止
  • ファイル拡張子とディレクトリ構造の制約を維持
  • 明確な'/articles/'や'/books/'パターン

により影響は小さいと考えています。

Resolves #120

📋 Tasks

プルリクエストを作成いただく際、お手数ですが以下の内容についてご確認をお願いします。

  • 📖 Contribution Guide を読んだ
  • 👩‍💻 canary ブランチに対するプルリクエストである
  • 実行して正しく動作しているか確認する
  • 不要なコードが含まれていないか( コメントやログの消し忘れに注意 )
  • XSS になるようなコードが含まれていないか
  • Pull Reuqest の内容は妥当か( 膨らみすぎてないか )

より詳しい内容は Pull Request Policy を参照してください。

@@ -225,7 +225,8 @@
"@types/webpack-env": "^1.17.0",
"@typescript-eslint/eslint-plugin": "^5.30.0",
"@typescript-eslint/parser": "^5.30.0",
"@vscode/test-web": "^0.0.26",
"@vscode/test-web": "^0.0.65",
"@vscode/vsce": "^3.2.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

[nits] vsce は非推奨だから @vscode/vsce に変えてねとコンソールに出たため従いました。

<span className={styles.topic}>{topic}</span>
<span key={topic} className={styles.topic}>
{topic}
</span>
Copy link
Member Author

Choose a reason for hiding this comment

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

[nits] ブラウザコンソールにエラーが出ていたので修正

article: new RegExp(/\/articles\/(?:[^/]+\/)*[^/]+\.md$/),
bookChapter: new RegExp(/\/books\/[^/]+\/[^/]+\.md$/),
bookConfig: new RegExp(`/books/[^/]+/${BOOK_CONFIG_FILE_PATTERN.source}$`), // prettier-ignore
bookCoverImage: new RegExp(`/books/[^/]+/${BOOK_COVER_IMAGE_FILE_PATTERN.source}$`), // prettier-ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] この部分をプレビューしようとしているファイルの妥当性に使用していました。ここをユーザーフォルダーが入っていても .test() メソッドでパスするような正規表現に修正しています。

Copy link
Member

Choose a reason for hiding this comment

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

後方一致にしたということですね!

Copy link
Member Author

Choose a reason for hiding this comment

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

はい、そのとおりです。

Copy link
Member

@cm-igarashi-ryosuke cm-igarashi-ryosuke left a comment

Choose a reason for hiding this comment

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

コメント確認お願いします!

article: new RegExp(/\/articles\/(?:[^/]+\/)*[^/]+\.md$/),
bookChapter: new RegExp(/\/books\/[^/]+\/[^/]+\.md$/),
bookConfig: new RegExp(`/books/[^/]+/${BOOK_CONFIG_FILE_PATTERN.source}$`), // prettier-ignore
bookCoverImage: new RegExp(`/books/[^/]+/${BOOK_COVER_IMAGE_FILE_PATTERN.source}$`), // prettier-ignore
Copy link
Member

Choose a reason for hiding this comment

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

後方一致にしたということですね!

bookConfig: new RegExp(`^${booksFolderUri}/[^/]+/${BOOK_CONFIG_FILE_PATTERN.source}$`), // prettier-ignore
bookCoverImage: new RegExp(`^${booksFolderUri}/[^/]+/${BOOK_COVER_IMAGE_FILE_PATTERN.source}$`), // prettier-ignore
book: new RegExp(/\/books\/[^/]+\/?$/),
article: new RegExp(/\/articles\/(?:[^/]+\/)*[^/]+\.md$/),
Copy link
Member

Choose a reason for hiding this comment

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

[q] ここだけ条件が変わっていると思うのですが意図した変更ですか?(articles以下のディレクトリが深くてもOKになっている?)

Copy link
Member Author

Choose a reason for hiding this comment

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

ありがとうございます。意図していませんでした。いろいろ試した結果の正規表現が残ってしまっていました。ご指摘ありがとうございます。ついでに 変数を使っていないところはわざわざ new RegExp() を使う必要もないため、合わせて修正しました。

0c39e80

Copy link
Member

@cm-igarashi-ryosuke cm-igarashi-ryosuke left a comment

Choose a reason for hiding this comment

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

LGTMです!

@cm-wada-yusuke cm-wada-yusuke merged commit 8979b8e into canary Feb 3, 2025
1 check passed
@cm-wada-yusuke cm-wada-yusuke deleted the feat-deep-file-preview branch February 3, 2025 09:03
@cm-wada-yusuke cm-wada-yusuke mentioned this pull request Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ユーザーフォルダ配下のコンテンツもプレビューだけはできるように
2 participants