Skip to content

Conversation

@yama
Copy link
Member

@yama yama commented Jan 22, 2026

Motivation

  • ファイルアップロード時に transalias 相当の正規化(stripAlias)がファイル名全体に対して行われたため、先頭が非ASCIIの日本語のみのファイル名でベース名が空になりアップロードが拒否される問題を解消するため。

Description

  • manager/media/browser/mcpuk/connectors/Commands/FileUpload.php のアップロード処理を修正し、拡張子を分離してベース名のみを stripAlias に渡すようにしました。
  • stripAlias の結果ベース名が空文字になった場合は date('Ymd')-<random8> 形式のフォールバック名を生成して拡張子を復元するようにしました。
  • 変更により拡張子は保持され、管理画面のファイルブラウザ経由のアップロード時に日本語(漢字)で始まるファイル名も正規化されて受け付けられるようになります。

Testing

  • 自動化テストは実行していません。

Codex Task

Copilot AI review requested due to automatic review settings January 22, 2026 10:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a file upload issue where Japanese filenames beginning with non-ASCII characters would be rejected during upload. The problem occurred because the stripAlias function was applied to the entire filename including the extension, potentially resulting in an empty basename when the filename contained only non-ASCII characters.

Changes:

  • Modified the filename cleaning logic to separate the basename from the extension before applying stripAlias
  • Added a fallback mechanism to generate date-based filenames when stripAlias returns an empty basename
  • Ensured the file extension is preserved after filename normalization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

$basename = substr($filename, 0, strrpos($filename, '.'));
$basename = $modx->stripAlias($basename, ['file_manager']);
if ($basename === '') {
$basename = date('Ymd') . '-' . substr(md5(uniqid(mt_rand(), true)), 0, 8);
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The fallback basename generation uses mt_rand() which is deprecated as of PHP 7.1 and should be replaced with random_int() for better randomness and future compatibility. Consider using substr(md5(uniqid((string)random_int(0, PHP_INT_MAX), true)), 0, 8) or a more modern approach like bin2hex(random_bytes(4)).

Suggested change
$basename = date('Ymd') . '-' . substr(md5(uniqid(mt_rand(), true)), 0, 8);
$basename = date('Ymd') . '-' . substr(md5(uniqid((string)random_int(0, PHP_INT_MAX), true)), 0, 8);

Copilot uses AI. Check for mistakes.
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