Enhance short-id deserialization and improve test coverage by grill-glitch · Pull Request #1438 · Watfaq/clash-rs

grill-glitch · 2026-05-22T13:11:33Z

What does this PR do?

Problem

Config parser currently expects reality.short-id to be a string. If users write short-id: (empty key) or an empty YAML sequence, serde errors with "invalid type: sequence, expected a string".

Fix

Add a custom serde deserializer that accepts string / null / unit / empty sequence and normalizes empty cases to "" (empty string). Existing non-empty short-id behavior unchanged.

Rationale

Improves UX and tolerates common YAML styles; downstream hex::decode("") already returns empty Vec<u8>, so behavior is consistent.

Changes

clash-lib/src/config/internal/proxy.rs: added deserialize_short_id custom deserializer and annotated RealityOpt.short_id with #[serde(deserialize_with = "deserialize_short_id", default)]

Type

Bug fix
New feature
Refactoring / cleanup
Other

Checklist

Tests added or not needed
Docs updated or not needed

Summary by CodeRabbit

Release Notes

Bug Fixes
- Improved handling of Reality proxy short-id configuration to accept multiple input formats (strings, null, and single-element sequences) while validating against invalid multi-element sequences.
Tests
- Added comprehensive test coverage for short-id deserialization scenarios and proxy configuration parsing.

…ll/empty/seq → empty string)

- Reorder serde imports to comply with case-sensitive ASCII ordering - Validate sequence length in visit_seq: error on multi-element sequences instead of silently discarding extra values

…t_id The deserialize_short_id function uses fmt::Formatter and fmt::Result with the fmt:: prefix, but only Display and Formatter were imported. Adding self to the fmt import makes the fmt module available as a path prefix. Fixes compilation error: cannot find module or crate 'fmt' in this scope

- Add unit tests for deserialize_short_id: - plain string, null, tilde (~), empty string - hex string, empty seq, single-element seq - single-element empty string seq - multi-element seq rejection (2 and 3 elements) - missing field (default) - Add integration tests for RealityOpt within OutboundVless: - short-id as string, null, seq, missing - reality-opts absent entirely - multi-value seq rejected at OutboundVless level

coderabbitai · 2026-05-22T13:11:51Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 984725f4-6416-46f4-9d0e-d210d9e8bf71

📥 Commits

Reviewing files that changed from the base of the PR and between 0fdd7a8 and 62e8fe7.

📒 Files selected for processing (1)

clash-lib/src/config/internal/proxy.rs

📝 Walkthrough

Walkthrough

This PR adds custom YAML deserialization for Reality configuration's short-id field. A new deserialize_short_id function normalizes null, unit, and empty sequences to empty strings, accepts single-element sequences, and rejects multi-value sequences. The field is integrated into RealityOpt with appropriate defaults. Comprehensive tests validate the deserializer and its integration with RealityOpt parsing.

Changes

Reality short-id deserialization

Layer / File(s)	Summary
Deserializer implementation and RealityOpt integration `clash-lib/src/config/internal/proxy.rs`	Introduces `deserialize_short_id` custom visitor that normalizes `null`, unit (`~`), and empty sequences to empty string; accepts single-element sequences and rejects multi-element sequences with a specific error. Wires the deserializer into `RealityOpt.short_id` with a string default. Adjusts import block formatting.
Test coverage for short-id deserialization `clash-lib/src/config/internal/proxy.rs`	New `short_id_tests` module tests deserializer behavior across string inputs, null/unit/empty/multi-element sequences, and missing fields. Integration tests verify `RealityOpt` parsing within `OutboundVless` reality configuration from YAML, including rejection of multi-value short-id entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A short-id arrives in YAML's form,
Null becomes empty—the new norm.
Single sequences flow through the gate,
Multiples bounce—that's too much freight!
Tests stand guard where deserts roam.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: introduction of custom short-id deserialization logic and expansion of test coverage for this new functionality.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-05-22T13:22:53Z

Codecov Report

❌ Patch coverage is 91.57895% with 16 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
clash-lib/src/config/internal/proxy.rs	91.57%	14 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

grill-glitch and others added 6 commits May 17, 2026 09:37

config: tolerate multiple YAML shapes for reality.short-id (accept nu…

5b715de

…ll/empty/seq → empty string)

fix: address CodeRabbitAI review - import order + seq length validation

81c67dc

- Reorder serde imports to comply with case-sensitive ASCII ordering - Validate sequence length in visit_seq: error on multi-element sequences instead of silently discarding extra values

Merge branch 'master' into fix/reality-shortid-deser

7327f4b

Merge branch 'Watfaq:master' into fix/reality-shortid-deser

62e8fe7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhance short-id deserialization and improve test coverage#1438

Enhance short-id deserialization and improve test coverage#1438
grill-glitch wants to merge 6 commits into
Watfaq:masterfrom
grill-glitch:fix/reality-shortid-deser

grill-glitch commented May 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 22, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

grill-glitch commented May 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Problem

Fix

Rationale

Changes

Type

Checklist

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented May 22, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

grill-glitch commented May 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 22, 2026 •

edited

Loading