Skip to content

qmlui: support concurrent native client sessions - #2094

Open
infrafast wants to merge 5 commits into
mcallegari:masterfrom
infrafast:fix/native-multiclient-clean
Open

qmlui: support concurrent native client sessions#2094
infrafast wants to merge 5 commits into
mcallegari:masterfrom
infrafast:fix/native-multiclient-clean

Conversation

@infrafast

@infrafast infrafast commented Aug 16, 2026

Copy link
Copy Markdown

Summary

This clean PR supersedes #2093. The previous PR branch accidentally received a later packaging commit that added out/rpi-arm64-rootfs, inflating the diff to 2910 files. This branch is recreated directly from the current official master and contains only the native multi-client implementation.

Changes

  • assign a UUID to every accepted native TCP connection;
  • store and address clients by session ID rather than IP address or client name;
  • retain client name, peer IP, and peer port as display-only metadata;
  • broadcast updates independently to every authenticated session;
  • remove only the session associated with the disconnected socket;
  • reject native actions from unauthenticated sessions;
  • enforce each authorized session's access mask for inbound actions;
  • make project transfer and authorization decisions target an immutable session ID;
  • serialize manual authorization requests through a FIFO queue;
  • retain immutable name/peer metadata and a guarded socket reference per queued request;
  • clean up active and queued requests when their socket disconnects;
  • show client name, peer endpoint, and session ID in the authorization popup;
  • add the runtime-only -sa / --server-allow-all option for unattended deployments;
  • log automatic authorization and print a prominent startup warning.

Compatibility and defaults

  • Native wire protocol unchanged.
  • Existing native clients remain compatible.
  • Manual authorization remains the default.
  • -sa / --server-allow-all is runtime-only and is not persisted.

Scope verification

Exactly 7 files are modified:

  • qmlui/app.cpp
  • qmlui/app.h
  • qmlui/main.cpp
  • qmlui/qml/MainView.qml
  • qmlui/qml/popup/PopupNetworkConnect.qml
  • qmlui/tardis/networkmanager.cpp
  • qmlui/tardis/networkmanager.h

README.md is unchanged from upstream, as requested by the maintainer. No build output, package, rootfs, or out/ content is included.

Validation

  • branch is based directly on current upstream/master (a7f1b68f);
  • full ARM64 qlcplus5 build completed successfully on the implementation head before the later portability/naming-only changes;
  • the branch includes the current upstream CLI rename work and is not behind master;
  • Windows portability fix remains declaration-only (NativeAccessRequest anonymous typedef struct -> named C++ struct) with no runtime logic or layout change;
  • maintainer feedback is applied: README.md restored to upstream and command-line option renamed to -sa / --server-allow-all;
  • GitHub Actions run #2482 on current head 6fa21927 completed successfully;
  • Linux v5: success, including configure, build, tests, install, AppImage build and load test;
  • Windows v5: success, including configure, build, install, windeployqt and installer package;
  • macOS v5: success, including configure, build, install, dylib fixup and DMG creation;
  • the complete workflow run also passed the v4 and coverage jobs.

Ready for maintainer review.

Key native TCP clients by a unique session ID instead of peer address. Serialize manual authorization requests, enforce per-session access masks, and add the runtime-only --allow-all-native option.
@coveralls

coveralls commented Aug 17, 2026

Copy link
Copy Markdown

Coverage Status

Coverage is 35.324%infrafast:fix/native-multiclient-clean into mcallegari:master. No base build found for mcallegari:master.

Copy link
Copy Markdown
Author

The current corrected head (08cf6597) is ready for CI revalidation, but GitHub Actions run #2479 is blocked with action_required before any jobs are created. Could a maintainer please approve/run the workflow for this fork PR? I am keeping the PR as draft until the Linux v5, Windows v5 and macOS v5 jobs have actually completed successfully.

@infrafast
infrafast marked this pull request as ready for review August 18, 2026 11:05
@mcallegari

mcallegari commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Please remove the changes to README.md and name the command line option like this:
"-sa" "--server-allow-all"

As I am renaming --remote to --server

Thanks

@infrafast
infrafast marked this pull request as draft August 18, 2026 13:45

Copy link
Copy Markdown
Author

Thanks. Done on current head 2f0854f1:

  • restored README.md exactly to upstream, so it is no longer part of the PR diff;
  • renamed the unattended native-server option to -sa / --server-allow-all as requested;
  • updated the startup warning to use --server-allow-all.

I have put the PR back in draft while revalidating the current head. GitHub Actions run #2480 is currently action_required before any jobs are created, so it needs maintainer approval to run. Once Linux v5, Windows v5 and macOS v5 are green again, I will mark the PR ready for review.

Resolve conflicts between the concurrent native client sessions work and
the remote project handling landed on master:

- m_hostsMap is now keyed by session ID, so NetProjectRequest identifies
  the requester from its own socket (hostForSocket) instead of a host name
  in the payload. getHostFromName() is gone and a session can no longer
  ask for the project on behalf of another
- NetProjectRequest requires an authenticated session, as it is a 'case'
  and would otherwise bypass the access mask check in 'default'
- notifyProjectChanging()/notifyProjectLoaded() iterate the session map
  and honour the QPointer sockets
- sendWorkspaceToClient() takes a session ID
- keep the per-socket RX buffering (m_rxBuffers) and its cleanup on
  disconnect, alongside the new session teardown
@mcallegari

Copy link
Copy Markdown
Owner

@infrafast today I worked a little bit on the Android version so native protocol is very important and needed some adjustments. Your PR is still valid and I appreciate it, thanks.
I resolved the conflicts I created today and once it builds it's ready to be merged.

@infrafast
infrafast marked this pull request as ready for review August 18, 2026 17:59

Copy link
Copy Markdown
Author

Thanks. The requested changes are now applied and revalidated on the current upstream master: README.md is unchanged, the unattended option is -sa / --server-allow-all, and GitHub Actions run #2482 is green on Linux v5, Windows v5 and macOS v5. The PR is back to Ready for review.

}
int NetworkManager::requiredAccessMask(int actionCode) const
{
if (actionCode >= 0x0100 && actionCode < 0x0200) return 1 << 6;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't quite like this one.
Better use named enums rather than numbers

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.

3 participants