Fix openssh key malformed comment#10
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
📝 WalkthroughWalkthrough将 OpenSSH 私钥解析中 RSA、Ed25519、ECDSA 三种类型的 comment 字段读取改为使用 reader.readUtf8(allowMalformed: true),并新增测试(包含生成畸形 comment 的 helper 与针对三种密钥类型的解析断言)。 变更SSH 密钥解析容错性
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/src/ssh_key_pair_test.dart (1)
122-133: ⚡ Quick win建议把畸形 comment 回归测试补齐到 RSA/ECDSA。
当前只覆盖了 Ed25519;但本 PR 同时改动了 RSA 和 ECDSA 的 comment 解析路径。建议各补 1 个 malformed comment 用例,避免后续回归漏检。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/ssh_key_pair_test.dart` around lines 122 - 133, Add parallel unit tests for RSA and ECDSA mirroring the Ed25519 case: create malformed-comment PEM fixtures for RSA and ECDSA, assert SSHKeyPair.isEncryptedPem(...) is false, call SSHKeyPair.fromPem(...) and expect a single key, cast to the corresponding classes (e.g., OpenSSHRSAKeyPair and OpenSSHECDSAKeyPair), and add simple assertions on their publicKey/privateKey (e.g., non-empty lengths or expected byte lengths) to ensure the comment-parsing changes are covered for both RSA and ECDSA.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/src/ssh_key_pair_test.dart`:
- Around line 122-133: Add parallel unit tests for RSA and ECDSA mirroring the
Ed25519 case: create malformed-comment PEM fixtures for RSA and ECDSA, assert
SSHKeyPair.isEncryptedPem(...) is false, call SSHKeyPair.fromPem(...) and expect
a single key, cast to the corresponding classes (e.g., OpenSSHRSAKeyPair and
OpenSSHECDSAKeyPair), and add simple assertions on their publicKey/privateKey
(e.g., non-empty lengths or expected byte lengths) to ensure the comment-parsing
changes are covered for both RSA and ECDSA.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7443f98d-8989-491a-9054-7f5f6a875d06
📒 Files selected for processing (2)
lib/src/ssh_key_pair.darttest/src/ssh_key_pair_test.dart
📜 Review details
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check
🔇 Additional comments (2)
lib/src/ssh_key_pair.dart (1)
354-354: LGTM!Also applies to: 412-412, 461-461
test/src/ssh_key_pair_test.dart (1)
74-81: LGTM!
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/src/ssh_key_pair_test.dart (2)
75-82: ⚡ Quick win建议统一测试数据生成方式。
当前 Ed25519 密钥使用预生成的常量,而 RSA(第 127-128 行)和 ECDSA(第 129-130 行)密钥通过
withMalformedOpenSSHComment辅助函数动态生成。这种不一致可能让维护者困惑。建议改为使用辅助函数从
ed25519Privatefixture 生成,与其他密钥类型保持一致:final ed25519PrivateWithMalformedComment = withMalformedOpenSSHComment(ed25519Private);这样可以提高测试代码的一致性和可维护性。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/ssh_key_pair_test.dart` around lines 75 - 82, Replace the hard-coded ed25519PrivateWithMalformedComment constant with a call to the existing helper to keep fixtures consistent: use withMalformedOpenSSHComment(ed25519Private) instead of the multiline literal; update the test file symbol ed25519PrivateWithMalformedComment to be assigned by invoking withMalformedOpenSSHComment on the ed25519Private fixture so Ed25519, RSA and ECDSA malformed-comment fixtures are generated the same way.
84-125: 建议为辅助函数添加文档注释。该函数的作用是将 OpenSSH 私钥的 comment 字段替换为畸形 UTF-8 序列,逻辑较为复杂。建议添加文档注释说明函数用途、参数和返回值。
另外,第 118 行的畸形字节
[0xbb, 0xa8]缺少说明。建议添加行内注释解释为何选择这些字节(它们是没有前导字节的 UTF-8 continuation bytes,构成无效的 UTF-8 序列)。📝 建议添加的文档注释
+/// Creates a test OpenSSH private key with a malformed UTF-8 comment field. +/// +/// Takes an unencrypted OpenSSH private key PEM and replaces the comment +/// field with invalid UTF-8 bytes [0xbb, 0xa8] to test malformed-comment +/// handling in the parser. String withMalformedOpenSSHComment(String pemText) { final pem = SSHPem.decode(pemText); final keyPairs = OpenSSHKeyPairs.decode(pem.content); ... reader.readString(); + // Write invalid UTF-8: continuation bytes without a leading byte writer.writeString(Uint8List.fromList([0xbb, 0xa8]));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/ssh_key_pair_test.dart` around lines 84 - 125, Add a Dart doc comment to the helper function withMalformedOpenSSHComment explaining its purpose (it replaces the OpenSSH private-key comment with an intentionally malformed UTF-8 sequence), the parameter it consumes (pemText as PEM string) and the return value (a PEM string with the modified privateKeyBlob), and briefly describe the transformation steps (decode PEM, parse key blob, rebuild blob with malformed comment). Also add an inline comment at the writer.writeString(Uint8List.fromList([0xbb, 0xa8])) call explaining that the bytes [0xBB, 0xA8] are chosen because they are UTF-8 continuation bytes without a leading byte, producing an invalid UTF-8 sequence for testing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/src/ssh_key_pair_test.dart`:
- Around line 75-82: Replace the hard-coded ed25519PrivateWithMalformedComment
constant with a call to the existing helper to keep fixtures consistent: use
withMalformedOpenSSHComment(ed25519Private) instead of the multiline literal;
update the test file symbol ed25519PrivateWithMalformedComment to be assigned by
invoking withMalformedOpenSSHComment on the ed25519Private fixture so Ed25519,
RSA and ECDSA malformed-comment fixtures are generated the same way.
- Around line 84-125: Add a Dart doc comment to the helper function
withMalformedOpenSSHComment explaining its purpose (it replaces the OpenSSH
private-key comment with an intentionally malformed UTF-8 sequence), the
parameter it consumes (pemText as PEM string) and the return value (a PEM string
with the modified privateKeyBlob), and briefly describe the transformation steps
(decode PEM, parse key blob, rebuild blob with malformed comment). Also add an
inline comment at the writer.writeString(Uint8List.fromList([0xbb, 0xa8])) call
explaining that the bytes [0xBB, 0xA8] are chosen because they are UTF-8
continuation bytes without a leading byte, producing an invalid UTF-8 sequence
for testing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dad4c29c-6151-411e-b68f-4e46c0170b55
📒 Files selected for processing (1)
test/src/ssh_key_pair_test.dart
📜 Review details
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check
- GitHub Check: check
🔇 Additional comments (5)
test/src/ssh_key_pair_test.dart (5)
7-7: LGTM!
127-130: LGTM!
139-149: LGTM!
158-170: LGTM!
197-208: LGTM!
lollipopkit/flutter_server_box/issues/1178
Summary by CodeRabbit
Bug修复
测试