fix: unpkg support non-npm publish tgz file #485
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| paths.push(entry.path.replace(/^package\//i, '/')); | ||
| // https://github.com/cnpm/cnpmcore/issues/452#issuecomment-1570077310 | ||
| // strip first dir, e.g.: 'package/', 'lodash-es/' | ||
| paths.push('/' + entry.path.split('/').slice(1).join('/')); | ||
| }, | ||
| }); | ||
| for (const path of paths) { |
There was a problem hiding this comment.
代码看起来像是一个文件管理器，以下是我的评估：
- 在onentry函数中进行路径检查可以有效过滤掉非文件和非预期的路径，这点没什么问题。
- 这个代码补丁在paths列表中添加了一些路径，但是它没有任何检查来确保这些路径存在或可读。这可能导致一些潜在的运行时错误。
- 从
package/开始的路径应该被忽略，因为它们不会被正确地处理。原有代码删除了此检查，使用了另一个方法来解决此问题。这种方法的优势在于它很容易被理解并且比较安全，但是目前的实现依然有问题。如果新增一个名称为'package'的隐藏文件夹，则所有包含了'package/'的路径都会失效。
为了修复这个问题，我建议做以下改动：
- 恢复原来的'package/'检查。
- 使用Node.js自带的path模块来操作路径（例如path.join, path.resolve）。
- 确保paths中的所有路径都是存在及可读的。
- 对路径中的特殊字符进行转义。
| assert.equal(res.status, 302); | ||
| assert.equal(res.header.location, `/${pkg.name}/1.0.0/files/index.js`); | ||
| }); | ||
|
|
||
| it('should ignore "." hidden dir', async () => { | ||
| // https://unpkg.com/browse/bovo-ui@0.0.4-36/ | ||
| const tarball = await TestUtil.readFixturesFile('unpkg.com/bovo-ui-0.0.4-36.tgz'); |
There was a problem hiding this comment.
这段代码补丁主要是在一个测试用例中添加了对于非npm的tgz包的测试支持。看起来没有什么明显的问题，但以下是一些改进建议：
-
在计算文件完整性时使用
crypto.subtle.digest()而不是通过 OpenSSL 进行 HMAC 的计算。因为 Node.js 12.4.0 版本以后，Node.js 内置API提供了一个稳定的和低级别的加密模块crypto.webcrypto，摒弃了依赖系统本身的 OpenSSL 库，从而将安全风险最小化。 -
可以使用类型检查工具例如TypeScript来使得代码更加可维护。
-
没有可读性强的变量名，变量名应该尽可能描述其用途。如果没有明确的意义，请使用常见约定命名。
-
将一些魔术数字（如201，200等）定义为常量或枚举类型来提高代码的可读性，尤其是当您需要知道哪些状态码是成功并且哪些是错误时。
-
通过代理对象（Proxy Objects）或 ES6 别名语法（Symbol）将私有成员标记为内部使用来避免泄漏公共接口。
-
对于一些无处不在的字符串，比如测试用例中的“location”，增加常量或者枚举可以让代码更加健壮且可读性更好。
[skip ci] ## [3.23.2](v3.23.1...v3.23.2) (2023-05-31) ### Bug Fixes * unpkg support non-npm publish tgz file ([#485](#485)) ([5fe883f](5fe883f)), closes [/github.com//issues/452#issuecomment-1570077310](https://github.com/cnpm//github.com/cnpm/cnpmcore/issues/452/issues/issuecomment-1570077310)
| paths.push(entry.path.replace(/^package\//i, '/')); | ||
| // https://github.com/cnpm/cnpmcore/issues/452#issuecomment-1570077310 | ||
| // strip first dir, e.g.: 'package/', 'lodash-es/' | ||
| paths.push('/' + entry.path.split('/').slice(1).join('/')); |
There was a problem hiding this comment.
https://github.com/npm/pacote/blob/main/lib/fetcher.js#L445
对 gitignore 还有额外处理.. 感觉可以用 pacote#extract 模拟一下安装解压的过程再批量上传一下文件？
#452 (comment)