feat(copy): support advanced copy configuration with custom target paths #1711
Conversation
- Add CopyConfig enum to support both basic and advanced copy modes
- Basic mode: maintains backward compatibility with string[] format
- Advanced mode: supports {from: string, to: string} format for custom paths
- Update copy plugin to handle both configuration formats
- Ensure target directories are created automatically
Example config:
{
'copy': [
'public', // basic mode
{ 'from': 'assets', 'to': 'static' } // advanced mode
]
}
Walkthrough该拉取请求引入了一个新的枚举类型 Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/plugins/copy.rs (1)
71-77: 在复制前验证源路径是否存在在复制操作中，如果源路径不存在，
copy函数可能会失败。建议在复制前检查源路径的存在性，并在不存在时提供有意义的错误消息或跳过复制。示例修改：
match config { CopyConfig::Basic(src) => { let src = context.root.join(src); + if !src.exists() { + eprintln!("Source path {:?} does not exist", src); + continue; + } debug!("copy {:?} to {:?}", src, dest); copy(&src, dest)?; } // 其他分支... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
crates/mako/src/config.rs(2 hunks)crates/mako/src/plugins/copy.rs(4 hunks)
🔇 Additional comments (1)
crates/mako/src/config.rs (1)
147-147: 确保所有使用config.copy的代码已更新
Config结构体中的copy字段类型从Vec<String>更改为Vec<CopyConfig>，这可能影响到依赖此字段的其他代码。请确认所有使用config.copy的代码都已更新以适应新的类型。
生成以下脚本以查找所有使用config.copy的代码位置：
✅ Verification successful
Let me check the implementation in the copy plugin to verify if it's already compatible with the new type.
代码已正确适配新的类型变更
从代码实现来看，crates/mako/src/plugins/copy.rs中的代码已经完全适配了Vec<CopyConfig>的新类型，正确处理了CopyConfig::Basic和CopyConfig::Advanced两种情况。没有发现其他使用config.copy的地方需要更新。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：查找所有使用`config.copy`的代码，确保兼容新的类型
rg 'config\.copy' --glob '!crates/mako/src/config.rs'
Length of output: 225
Script:
#!/bin/bash
# Check the implementation in copy.rs to see how config.copy is used
rg -A 10 'config\.copy' crates/mako/src/plugins/copy.rs
Length of output: 1012
| for config in context.config.copy.iter() { | ||
| let src = match config { | ||
| CopyConfig::Basic(src) => context.root.join(src), | ||
| CopyConfig::Advanced { from, .. } => context.root.join(from), | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议检查源路径是否存在
当前代码在处理src之前未检查其是否存在，可能会在源路径不存在时引发错误。建议在处理之前添加对src存在性的验证，并在不存在时进行适当的错误处理或日志记录。
示例修改：
for config in context.config.copy.iter() {
let src = match config {
CopyConfig::Basic(src) => context.root.join(src),
CopyConfig::Advanced { from, .. } => context.root.join(from),
};
+ if !src.exists() {
+ eprintln!("Source path {:?} does not exist", src);
+ continue;
+ }
if src.exists() {
debug!("watch {:?}", src);
// ...
}
}Committable suggestion skipped: line range outside the PR's diff.
crates/mako/src/plugins/copy.rs
Outdated
| let target = dest.join(to.trim_start_matches("/")); | ||
|
|
||
| if !target.exists() { | ||
| fs::create_dir_all(&target)?; | ||
| } | ||
|
|
||
| debug!("copy {:?} to {:?}", src, target); | ||
| copy(&src, &target)?; | ||
| } |
There was a problem hiding this comment.
防止目标路径的目录遍历漏洞
在处理CopyConfig::Advanced配置时，to字段可能包含恶意路径（如../），导致文件复制到意外的位置。建议对目标路径进行规范化和验证，确保目标路径位于预期的目标目录中，防止潜在的安全问题。
示例修改：
let target = dest.join(to.trim_start_matches("/"));
+ let target = target.canonicalize()?;
+ let dest_canonical = dest.canonicalize()?;
+ if !target.starts_with(&dest_canonical) {
+ return Err(anyhow!("Invalid target path: {:?}", target));
+ }
if !target.exists() {
fs::create_dir_all(&target)?;
}Committable suggestion skipped: line range outside the PR's diff.
| #[derive(Deserialize, Serialize, Debug)] | ||
| #[serde(untagged)] | ||
| pub enum CopyConfig { | ||
| Basic(String), | ||
| Advanced { from: String, to: String }, | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
为CopyConfig添加配置验证
CopyConfig中的from和to字段直接来自用户配置，可能包含非法或不安全的路径。建议在配置解析时添加路径验证，确保路径不存在注入风险并且指向合法的位置。
Add path canonicalization and validation to ensure target paths remain within the destination directory
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/plugins/copy.rs (1)
71-78: 建议优化基础配置的错误处理基础配置的处理逻辑过于简单，建议添加更详细的错误处理和日志记录。
建议修改如下：
CopyConfig::Basic(src) => { let src = context.root.join(src); + debug!("正在处理基础复制配置"); + if !src.exists() { + debug!("源路径不存在: {:?}", src); + continue; + } debug!("copy {:?} to {:?}", src, dest); copy(&src, dest)?; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
crates/mako/src/plugins/copy.rs(4 hunks)
🔇 Additional comments (2)
crates/mako/src/plugins/copy.rs (2)
34-39: 建议在处理源路径之前进行验证
这段代码在处理配置时缺少对源路径的预先验证。虽然后续有exists()检查，但建议在匹配过程中就进行更全面的验证。
建议修改如下：
for config in context.config.copy.iter() {
let src = match config {
CopyConfig::Basic(src) => context.root.join(src),
CopyConfig::Advanced { from, .. } => context.root.join(from),
};
+
+ if !src.exists() {
+ debug!("跳过不存在的源路径: {:?}", src);
+ continue;
+ }79-96: 🛠️ Refactor suggestion
建议增强高级配置的路径处理和安全性
当前的路径验证逻辑可以进一步加强，同时建议添加更多的安全检查。
建议修改如下：
CopyConfig::Advanced { from, to } => {
let src = context.root.join(from);
+ if !src.exists() {
+ debug!("源路径不存在: {:?}", src);
+ continue;
+ }
let target = dest.join(to.trim_start_matches("/"));
let target = target.canonicalize()?;
let dest_canonical = dest.canonicalize()?;
if !target.starts_with(&dest_canonical) {
- return Err(anyhow!("Invalid target path: {:?}", target));
+ return Err(anyhow!("目标路径不安全: {:?}", target));
}
+ // 验证目标路径不包含特殊字符
+ if to.contains("..") {
+ return Err(anyhow!("目标路径包含非法字符: {:?}", to));
+ }
+
if !target.exists() {
+ debug!("创建目标目录: {:?}", target);
fs::create_dir_all(&target)?;
}
debug!("copy {:?} to {:?}", src, target);
copy(&src, &target)?;
}Likely invalid or redundant comment.
crates/mako/src/plugins/copy.rs
Outdated
| let target = target.canonicalize()?; | ||
| let dest_canonical = dest.canonicalize()?; | ||
| if !target.starts_with(&dest_canonical) { | ||
| return Err(anyhow!("Invalid target path: {:?}", target)); | ||
| } |
There was a problem hiding this comment.
路径规范化可能导致错误
使用canonicalize()进行路径规范化时，如果路径不存在会导致错误。建议在这种情况下使用更健壮的处理方式。
建议添加错误处理：
- let target = target.canonicalize()?;
- let dest_canonical = dest.canonicalize()?;
+ let target = target.canonicalize().map_err(|e| {
+ anyhow!("无法规范化目标路径 {:?}: {}", target, e)
+ })?;
+ let dest_canonical = dest.canonicalize().map_err(|e| {
+ anyhow!("无法规范化输出目录 {:?}: {}", dest, e)
+ })?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let target = target.canonicalize()?; | |
| let dest_canonical = dest.canonicalize()?; | |
| if !target.starts_with(&dest_canonical) { | |
| return Err(anyhow!("Invalid target path: {:?}", target)); | |
| } | |
| let target = target.canonicalize().map_err(|e| { | |
| anyhow!("无法规范化目标路径 {:?}: {}", target, e) | |
| })?; | |
| let dest_canonical = dest.canonicalize().map_err(|e| { | |
| anyhow!("无法规范化输出目录 {:?}: {}", dest, e) | |
| })?; | |
| if !target.starts_with(&dest_canonical) { | |
| return Err(anyhow!("Invalid target path: {:?}", target)); | |
| } |
- Updated the type of the `copy` property in the `BuildParams` interface to support both `string` and `{ from: string; to: string }`.
- Ensured the `copy` configuration is properly validated to handle both types.
- Updated the `copy` property type in the configuration documentation to reflect the change from `string[]` to `(string | { from: string; to: string })[]`.
- Clarified that the `copy` configuration can now accept both strings and objects with `from` and `to` properties.
- Update config.copy test fixtures to cover from/to pattern - Add assertions for copied files in new location - Adjust copy plugin path validation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
e2e/fixtures/config.copy/mako.config.json (1)
2-8: 配置示例清晰展示了新旧两种格式配置文件很好地展示了两种复制模式的用法：
- 基础模式：使用字符串格式
"src/assets"- 高级模式：使用对象格式
{"from": "src/assets", "to": "assets-from-to"}建议在项目文档中添加这个示例，以帮助用户理解新的配置选项。
e2e/fixtures/config.copy/expect.js (1)
5-9: 建议增加更多测试用例当前测试覆盖了基本功能，建议添加以下测试场景：
- 源路径不存在的情况
- 目标路径包含特殊字符的情况
- 复制大量文件时的性能测试
- 复制时发生冲突的情况
crates/mako/src/plugins/copy.rs (2)
71-77: 建议优化错误处理在基础模式下的复制操作中，建议添加更详细的错误上下文信息，以便于调试：
-copy(&src, dest)?; +copy(&src, dest).map_err(|e| { + anyhow!("复制文件失败 - 从 {:?} 到 {:?}: {}", src, dest, e) +})?;
Line range hint
115-134: 建议添加进度报告功能对于大文件或大量文件的复制操作，建议添加进度报告功能：
let options = fs_extra::dir::CopyOptions::new() .content_only(true) .skip_exist(false) + .buffer_size(64 * 1024) // 优化缓冲区大小 .overwrite(true); +let handle = |process_info: fs_extra::TransitProcess| { + debug!( + "复制进度: {}/{} bytes", + process_info.copied_bytes, + process_info.total_bytes + ); + fs_extra::dir::TransitProcessResult::ContinueOrAbort +}; -fs_extra::dir::copy(&entry, dest, &options)?; +fs_extra::dir::copy_with_progress(&entry, dest, &options, handle)?;docs/config.zh-CN.md (1)
119-119: 文档更新准确反映了新的复制配置选项类型定义和默认值的说明清晰准确。建议添加一个配置示例来帮助用户更好地理解新的对象格式用法。
建议添加如下示例：
### copy - 类型：`(string | { from: string; to: string })[]` - 默认值：`["public"]` 指定需要复制的文件或目录。默认情况下，会将 `public` 目录下的文件复制到输出目录。 +例如： +```json +{ + "copy": [ + "public", + { "from": "./src/assets", "to": "static/assets" } + ] +} +```docs/config.md (1)
119-119: 建议添加配置示例文档中已更新了
copy配置的类型定义，但建议添加使用新的{ from, to }对象格式的具体示例，以帮助用户更好地理解如何使用这个新特性。例如：{ "copy": [ "public", // 基础字符串格式 { "from": "assets/images", // 源路径 "to": "static/img" // 目标路径 } ] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
crates/binding/src/lib.rs(1 hunks)crates/mako/src/plugins/copy.rs(4 hunks)docs/config.md(1 hunks)docs/config.zh-CN.md(1 hunks)e2e/fixtures/config.copy/expect.js(1 hunks)e2e/fixtures/config.copy/mako.config.json(1 hunks)packages/bundler-mako/index.js(1 hunks)packages/mako/binding.d.ts(1 hunks)
🔇 Additional comments (5)
crates/mako/src/plugins/copy.rs (2)
34-39: 建议检查源路径是否存在
当前代码在处理src之前未检查其是否存在，可能会在源路径不存在时引发错误。建议在处理之前添加对src存在性的验证。
79-94:
建议加强路径安全性和错误处理
- 路径安全：
let target = dest.join(to.trim_start_matches("/"));
+let target = target.canonicalize().map_err(|e| {
+ anyhow!("无法解析目标路径 {:?}: {}", target, e)
+})?;
+let dest_path = dest_path.canonicalize().map_err(|e| {
+ anyhow!("无法解析输出目录 {:?}: {}", dest_path, e)
+})?;- 目录创建：
-if !target.exists() {
- fs::create_dir_all(&target)?;
+if !target.exists() {
+ fs::create_dir_all(&target).map_err(|e| {
+ anyhow!("创建目标目录失败 {:?}: {}", target, e)
+ })?;
}packages/mako/binding.d.ts (1)
137-137: LGTM: Type definition correctly supports both basic and advanced copy configurations
The updated type definition properly supports both the legacy string array format and the new object format with from and to fields, ensuring type safety and backward compatibility.
crates/binding/src/lib.rs (1)
77-77: LGTM: Rust implementation aligns with TypeScript definition
The BuildParams struct's copy field type has been correctly updated to support both string and object configurations, maintaining consistency with the TypeScript interface.
packages/bundler-mako/index.js (1)
256-258: 实现正确且错误提示清晰
配置校验逻辑实现完善：
- 正确支持字符串和对象两种格式
- 对象格式必须包含
from和to字段 - 错误提示信息清晰明确
- Add directory cleanup when path validation fails - Use canonicalized paths for more reliable path validation - Add concatenateModules option type to BuildParams
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/plugins/copy.rs (1)
79-99: 建议添加源路径存在性检查在进行复制操作之前，应该验证源路径是否存在，以避免不必要的操作和潜在的错误。
CopyConfig::Advanced { from, to } => { let src = context.root.join(from); + if !src.exists() { + debug!("源路径不存在，跳过复制: {:?}", src); + continue; + } let target = dest.join(to.trim_start_matches("/")); // ... 后续代码 ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
crates/mako/src/plugins/copy.rs(4 hunks)packages/mako/binding.d.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/mako/binding.d.ts
🔇 Additional comments (2)
crates/mako/src/plugins/copy.rs (2)
34-39: 建议在处理配置前添加源路径验证
当前代码在处理配置之前没有验证源路径是否存在，这可能导致在后续处理中出现问题。建议在匹配配置类型之后、使用路径之前添加验证。
for config in context.config.copy.iter() {
let src = match config {
CopyConfig::Basic(src) => context.root.join(src),
CopyConfig::Advanced { from, .. } => context.root.join(from),
};
+ if !src.exists() {
+ debug!("跳过不存在的源路径: {:?}", src);
+ continue;
+ }
if src.exists() {
debug!("watch {:?}", src);
// ...
}
}88-95: 🛠️ Refactor suggestion
建议增强路径规范化的错误处理
当前的路径规范化错误处理可能不够健壮，特别是在处理不存在的路径时。建议添加更详细的错误信息和恢复机制。
- let canonical_target = target.canonicalize()?;
- let canonical_dest_path = dest.canonicalize()?;
+ let canonical_target = target.canonicalize().map_err(|e| {
+ anyhow!("无法规范化目标路径 {:?}: {}", target, e)
+ })?;
+ let canonical_dest_path = dest.canonicalize().map_err(|e| {
+ anyhow!("无法规范化输出目录 {:?}: {}", dest, e)
+ })?;
if !canonical_target.starts_with(&canonical_dest_path) {
if was_created {
- fs::remove_dir_all(&target)?;
+ if let Err(e) = fs::remove_dir_all(&target) {
+ debug!("清理无效目标目录失败: {:?}, 错误: {}", target, e);
+ }
}
return Err(anyhow!("无效的目标路径: {:?}", target));
}Likely invalid or redundant comment.
| let was_created = if !target.exists() { | ||
| fs::create_dir_all(&target).is_ok() | ||
| } else { | ||
| false | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议改进目录创建的错误处理
当前代码使用is_ok()简单处理目录创建的结果，建议添加更详细的错误处理和日志记录。
- let was_created = if !target.exists() {
- fs::create_dir_all(&target).is_ok()
- } else {
- false
- };
+ let was_created = if !target.exists() {
+ match fs::create_dir_all(&target) {
+ Ok(_) => {
+ debug!("成功创建目标目录: {:?}", target);
+ true
+ }
+ Err(e) => {
+ debug!("创建目标目录失败: {:?}, 错误: {}", target, e);
+ false
+ }
+ }
+ } else {
+ false
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let was_created = if !target.exists() { | |
| fs::create_dir_all(&target).is_ok() | |
| } else { | |
| false | |
| }; | |
| let was_created = if !target.exists() { | |
| match fs::create_dir_all(&target) { | |
| Ok(_) => { | |
| debug!("成功创建目标目录: {:?}", target); | |
| true | |
| } | |
| Err(e) => { | |
| debug!("创建目标目录失败: {:?}, 错误: {}", target, e); | |
| false | |
| } | |
| } | |
| } else { | |
| false | |
| }; |
🎯 Features:
• Added CopyConfig enum supporting both basic and advanced modes
• Maintained backward compatibility with string[] format
• Introduced {from, to} format for custom target paths
• Enhanced copy plugin for both configuration types
• Added automatic target directory creation
📝 Example Configuration:
🔍 Details:
• Basic mode preserves existing string[] functionality
• Advanced mode enables precise path control
• Directories are created automatically as needed
• Both modes can be mixed in the same config"
Summary by CodeRabbit
新功能
CopyConfig枚举，支持基本和高级的文件复制配置。CopyPlugin，支持更灵活的源-目标路径关系，确保高级配置下的路径有效性。文档
测试