Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public String createManager(@Context GraphManager manager,
AuthManager authManager = manager.authManager();
validUser(authManager, user);

String creator = HugeGraphAuthProxy.getContext().user().username();
String creator = HugeGraphAuthProxy.username();
Copy link
Member

Choose a reason for hiding this comment

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

⚠️ Important: 全局一致性检查

建议搜索整个代码库中所有 HugeGraphAuthProxy.getContext().user().username() 的调用点,确保都已替换为新的 username() 方法,避免遗漏其他潜在的 NPE 风险点。

可以使用以下命令检查:

grep -r "HugeGraphAuthProxy.getContext().user().username()" --include="*.java"

switch (type) {
case SPACE:
validGraphSpace(manager, graphSpace);
Expand Down Expand Up @@ -124,7 +124,7 @@ public void delete(@Context GraphManager manager,
AuthManager authManager = manager.authManager();
validType(type);
validUser(authManager, user);
String actionUser = HugeGraphAuthProxy.getContext().user().username();
String actionUser = HugeGraphAuthProxy.username();

switch (type) {
case SPACE:
Expand Down Expand Up @@ -193,7 +193,7 @@ public String checkRole(@Context GraphManager manager,

validType(type);
AuthManager authManager = manager.authManager();
String user = HugeGraphAuthProxy.getContext().user().username();
String user = HugeGraphAuthProxy.username();

boolean result;
switch (type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public Object create(@Context GraphManager manager,
}
}

String creator = HugeGraphAuthProxy.getContext().user().username();
String creator = HugeGraphAuthProxy.username();

if (StringUtils.isNotEmpty(clone)) {
// Clone from existing graph
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public String create(@Context GraphManager manager,

jsonGraphSpace.checkCreate(false);

String creator = HugeGraphAuthProxy.getContext().user().username();
String creator = HugeGraphAuthProxy.username();
GraphSpace exist = manager.graphSpace(jsonGraphSpace.name);
E.checkArgument(exist == null, "The graph space '%s' has existed",
jsonGraphSpace.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ public static Context setAdmin() {
public static Context getContext() {
// Return task context first
String taskContext = TaskManager.getContext();

User user = User.fromJson(taskContext);
if (user != null) {
return new Context(user);
Expand Down Expand Up @@ -953,6 +954,14 @@ public void updateTime(Date updateTime) {
this.hugegraph.updateTime(updateTime);
}

public static String username() {
Copy link
Member

Choose a reason for hiding this comment

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

⚠️ Important: 缺少单元测试

这是一个 NPE 修复,应该添加单元测试来验证:

  1. 非认证模式下调用 username() 的行为
  2. getContext() 返回 null 时的处理
  3. 各个调用方(ManagerAPI, GraphsAPI, GraphSpaceAPI)在非认证模式下的正确性

建议添加测试类或测试方法覆盖这些场景。

Context context = HugeGraphAuthProxy.getContext();
Copy link
Member

Choose a reason for hiding this comment

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

⚠️ Important: 空指针安全性不完整

虽然修复了 context 为 null 的情况,但没有处理 context.user 为 null 的场景。建议增加防御性检查:

Suggested change
Context context = HugeGraphAuthProxy.getContext();
public static String username() {
Context context = HugeGraphAuthProxy.getContext();
if (context == null || context.user == null) {
return "anonymous";
}
return context.user.username();
}

或者检查一下 Context 的构造函数是否保证 user 字段非空。

if (context == null) {
return "anonymous";
}
return context.user.username();
Copy link
Member

Choose a reason for hiding this comment

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

‼️ Critical: 返回值设计问题

新增的 username() 方法在非认证模式下返回 "anonymous",但需要考虑:

  1. 业务逻辑正确性: 在 ManagerAPI.createManager()GraphSpaceAPI.create() 中,这个 username 被用作 creator 字段。如果在非认证模式下所有创建者都是 "anonymous",会导致无法追踪真实的资源创建者。

  2. 建议方案:

    • 如果非认证模式下不需要追踪创建者,应该返回 null 而不是 "anonymous",让调用方明确处理
    • 或者在调用方检查认证模式,非认证模式下使用其他标识(如 IP、session ID 等)
Suggested change
return context.user.username();
public static String username() {
Context context = HugeGraphAuthProxy.getContext();
if (context == null || context.user == null) {
return null; // 让调用方明确处理非认证场景
}
return context.user.username();
}

请确认非认证模式下的业务预期行为。

}

private <V> Cache<Id, V> cache(String prefix, long capacity,
long expiredTime) {
String name = prefix + "-" + this.hugegraph.spaceGraphName();
Expand Down
Loading