Skip to content

Fix/pr 2900 #2998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Calcium-Ion merged 2 commits into from
Feb 24, 2026
Merged

Fix/pr 2900 #2998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5d47da
feat: allow custom username for new users
feitianbubu Feb 9, 2026
af31935
fix: check oauthUser.Username length
seefs001 Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions controller/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,16 @@ func findOrCreateOAuthUser(c *gin.Context, provider oauth.Provider, oauthUser *o

// Set up new user
user.Username = provider.GetProviderPrefix() + strconv.Itoa(model.GetMaxUserId()+1)

if oauthUser.Username != "" {
if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err == nil && !exists {
// 防止索引退化
if len(oauthUser.Username) <= model.UserNameMaxLength {
user.Username = oauthUser.Username
}
}
}
Comment on lines +241 to +248

@coderabbitai coderabbitai Bot Feb 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

go-playground validator v10 max tag strings rune count utf8

💡 Result:

In go-playground/validator/v10, the max tag (and min/len/gt/gte/lt/lte on strings) measures string length in Unicode code points (runes), not bytes:

  • max is implemented as hasMaxOf → isLte [1].
  • For reflect.String, isLte compares int64(utf8.RuneCountInString(field.String())) <= p [1].
  • The public docs also describe min (and by extension max) for strings as “number of characters” [2].

Notes:

  • This is rune count, not grapheme-cluster/user-perceived “characters” (some emojis/combining sequences can count as >1 rune).
  • It does not enforce “valid UTF-8”; it just counts using utf8.RuneCountInString semantics.

Sources: [1] baked-in implementation (hasMaxOf/isLte uses utf8.RuneCountInString) https://docs-go.hexacode.org/src/github.com/go-playground/validator/v10/baked_in.go, [2] pkg.go.dev docs for min on strings (“characters”) https://pkg.go.dev/github.com/go-playground/validator/v10

🏁 Script executed:

# Find the oauth.go file and examine the context around lines 241-248
find . -name "oauth.go" -type f

Repository: QuantumNous/new-api

Length of output: 84

🏁 Script executed:

# Search for model.UserNameMaxLength definition
rg "UserNameMaxLength" --type go

Repository: QuantumNous/new-api

Length of output: 184

🏁 Script executed:

# Check imports in controller/oauth.go to see current JSON handling patterns
head -50 controller/oauth.go

Repository: QuantumNous/new-api

Length of output: 1316

🏁 Script executed:

# Search for oauthUser struct definition and its validator tags
rg "type.*OAuthUser" --type go -A 10

Repository: QuantumNous/new-api

Length of output: 1138

🏁 Script executed:

# Get more context around lines 241-248 in controller/oauth.go
sed -n '230,260p' controller/oauth.go

Repository: QuantumNous/new-api

Length of output: 894

🏁 Script executed:

# Find CheckUserExistOrDeleted function to understand what errors it returns
rg "func.*CheckUserExistOrDeleted" --type go -A 15

Repository: QuantumNous/new-api

Length of output: 816

🏁 Script executed:

# Check User struct definition and validator tags
rg "type User struct" --type go -A 30 model/user.go

Repository: QuantumNous/new-api

Length of output: 2899

🏁 Script executed:

# Check if validator tags reference UserNameMaxLength elsewhere
rg "validate.*UserNameMaxLength|UserNameMaxLength.*validate" --type go

Repository: QuantumNous/new-api

Length of output: 45

🏁 Script executed:

# Check the full context - where does oauthUser get validated before reaching this code?
sed -n '220,245p' controller/oauth.go

Repository: QuantumNous/new-api

Length of output: 1035

Use utf8.RuneCountInString() instead of len() to count username length consistently with the validator constraint.

The User.Username field has a validate:"max=20" constraint that counts UTF-8 runes (not bytes), but this code checks len(oauthUser.Username) <= model.UserNameMaxLength which counts bytes. A 15-character CJK username (45 bytes but only 15 runes) would pass validation in a normal registration flow but get silently rejected here, falling back to an auto-generated username.

Additionally, when CheckUserExistOrDeleted returns an error, it is currently silently swallowed without logging. This makes transient database failures invisible.

Proposed fix 
+import "unicode/utf8"

 if oauthUser.Username != "" {
     if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err == nil && !exists {
-        if len(oauthUser.Username) <= model.UserNameMaxLength {
+        if utf8.RuneCountInString(oauthUser.Username) <= model.UserNameMaxLength {
             user.Username = oauthUser.Username
         }
+    } else if err != nil {
+        common.SysLog(fmt.Sprintf("[OAuth] failed to check username existence for %q: %v", oauthUser.Username, err))
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if oauthUser.Username != "" {
if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err == nil && !exists {
// 防止索引退化
if len(oauthUser.Username) <= model.UserNameMaxLength {
user.Username = oauthUser.Username
}
}
}
if oauthUser.Username != "" {
if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err == nil && !exists {
// 防止索引退化
if utf8.RuneCountInString(oauthUser.Username) <= model.UserNameMaxLength {
user.Username = oauthUser.Username
}
} else if err != nil {
common.SysLog(fmt.Sprintf("[OAuth] failed to check username existence for %q: %v", oauthUser.Username, err))
}
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@controller/oauth.go` around lines 241 - 248, The username length check uses
len() (bytes) so multi-byte UTF-8 names can be wrongly rejected and DB errors
from CheckUserExistOrDeleted are swallowed; replace len(oauthUser.Username) <=
model.UserNameMaxLength with utf8.RuneCountInString(oauthUser.Username) <=
model.UserNameMaxLength to count runes consistently with the validator, and when
calling model.CheckUserExistOrDeleted(...) propagate or at minimum log the
returned error (use the existing logger) instead of ignoring it so transient DB
errors are visible; keep the surrounding logic that assigns user.Username =
oauthUser.Username when the name is valid and not existing/deleted.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find and examine the actual code in controller/oauth.go
fd -type f -name "oauth.go" -path "*/controller/*" --exec cat -n {} \; | head -300

Repository: QuantumNous/new-api

Length of output: 233

🏁 Script executed:

# Search for UserNameMaxLength definition
rg "UserNameMaxLength" -B 2 -A 2

Repository: QuantumNous/new-api

Length of output: 571

🏁 Script executed:

# Check if go-playground/validator is used in the codebase
rg "github.com/go-playground/validator" -n

Repository: QuantumNous/new-api

Length of output: 638

🌐 Web query:

go-playground/validator v10 max constraint string rune count implementation

💡 Result:

In go-playground/validator/v10 (e.g., v10.30.1), the max tag for strings is implemented as hasMaxOf, which simply delegates to isLte (“less-than-or-equal”). isLte measures string length using utf8.RuneCountInString(field.String()), i.e. rune/code-point count, not byte length (and not grapheme-cluster/user-perceived character count). max and lte therefore behave the same for strings. [1]

The package docs also describe max for strings as “string length is at most that number of characters.” [2]

Sources
[1] baked_in.go (hasMaxOf → isLte; isLte uses utf8.RuneCountInString for strings) (raw.githubusercontent.com)
[2] Package documentation for max (pkg.go.dev)

Citations:

🏁 Script executed:

# Find User struct definition and Username field tag
rg "type User struct" -A 50 | head -80

Repository: QuantumNous/new-api

Length of output: 3986

🏁 Script executed:

# Search for Username field with validation tags
rg "Username.*validate" -n

Repository: QuantumNous/new-api

Length of output: 168

🏁 Script executed:

# Check CheckUserExistOrDeleted function and error handling patterns
rg "CheckUserExistOrDeleted" -B 2 -A 3

Repository: QuantumNous/new-api

Length of output: 1253

🏁 Script executed:

# Search for other uses of len() on username fields in oauth.go
rg "len\(.*Username\)" -n

Repository: QuantumNous/new-api

Length of output: 198

Use utf8.RuneCountInString() instead of len() to match validator v10's max constraint, and log errors from CheckUserExistOrDeleted().

The Username field has validate:"max=20" which measures rune count via utf8.RuneCountInString(). However, the guard at line 244 uses len(), which counts bytes. For multi-byte usernames (e.g., 15 CJK characters = 45 bytes), this incorrectly rejects valid usernames that would pass validation.

Additionally, errors from CheckUserExistOrDeleted() are silently discarded, making DB failures invisible. Other parts of the codebase (e.g., controller/user.go) properly log these errors with common.SysLog().

Proposed fix 
+import "unicode/utf8"

 if oauthUser.Username != "" {
-    if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err == nil && !exists {
-        // 防止索引退化
-        if len(oauthUser.Username) <= model.UserNameMaxLength {
-            user.Username = oauthUser.Username
-        }
-    }
+    if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err != nil {
+        common.SysLog(fmt.Sprintf("[OAuth] failed to check username %q: %v", oauthUser.Username, err))
+    } else if !exists && utf8.RuneCountInString(oauthUser.Username) <= model.UserNameMaxLength {
+        user.Username = oauthUser.Username
+    }
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@controller/oauth.go` around lines 241 - 248, Replace the byte-length check
with a rune-length check and surface DB errors: call
utf8.RuneCountInString(oauthUser.Username) instead of len(oauthUser.Username)
when comparing to model.UserNameMaxLength, and when invoking
model.CheckUserExistOrDeleted(oauthUser.Username, "") handle the returned error
by logging it via common.SysLog(...) before proceeding (only set user.Username =
oauthUser.Username when no error and exists==false and the rune-count check
passes).


if oauthUser.DisplayName != "" {
user.DisplayName = oauthUser.DisplayName
} else if oauthUser.Username != "" {
Expand Down Expand Up @@ -295,12 +305,12 @@ func findOrCreateOAuthUser(c *gin.Context, provider oauth.Provider, oauthUser *o
// Set the provider user ID on the user model and update
provider.SetProviderUserID(user, oauthUser.ProviderUserID)
if err := tx.Model(user).Updates(map[string]interface{}{
"github_id": user.GitHubId,
"discord_id": user.DiscordId,
"oidc_id": user.OidcId,
"linux_do_id": user.LinuxDOId,
"wechat_id": user.WeChatId,
"telegram_id": user.TelegramId,
"github_id": user.GitHubId,
"discord_id": user.DiscordId,
"oidc_id": user.OidcId,
"linux_do_id": user.LinuxDOId,
"wechat_id": user.WeChatId,
"telegram_id": user.TelegramId,
}).Error; err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions model/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"gorm.io/gorm"
)

const UserNameMaxLength = 20

// User if you add sensitive fields, don't forget to clean them in setupLogin function.
// Otherwise, the sensitive information will be saved on local storage in plain text!
type User struct {
Expand Down