Fix PR review bugs, add SetUserSettings/GetUserSettings, remove rate limiting#14
Merged
Merged
Conversation
…ings/GetUserSettings, remove rate limiting Agent-Logs-Url: https://github.com/XMOJ-Script-dev/xmoj-bbs-v2/sessions/31a1a3f5-0f00-4a3b-bf0f-b60237192515 Co-authored-by: PythonSmall-Q <106425289+PythonSmall-Q@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
PythonSmall-Q
March 29, 2026 01:46
View session
Reviewer's GuideImplements two new authenticated user settings routes, updates database metadata to allow the new table, fixes missing AGPL headers and a silent failure in scheduled cleanup, and removes the legacy rate-limiting middleware. Sequence diagram for SetUserSettings routesequenceDiagram
actor UserClient
participant SetUserSettingsRoute
participant AuthContext
participant UserSettingsTable
UserClient->>SetUserSettingsRoute: HTTP POST /SetUserSettings (Data.Settings)
SetUserSettingsRoute->>SetUserSettingsRoute: CheckParams(Data)
SetUserSettingsRoute-->>UserClient: Result(false, "设置内容过大") if SettingsString length > 10000
SetUserSettingsRoute->>SetUserSettingsRoute: JSON.parse(SettingsString)
SetUserSettingsRoute-->>UserClient: Result(false, "设置格式有误") on parse or shape error
SetUserSettingsRoute->>AuthContext: auth.database.GetTableSize("user_settings", { user_id: auth.username })
AuthContext->>UserSettingsTable: COUNT rows for user_id
UserSettingsTable-->>AuthContext: TableSize
AuthContext-->>SetUserSettingsRoute: TableSize
alt TableSize === 0
SetUserSettingsRoute->>AuthContext: auth.database.Insert("user_settings", { user_id, settings })
AuthContext->>UserSettingsTable: INSERT user_settings row
UserSettingsTable-->>AuthContext: insert ok
else TableSize > 0
SetUserSettingsRoute->>AuthContext: auth.database.Update("user_settings", { settings }, { user_id })
AuthContext->>UserSettingsTable: UPDATE user_settings row
UserSettingsTable-->>AuthContext: update ok
end
SetUserSettingsRoute-->>UserClient: Result(true, "保存设置成功")
rect rgb(250,230,230)
SetUserSettingsRoute->>SetUserSettingsRoute: catch(error)
SetUserSettingsRoute->>AuthContext: Output.Error("SetUserSettings error: " + errorMsg)
SetUserSettingsRoute-->>UserClient: Result(false, "保存设置失败: " + errorMsg)
end
Sequence diagram for GetUserSettings routesequenceDiagram
actor UserClient
participant GetUserSettingsRoute
participant AuthContext
participant UserSettingsTable
UserClient->>GetUserSettingsRoute: HTTP GET /GetUserSettings
GetUserSettingsRoute->>AuthContext: auth.database.Select("user_settings", [settings], { user_id: auth.username })
AuthContext->>UserSettingsTable: SELECT settings WHERE user_id
UserSettingsTable-->>AuthContext: rows
AuthContext-->>GetUserSettingsRoute: SettingsData
alt SettingsData length === 0
GetUserSettingsRoute-->>UserClient: Result(true, "获得设置成功", { Settings: {} })
else SettingsData length > 0
GetUserSettingsRoute->>GetUserSettingsRoute: JSON.parse(SettingsData[0].settings)
GetUserSettingsRoute-->>UserClient: Result(false, "设置数据损坏") on parse or shape error
GetUserSettingsRoute-->>UserClient: Result(true, "获得设置成功", { Settings: SettingsObject }) on success
end
rect rgb(250,230,230)
GetUserSettingsRoute->>GetUserSettingsRoute: catch(error)
GetUserSettingsRoute->>AuthContext: Output.Error("GetUserSettings error: " + errorMsg)
GetUserSettingsRoute-->>UserClient: Result(false, "获得设置失败: " + errorMsg)
end
ER diagram for user_settings table additionerDiagram
user_settings {
string user_id
text settings
}
Flow diagram for scheduled cleanup error handling changeflowchart TD
A["Scheduled event trigger"] --> B["Create Database instance"]
B --> C["Define async cleanup function"]
subgraph CleanupLogic
C --> D["Delete old short_message rows"]
D --> E["Delete expired phpsessid rows"]
end
C --> F["context.waitUntil(cleanup().catch(...))"]
F --> G{"cleanup throws error?"}
G -->|No| H["Scheduled task completes silently"]
G -->|Yes| I["Output.Error('Scheduled cleanup failed: ' + message)"]
I --> H
File-Level Changes
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 3 issues, and left some high level feedback:
- In SetUserSettings, using GetTableSize to branch between Insert and Update is a bit indirect; consider using a select-then-branch or a helper/upsert-style abstraction that avoids the extra aggregation call and makes the intent (insert-or-update) clearer.
- Both SetUserSettings and GetUserSettings implement similar JSON parsing and object-shape validation for the settings payload; consider extracting this into a shared helper to keep validation logic consistent and reduce duplication.
- The routes currently include raw error messages (error.message) in user-facing Result messages, which can leak implementation details; consider returning a generic failure message to clients while logging the detailed error only via Output.Error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In SetUserSettings, using GetTableSize to branch between Insert and Update is a bit indirect; consider using a select-then-branch or a helper/upsert-style abstraction that avoids the extra aggregation call and makes the intent (insert-or-update) clearer.
- Both SetUserSettings and GetUserSettings implement similar JSON parsing and object-shape validation for the settings payload; consider extracting this into a shared helper to keep validation logic consistent and reduce duplication.
- The routes currently include raw error messages (error.message) in user-facing Result messages, which can leak implementation details; consider returning a generic failure message to clients while logging the detailed error only via Output.Error.
## Individual Comments
### Comment 1
<location path="server/routes/GetUserSettings.ts" line_range="35-41" />
<code_context>
+ return new Result(true, "获得设置成功", { Settings: {} });
+ }
+
+ let SettingsObject: object;
+ try {
+ SettingsObject = JSON.parse(SettingsData[0]["settings"]);
+ } catch (_) {
+ return new Result(false, "设置数据损坏");
+ }
+ if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) {
+ return new Result(false, "设置数据损坏");
+ }
</code_context>
<issue_to_address>
**suggestion:** Align the parsed settings type with a more precise structure instead of `object`.
Here you rely on `SettingsObject` being a non-null, non-array object, but it’s typed as `object`. Typing this as `unknown` (with runtime narrowing) or `Record<string, unknown>` would better reflect the checks you perform and keep the types aligned with the actual shape of the data.
Suggested implementation:
```typescript
let SettingsObject: unknown;
try {
SettingsObject = JSON.parse(SettingsData[0]["settings"]);
} catch (_) {
return new Result(false, "设置数据损坏");
}
if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) {
return new Result(false, "设置数据损坏");
}
const TypedSettingsObject = SettingsObject as Record<string, unknown>;
```
Anywhere later in this function where `SettingsObject` was used as an object, switch to `TypedSettingsObject` so that you benefit from the more precise `Record<string, unknown>` type. If you prefer to keep the original variable name, you can instead reassign with a type assertion, e.g. `SettingsObject = SettingsObject as Record<string, unknown>;` and adjust the code accordingly.
</issue_to_address>
### Comment 2
<location path="server/routes/SetUserSettings.ts" line_range="67-71" />
<code_context>
+ }
+
+ return new Result(true, "获得设置成功", { Settings: SettingsObject });
+ } catch (error) {
+ if (error instanceof Result) return error;
+ const errorMsg = error instanceof Error ? error.message : String(error);
</code_context>
<issue_to_address>
**🚨 issue (security):** Avoid leaking raw error messages to clients in the user settings API.
Right now `errorMsg` is sent back to the client, which can leak internal details (e.g., SQL errors, stack traces). Instead, keep the detailed message in server logs via `Output.Error` and return a generic client-facing error (optionally with an error code). Apply the same pattern in `GetUserSettings`.
</issue_to_address>
### Comment 3
<location path="server/routes/GetUserSettings.ts" line_range="46-50" />
<code_context>
+ }
+
+ return new Result(true, "获得设置成功", { Settings: SettingsObject });
+ } catch (error) {
+ if (error instanceof Result) return error;
+ const errorMsg = error instanceof Error ? error.message : String(error);
+ Output.Error("GetUserSettings error: " + errorMsg);
+ return new Result(false, "获得设置失败: " + errorMsg);
+ }
+});
</code_context>
<issue_to_address>
**🚨 issue (security):** Return a generic error message instead of propagating internal error text.
The raw `errorMsg` is still included in the client-facing `Result`. To avoid leaking internal error details, keep the full message only in `Output.Error` and return a stable, user-friendly error string or code instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+35
to
+41
| let SettingsObject: object; | ||
| try { | ||
| SettingsObject = JSON.parse(SettingsData[0]["settings"]); | ||
| } catch (_) { | ||
| return new Result(false, "设置数据损坏"); | ||
| } | ||
| if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) { |
There was a problem hiding this comment.
suggestion: Align the parsed settings type with a more precise structure instead of object.
Here you rely on SettingsObject being a non-null, non-array object, but it’s typed as object. Typing this as unknown (with runtime narrowing) or Record<string, unknown> would better reflect the checks you perform and keep the types aligned with the actual shape of the data.
Suggested implementation:
let SettingsObject: unknown;
try {
SettingsObject = JSON.parse(SettingsData[0]["settings"]);
} catch (_) {
return new Result(false, "设置数据损坏");
}
if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) {
return new Result(false, "设置数据损坏");
}
const TypedSettingsObject = SettingsObject as Record<string, unknown>;Anywhere later in this function where SettingsObject was used as an object, switch to TypedSettingsObject so that you benefit from the more precise Record<string, unknown> type. If you prefer to keep the original variable name, you can instead reassign with a type assertion, e.g. SettingsObject = SettingsObject as Record<string, unknown>; and adjust the code accordingly.
Comment on lines
+67
to
+71
| } catch (error) { | ||
| if (error instanceof Result) return error; | ||
| const errorMsg = error instanceof Error ? error.message : String(error); | ||
| Output.Error("SetUserSettings error: " + errorMsg); | ||
| return new Result(false, "保存设置失败: " + errorMsg); |
There was a problem hiding this comment.
🚨 issue (security): Avoid leaking raw error messages to clients in the user settings API.
Right now errorMsg is sent back to the client, which can leak internal details (e.g., SQL errors, stack traces). Instead, keep the detailed message in server logs via Output.Error and return a generic client-facing error (optionally with an error code). Apply the same pattern in GetUserSettings.
Comment on lines
+46
to
+50
| } catch (error) { | ||
| if (error instanceof Result) return error; | ||
| const errorMsg = error instanceof Error ? error.message : String(error); | ||
| Output.Error("GetUserSettings error: " + errorMsg); | ||
| return new Result(false, "获得设置失败: " + errorMsg); |
There was a problem hiding this comment.
🚨 issue (security): Return a generic error message instead of propagating internal error text.
The raw errorMsg is still included in the client-facing Result. To avoid leaking internal error details, keep the full message only in Output.Error and return a stable, user-friendly error string or code instead.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/routes/GetUserSettings.ts">
<violation number="1" location="server/routes/GetUserSettings.ts:50">
P2: Do not return raw internal exception messages to clients; return a generic failure message and keep details only in logs.</violation>
</file>
<file name="server/routes/SetUserSettings.ts">
<violation number="1" location="server/routes/SetUserSettings.ts:35">
P2: The settings size limit uses `String.length` instead of UTF-8 byte length, so multibyte JSON can exceed the intended 10 KB cap while passing validation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (error instanceof Result) return error; | ||
| const errorMsg = error instanceof Error ? error.message : String(error); | ||
| Output.Error("GetUserSettings error: " + errorMsg); | ||
| return new Result(false, "获得设置失败: " + errorMsg); |
There was a problem hiding this comment.
P2: Do not return raw internal exception messages to clients; return a generic failure message and keep details only in logs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/GetUserSettings.ts, line 50:
<comment>Do not return raw internal exception messages to clients; return a generic failure message and keep details only in logs.</comment>
<file context>
@@ -0,0 +1,52 @@
+ if (error instanceof Result) return error;
+ const errorMsg = error instanceof Error ? error.message : String(error);
+ Output.Error("GetUserSettings error: " + errorMsg);
+ return new Result(false, "获得设置失败: " + errorMsg);
+ }
+});
</file context>
| })); | ||
|
|
||
| const SettingsString: string = Data["Settings"]; | ||
| if (SettingsString.length > MAX_SETTINGS_LENGTH) { |
There was a problem hiding this comment.
P2: The settings size limit uses String.length instead of UTF-8 byte length, so multibyte JSON can exceed the intended 10 KB cap while passing validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/SetUserSettings.ts, line 35:
<comment>The settings size limit uses `String.length` instead of UTF-8 byte length, so multibyte JSON can exceed the intended 10 KB cap while passing validation.</comment>
<file context>
@@ -0,0 +1,73 @@
+ }));
+
+ const SettingsString: string = Data["Settings"];
+ if (SettingsString.length > MAX_SETTINGS_LENGTH) {
+ return new Result(false, "设置内容过大");
+ }
</file context>
There was a problem hiding this comment.
Pull request overview
This PR updates the server API by porting user settings routes from XMOJ-bbs, tightening operational behavior in the scheduled cleanup plugin, restoring AGPL headers in several route files, and removing the legacy rate-limiting middleware.
Changes:
- Added
SetUserSettings/GetUserSettingsroutes and allowed DB access to theuser_settingstable/columns. - Ensured scheduled cleanup errors are surfaced via logging instead of being silently swallowed.
- Restored full AGPL headers in multiple routes and removed
0.rate-limit.ts(plus lockfile cleanup).
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server/utils/database.ts | Whitelists user_settings table/columns for the DB abstraction. |
| server/routes/SetUserSettings.ts | New route to validate and persist per-user settings JSON. |
| server/routes/GetUserSettings.ts | New route to retrieve stored per-user settings JSON (or {}). |
| server/plugins/scheduled.ts | Refactors cleanup into a named function and logs errors via Output.Error. |
| server/middleware/0.rate-limit.ts | Removes auto-mounted rate limiting middleware. |
| server/routes/GetMailList.ts | Restores full AGPL header. |
| server/routes/GetMail.ts | Restores full AGPL header. |
| server/routes/GetBoards.ts | Restores full AGPL header. |
| server/routes/GetBadge.ts | Restores full AGPL header. |
| server/routes/GetBBSMentionList.ts | Restores full AGPL header. |
| server/routes/EditReply.ts | Restores full AGPL header. |
| server/routes/EditBadge.ts | Restores full AGPL header. |
| server/routes/DeletePost.ts | Restores full AGPL header. |
| package-lock.json | Removes sqlstring from the lockfile dependency tree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+49
to
+63
| const existingSize = ThrowErrorIfFailed( | ||
| await auth.database.GetTableSize("user_settings", { user_id: auth.username }) | ||
| )["TableSize"]; | ||
|
|
||
| if (existingSize === 0) { | ||
| ThrowErrorIfFailed(await auth.database.Insert("user_settings", { | ||
| user_id: auth.username, | ||
| settings: SettingsString | ||
| })); | ||
| } else { | ||
| ThrowErrorIfFailed(await auth.database.Update("user_settings", { | ||
| settings: SettingsString | ||
| }, { | ||
| user_id: auth.username | ||
| })); |
There was a problem hiding this comment.
This insert-then-update flow is not atomic: concurrent requests can both observe existingSize === 0 and insert duplicate rows for the same user_id. If user_id is meant to be unique, use a database-level UPSERT (e.g., INSERT ... ON CONFLICT(user_id) DO UPDATE) or another atomic strategy so the table cannot accumulate duplicates under concurrency.
Suggested change
| const existingSize = ThrowErrorIfFailed( | |
| await auth.database.GetTableSize("user_settings", { user_id: auth.username }) | |
| )["TableSize"]; | |
| if (existingSize === 0) { | |
| ThrowErrorIfFailed(await auth.database.Insert("user_settings", { | |
| user_id: auth.username, | |
| settings: SettingsString | |
| })); | |
| } else { | |
| ThrowErrorIfFailed(await auth.database.Update("user_settings", { | |
| settings: SettingsString | |
| }, { | |
| user_id: auth.username | |
| })); | |
| try { | |
| // Optimistically try to insert; if the row already exists (e.g., unique constraint on user_id), | |
| // the insert will fail and we fall back to an update below. | |
| ThrowErrorIfFailed( | |
| await auth.database.Insert("user_settings", { | |
| user_id: auth.username, | |
| settings: SettingsString | |
| }) | |
| ); | |
| } catch (_) { | |
| // On insert failure (such as a duplicate key), update the existing row instead. | |
| ThrowErrorIfFailed( | |
| await auth.database.Update( | |
| "user_settings", | |
| { | |
| settings: SettingsString | |
| }, | |
| { | |
| user_id: auth.username | |
| } | |
| ) | |
| ); |
|
|
||
| export default eventHandler(async (event) => { | ||
| try { | ||
| const { auth } = event.context; |
There was a problem hiding this comment.
This route assumes event.context.auth is always present. In server/middleware/1.auth.ts, the middleware can return early without setting auth (e.g., when the body is missing/not an object), which would cause a runtime error here and potentially leak internal error details. Add an explicit if (!auth || !auth.database) return new Result(false, "身份验证失败"); guard like other routes do.
Suggested change
| const { auth } = event.context; | |
| const { auth } = event.context; | |
| if (!auth || !auth.database) return new Result(false, "身份验证失败"); |
Comment on lines
+24
to
+66
| export default eventHandler(async (event) => { | ||
| try { | ||
| const { auth } = event.context; | ||
| const body = await readBody(event); | ||
| const { Data } = body || {}; | ||
|
|
||
| ThrowErrorIfFailed(CheckParams(Data, { | ||
| "Settings": "string" | ||
| })); | ||
|
|
||
| const SettingsString: string = Data["Settings"]; | ||
| if (SettingsString.length > MAX_SETTINGS_LENGTH) { | ||
| return new Result(false, "设置内容过大"); | ||
| } | ||
|
|
||
| let SettingsObject: object; | ||
| try { | ||
| SettingsObject = JSON.parse(SettingsString); | ||
| } catch (_) { | ||
| return new Result(false, "设置格式有误"); | ||
| } | ||
| if (typeof SettingsObject !== "object" || Array.isArray(SettingsObject) || SettingsObject === null) { | ||
| return new Result(false, "设置格式有误"); | ||
| } | ||
|
|
||
| const existingSize = ThrowErrorIfFailed( | ||
| await auth.database.GetTableSize("user_settings", { user_id: auth.username }) | ||
| )["TableSize"]; | ||
|
|
||
| if (existingSize === 0) { | ||
| ThrowErrorIfFailed(await auth.database.Insert("user_settings", { | ||
| user_id: auth.username, | ||
| settings: SettingsString | ||
| })); | ||
| } else { | ||
| ThrowErrorIfFailed(await auth.database.Update("user_settings", { | ||
| settings: SettingsString | ||
| }, { | ||
| user_id: auth.username | ||
| })); | ||
| } | ||
|
|
||
| return new Result(true, "保存设置成功"); |
There was a problem hiding this comment.
Missing test coverage for this new route's behaviors (JSON validation, size limit, insert vs update path, and return of stored settings). The repo already has Vitest route tests under tests/*Route.spec.ts, so adding similar specs for SetUserSettings (and the paired GetUserSettings) would help prevent regressions.
Comment on lines
+23
to
+29
| const { auth } = event.context; | ||
|
|
||
| const SettingsData: any[] = ThrowErrorIfFailed( | ||
| await auth.database.Select("user_settings", ["settings"], { | ||
| user_id: auth.username | ||
| }) | ||
| ); |
There was a problem hiding this comment.
This handler assumes event.context.auth is always set; if auth middleware returns early without populating it, auth.database.Select(...) will throw and the route will respond with an internal error message. Add an explicit if (!auth || !auth.database) return new Result(false, "身份验证失败"); guard before DB access.
Comment on lines
+35
to
+37
| let SettingsObject: object; | ||
| try { | ||
| SettingsObject = JSON.parse(SettingsData[0]["settings"]); |
There was a problem hiding this comment.
If user_settings ever contains more than one row for a user (possible with the non-atomic insert/update in SetUserSettings), this will return an arbitrary row (SettingsData[0]). Consider enforcing uniqueness at the DB level and/or adding Limit: 1 with a deterministic ordering, or handling multiple rows explicitly (e.g., pick latest).
Suggested change
| let SettingsObject: object; | |
| try { | |
| SettingsObject = JSON.parse(SettingsData[0]["settings"]); | |
| // Handle potential multiple rows deterministically by selecting a specific row. | |
| let selectedSettingsRow = SettingsData[0]; | |
| if (SettingsData.length > 1) { | |
| const sortedSettingsData = [...SettingsData].sort((a, b) => { | |
| const sa = typeof a.settings === "string" ? a.settings : JSON.stringify(a.settings); | |
| const sb = typeof b.settings === "string" ? b.settings : JSON.stringify(b.settings); | |
| return sa.localeCompare(sb); | |
| }); | |
| selectedSettingsRow = sortedSettingsData[sortedSettingsData.length - 1]; | |
| } | |
| let SettingsObject: object; | |
| try { | |
| SettingsObject = JSON.parse(selectedSettingsRow["settings"]); |
Comment on lines
+28
to
+34
| const { Data } = body || {}; | ||
|
|
||
| ThrowErrorIfFailed(CheckParams(Data, { | ||
| "Settings": "string" | ||
| })); | ||
|
|
||
| const SettingsString: string = Data["Settings"]; |
There was a problem hiding this comment.
body/Data may be undefined here (e.g., malformed request), and CheckParams(Data, ...) will throw a TypeError before it can return a user-friendly Result. Consider validating body?.Data is a plain object (or defaulting Data to {}) and returning a parameter error when missing.
Suggested change
| const { Data } = body || {}; | |
| ThrowErrorIfFailed(CheckParams(Data, { | |
| "Settings": "string" | |
| })); | |
| const SettingsString: string = Data["Settings"]; | |
| // Validate that body and body.Data are plain objects before using them | |
| if (typeof body !== "object" || body === null || | |
| typeof (body as any).Data !== "object" || | |
| (body as any).Data === null || | |
| Array.isArray((body as any).Data) | |
| ) { | |
| return new Result(false, "参数错误"); | |
| } | |
| const { Data } = body as { Data: Record<string, unknown> }; | |
| ThrowErrorIfFailed(CheckParams(Data, { | |
| "Settings": "string" | |
| })); | |
| const SettingsString: string = Data["Settings"] as string; |
Comment on lines
+22
to
+37
| const MAX_SETTINGS_LENGTH = 10000; | ||
|
|
||
| export default eventHandler(async (event) => { | ||
| try { | ||
| const { auth } = event.context; | ||
| const body = await readBody(event); | ||
| const { Data } = body || {}; | ||
|
|
||
| ThrowErrorIfFailed(CheckParams(Data, { | ||
| "Settings": "string" | ||
| })); | ||
|
|
||
| const SettingsString: string = Data["Settings"]; | ||
| if (SettingsString.length > MAX_SETTINGS_LENGTH) { | ||
| return new Result(false, "设置内容过大"); | ||
| } |
There was a problem hiding this comment.
The PR description says settings are limited to �10 KB, but this uses SettingsString.length (character count), which can exceed 10 KB in UTF-8 (e.g., emoji/CJK). Prefer a byte-length check (e.g., TextEncoder().encode(SettingsString).length) or use CheckParams' maxBytes support to enforce the real 10 KB limit.
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.
Addresses unresolved review comments from PR #4, ports two missing routes from
XMOJ-Script-dev/XMOJ-bbs, and removes the rate limiting middleware.Bug Fixes
/* Copyright header omitted */placeholder with the full AGPL header in 8 route files (DeletePost,GetBoards,GetBBSMentionList,EditBadge,GetMail,GetBadge,EditReply,GetMailList)scheduled.tssilent failure: ThewaitUntilpromise could silently swallow errors. Extracted cleanup to a named async function with.catch()logging:New Features (ported from XMOJ-bbs)
SetUserSettings— Persists authenticated user's settings as a validated JSON object (≤10 KB) via insert-or-update onuser_settingsGetUserSettings— Returns the stored settings object for the authenticated user, or{}if none saveddatabase.ts: Addeduser_settings(user_id,settings) toALLOWED_TABLES/ALLOWED_COLUMNSRate Limiting
server/middleware/0.rate-limit.tsentirelySummary by Sourcery
Fix server-side issues, add persistent user settings APIs, and remove request rate limiting middleware.
New Features:
Bug Fixes:
Enhancements:
Summary by cubic
Fixes review feedback, adds
SetUserSettings/GetUserSettingsto persist per‑user settings, and removes the request rate‑limiting middleware. Improves scheduled cleanup reliability and ensures proper AGPL headers.New Features
SetUserSettings: save validated JSON (≤10 KB) for the authenticated user via insert-or-update onuser_settings.GetUserSettings: return stored settings or{}if none; updateduser_settingstable/columns allowlist.Bug Fixes
server/plugins/scheduled.tsby moving work intocleanup()and logging errors withOutput.Error.Written for commit 5433239. Summary will update on new commits.