Repository navigation
feat: store logind session id as string instead of int - #114
Merged
Merged
Conversation
wineee
marked this pull request as draft
October 9, 2026 10:27
Reviewer's GuideStores and transports logind session IDs as opaque strings across authentication, daemon IPC, socket commands, and Treeland integration, while using robust length-prefixed pipe framing and an empty-string greeter sentinel to prevent non-numeric ID collisions. Sequence diagram for string session ID propagation during loginsequenceDiagram
participant Display
participant Auth
participant Child as SessionLeaderChild
participant Logind
participant Treeland
Display->>Auth: openSession(command, env, cookie)
Auth->>Child: fork and start session
Child->>Child: read XDG_SESSION_ID
Child->>Auth: writeAll(idLen) + writeAll(idBytes)
Child->>Auth: writeAll(sessionPid)
Auth->>Auth: readFull(frame) and validate idLen
Auth->>Display: return QString xdgSessionId
Display->>Logind: ActivateSession(xdgSessionId)
Display->>Treeland: activateUser(user, xdgSessionId)
Sequence diagram for string session ID lock and logout commandssequenceDiagram
actor Greeter
participant SocketServer
participant Display
participant Logind
Greeter->>SocketServer: Logout or Lock
SocketServer->>SocketServer: input >> QString id
SocketServer->>Display: logout(socket, id) or lock(socket, id)
Display->>Logind: TerminateSession(id) or LockSession(id)
Greeter->>SocketServer: Unlock(user, password)
SocketServer->>Display: unlock(socket, user, password)
Display->>Logind: UnlockSession(auth.xdgSessionId)
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/daemon/Auth.cpp" line_range="338-339" />
<code_context>
- if (write(pipefd[1], &xdgSessionId, sizeof(int)) != sizeof(int)) {
+ const QByteArray idBytes = xdgSessionId.toLocal8Bit();
+ const quint32 idLen = static_cast<quint32>(idBytes.size());
+ if (write(pipefd[1], &idLen, sizeof(idLen)) != sizeof(idLen)
+ || write(pipefd[1], idBytes.constData(), idBytes.size()) != idBytes.size()) {
qCritical() << "[SessionLeader] Failed to write XDG_SESSION_ID to parent process!";
exit(1);
</code_context>
<issue_to_address>
**Valid sessions remain untracked**
When a pipe read or write is interrupted or transfers fewer bytes than requested, `Auth::openSession()` treats a partial transfer as fatal and returns an empty ID; `Display::startUserSession()` then deletes `Auth` while `sessionOpened` is false, so its destructor skips child cleanup and the opened session remains untracked.
Use complete-transfer loops for the pipe reads and writes, retrying on `EINTR` and continuing until each frame is fully transferred or a real error occurs.
Also at `src/daemon/Auth.cpp:377-383`.
</issue_to_address>
### Comment 2
<location path="src/daemon/SocketServer.cpp" line_range="160" />
<code_context>
- // read username
- int id;
+ // read session id
+ QString id;
input >> id;
// emit signal
</code_context>
<issue_to_address>
**Session requests fail across versions**
When the daemon and a greeter peer use different session-ID wire formats, `SocketServer::readyRead()` decodes logout and lock IDs as `QString`, while older greeters serialize or parse them as integers; activation IDs have the same mismatch. `QDataStream` misreads the IDs, so activation, locking, or logout fails or targets the wrong session.
Keep the session-ID wire format consistent across peers, or add version negotiation and compatibility handling for the integer format.
Also at `src/daemon/SocketServer.cpp:169`, `src/daemon/TreelandDisplayServer.cpp:78`, `src/daemon/TreelandDisplayServer.cpp:89`.
</issue_to_address>
There was a problem hiding this comment.
🔵 Needs a closer look
The coordinated fork-pipe and cross-project socket protocol changes require the documented DDM–Treeland integration testing. / 父子进程管道及跨项目套接字协议变更需要执行所述的 DDM–Treeland 集成测试。
0 open findings
What changed in this PR
Preserves non-numeric logind session IDs throughout DDM’s authentication and display-management flow.
中文：在 DDM 的认证与显示管理流程中完整保留非数字 logind 会话 ID。
Changes:
- Converts session IDs from integers to
QString. - Adds length-prefixed parent/child pipe transport.
- Updates DDM–Treeland socket messages accordingly.
变更：
- 将会话 ID 从整数改为
QString。 - 使用长度前缀在父子进程间传输会话 ID。
- 同步更新 DDM–Treeland 套接字消息。
| File | Description |
|---|---|
src/daemon/Auth.h |
Updates session ID storage and API types. / 更新会话 ID 存储和 API 类型。 |
src/daemon/Auth.cpp |
Preserves and transports string session IDs. / 保留并传输字符串会话 ID。 |
src/daemon/Display.h |
Updates activation, lock, and logout interfaces. / 更新激活、锁定和注销接口。 |
src/daemon/Display.cpp |
Uses string IDs for logind operations. / 使用字符串 ID 调用 logind。 |
src/daemon/SeatManager.cpp |
Uses an empty string for the greeter sentinel. / 使用空字符串作为登录界面哨兵。 |
src/daemon/SocketServer.h |
Updates socket signal parameter types. / 更新套接字信号参数类型。 |
src/daemon/SocketServer.cpp |
Deserializes lock and logout IDs as strings. / 将锁定和注销 ID 反序列化为字符串。 |
src/daemon/TreelandDisplayServer.h |
Updates the activation interface. / 更新激活接口。 |
src/daemon/TreelandDisplayServer.cpp |
Sends string IDs to Treeland. / 向 Treeland 发送字符串 ID。 |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wineee
force-pushed
the
feat/session-id-string
branch
from
October 10, 2026 02:12
fc2f006 to
0193b7f
Compare
wineee
force-pushed
the
feat/session-id-string
branch
2 times, most recently
from
October 10, 2026 02:48
c576997 to
3b85900
Compare
wineee
marked this pull request as ready for review
October 10, 2026 02:54
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/daemon/SocketServer.cpp" line_range="159-163" />
<code_context>
qDebug() << "Message received from greeter: Logout";
- // read username
- int id;
+ // read session id
+ QString id;
input >> id;
// emit signal
emit logout(socket, id);
</code_context>
<issue_to_address>
**Greeter can target arbitrary sessions**
When a process running as `dde` can connect to the greeter socket and knows another session's ID, `SocketServer::readyRead()` forwards the supplied ID without checking that it belongs to the requesting socket or its user, and `Display::logout()` passes it directly to `TerminateSession()`; the lock path similarly calls `LockSession()`. A process running as `dde` that can connect to the greeter socket can therefore terminate or lock another user's session by supplying its ID.
Authorize each requested action against the session associated with the requesting socket or otherwise verify that the session belongs to the caller before calling logind.
Also at `src/daemon/SocketServer.cpp:166-171`, `src/daemon/Display.cpp:510-522`.
</issue_to_address>
### Comment 2
<location path="src/daemon/Auth.cpp" line_range="437-440" />
<code_context>
+ }
+ // XDG_SESSION_ID is a short opaque string (numeric or e.g. "c1"),
+ // so guard against a corrupted/oversized length field from the pipe.
+ if (idLen == 0 || idLen > kMaxSessionIdLen) {
+ qWarning() << "[Auth] Invalid XDG_SESSION_ID length:" << idLen;
+ close(pipefd[0]);
+ return {};
+ }
+ QByteArray idBytes(static_cast<int>(idLen), Qt::Uninitialized);
</code_context>
<issue_to_address>
**Failed starts leave sessions open**
When PAM has opened the session and `openSession()` rejects the ID or fails before receiving the child PID, `openSession()` returns before setting `sessionOpened`, so `Auth::~Auth()` skips PAM cleanup; on the rejected-ID path, closing the pipe also leaves the child able to start the user process, so failed startup leaves the PAM session and potentially the process behind.
Track the PAM session as opened as soon as PAM opens it, and on every subsequent failure close it and terminate or reap the child before returning.
Also at `src/daemon/Auth.cpp:450-453`.
</issue_to_address>
### Comment 3
<location path="src/daemon/SocketServer.cpp" line_range="160" />
<code_context>
- // read username
- int id;
+ // read session id
+ QString id;
input >> id;
// emit signal
</code_context>
<issue_to_address>
**Mixed versions break session actions**
When the daemon and its greeter or Treeland peer run different protocol versions, `SocketServer::readyRead()` reads logout and lock IDs as `QString` while older greeters send integers, and the new activation writer sends a `QString` to older receivers expecting an integer. The peers misparse these fields, so lock, logout, or activation fails or targets the wrong session.
Keep the wire format backward compatible or add protocol-version negotiation and compatibility handling for these fields.
Also at `src/daemon/TreelandDisplayServer.cpp:78`, `src/daemon/TreelandDisplayServer.cpp:89`, `src/daemon/SocketServer.cpp:169-171`.
</issue_to_address>
wineee
force-pushed
the
feat/session-id-string
branch
from
October 10, 2026 03:05
3b85900 to
91ade20
Compare
1. Change Auth::xdgSessionId and openSession() to QString so non-numeric logind session ids (e.g. "c1") are preserved end to end. 2. Pass the session id over the parent/child pipe as length-prefixed bytes instead of a raw int. 3. Update SocketServer, Display and TreelandDisplayServer to send and receive session ids as strings. Log: No user-facing changes Influence: 1. Verify a normal user session still activates, locks, unlocks and logs out correctly after login via ddm. 2. Verify switching back to the greeter (dde session with empty id) does not crash or misroute the session. 3. Verify non-numeric logind session ids no longer collide with the dde greeter sentinel session. feat: 将 logind 会话 id 由 int 改为 string 存储 1. 将 Auth::xdgSessionId 与 openSession() 改为 QString，使 c1 这类非 数字会话 id 可端到端透传。 2. 父子进程间改为以「长度+字节」方式传递会话 id，取代原始 int。 3. 更新 SocketServer、Display 与 TreelandDisplayServer，以字符串收 发会话 id。 Log: 无用户可见变化 Influence: 1. 验证通过 ddm 登录后，普通用户会话的激活、锁定、解锁、注销均正常。 2. 验证切回登录界面（dde 会话 id 为空）不会崩溃或误路由会话。 3. 验证非数字 logind 会话 id 不再与 dde 登录界面哨兵会话冲突。 Fixes: linuxdeepin/treeland#1464
wineee
force-pushed
the
feat/session-id-string
branch
from
October 10, 2026 03:15
91ade20 to
89e59b7
Compare
zccrs
approved these changes
Oct 10, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee, zccrs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: No user-facing changes
Influence:
feat: 将 logind 会话 id 由 int 改为 string 存储
Log: 无用户可见变化
Influence:
Fixes: linuxdeepin/treeland#1464
Summary by Sourcery
Store logind session identifiers as strings throughout daemon, display, and greeter communication paths.
Bug Fixes:
Enhancements: