Skip to content

Adapted for OHOS#567

Open
FrankHan052176 wants to merge 4 commits into
rustdesk:mainfrom
FrankHan052176:ohos/rustls-on-ohos
Open

Adapted for OHOS#567
FrankHan052176 wants to merge 4 commits into
rustdesk:mainfrom
FrankHan052176:ohos/rustls-on-ohos

Conversation

@FrankHan052176

@FrankHan052176 FrankHan052176 commented Jul 6, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • Bug Fixes
    • Improved OHOS compatibility by aligning configuration paths, ID/key generation, and network/TLS behavior with the platform’s available TLS support.
    • Updated websocket TLS connector handling and disabled OHOS cert-validation relaxation/retry logic where not supported.
    • Fixed HTTPS proxy TLS handling for OHOS and adjusted TLS cache behavior accordingly.
    • Standardized OHOS fingerprinting to use a consistent placeholder address.
    • Narrowed Linux-only IPC/system logic so it won’t run on OHOS.
  • Maintenance
    • Updated cryptography dependency source and refined platform-target dependency selection for TLS/websocket support.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1acb52c5-f813-404a-9aef-35f8910b771c

📥 Commits

Reviewing files that changed from the base of the PR and between 39124f9 and cf31ab2.

📒 Files selected for processing (1)
  • src/tls.rs

📝 Walkthrough

Walkthrough

This PR adds target_env = "ohos" conditional compilation across dependency selection, config and ID handling, fingerprinting, top-level exports, platform gating, TLS connection paths, and TLS cache handling.

Changes

OHOS Platform Support

Layer / File(s) Summary
Dependency selection for OHOS builds
Cargo.toml
Switches sodiumoxide to the upstream git dependency, splits TLS dependencies by target, and excludes OHOS from a Linux-only dependency section.
Config paths, IPC directories, and IDs
src/config.rs
Updates OHOS handling in config paths, IPC directory selection, log paths, online-state gating, and ID generation while excluding OHOS from Linux-only branches.
Fingerprinting, exports, and platform gates
src/fingerprint.rs, src/lib.rs, src/platform/mod.rs
Zeros fingerprint addresses for OHOS, adjusts export and get_uuid() guards, and excludes Linux platform code from OHOS builds.
Proxy TLS dispatch and fallback
src/proxy.rs
Restricts native-TLS proxy code to non-OHOS targets, routes OHOS NativeTls connections through rustls, and makes OHOS fallback log and bail instead of retrying native-TLS.
Websocket TLS connector and retry gating
src/websocket.rs
Builds rustls connectors on OHOS, gates native-TLS-specific code, and removes invalid-certificate retry branches from OHOS builds.
OHOS TLS cache handling
src/tls.rs
Skips TLS cache updates when OHOS uses NativeTls.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Build as Cargo target configuration
  participant Runtime as OHOS runtime
  participant TLS as Proxy/Websocket TLS
  participant Cache as TLS cache

  Build->>Runtime: Select OHOS cfg and rustls dependencies
  Runtime->>TLS: Create rustls connector for TLS connections
  TLS->>Cache: Skip NativeTls cache update on OHOS
  TLS-->>Runtime: Return connection or fail without native-TLS fallback
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and clearly matches the main change: adapting the project for OHOS support.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@FrankHan052176

Copy link
Copy Markdown
Author

OHOS is “OpenHarmonyOS”

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@Cargo.toml`:
- Line 36: Pin the forked sodiumoxide dependency in Cargo.toml by replacing the
floating branch reference with a fixed rev or tag, and if this fork is only
needed for OHOS, move the dependency entry into the OHOS target-specific section
instead of keeping it global. Locate the existing sodiumoxide dependency
declaration and update it so builds use a stable, reproducible source.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7fecf016-447b-43f9-bf12-a4122a248b8a

📥 Commits

Reviewing files that changed from the base of the PR and between c7f49e4 and 39124f9.

📒 Files selected for processing (1)
  • Cargo.toml

Comment thread Cargo.toml
@FrankHan052176

FrankHan052176 commented Jul 6, 2026

Copy link
Copy Markdown
Author

Since the upstream sodiumoxide is archived, I had to fork it to make the necessary adaptations for openharmonyos via a git link.
Here is my fork: https://github.com/FrankHan052176/sodiumoxide
To make it easier to maintain, would it be possible to move this fork directly into the rustdesk-org organization? I'm happy to transfer it if you agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant