lion/lionwebsite-backend
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

修复越权删除、畸形链接 500 及两处 NPE

Browse Source 
修复上一提交的测试所发现的缺陷。

1) 越权删除（安全，最严重）：deleteGalleryByGid 的授权条件是
   `!(collector.isEmpty() || collector.size()==1 && ...)`。当画廊无任何收藏时
   collector.isEmpty() 使整个条件短路放行，下载者身份完全未校验，
   任意有效授权码用户都能删除他人任务记录（无收藏正是最常见情形）。
   改为与提示文案一致的判定：被别人收藏 或 请求者不是下载人 即拒绝。

2) 被拒请求仍删除节点文件且对外显示成功：remoteService.deleteGallery 原先位于
   授权判断之外，即使拒绝也会向节点下发删除指令；且 switch 中 case 0 的
   response.success() 会覆盖先前的 failure。现改为授权失败即提前返回。

3) 畸形链接导致 500：link.split("/")[4] 段数不足抛 ArrayIndexOutOfBoundsException，
   而只捕获 NumberFormatException；项目无 @ControllerAdvice，异常穿透为 500。
   抽出 parseGidFromLink() 统一把 null/段数不足/非数字转成业务失败。

4) Response.isSuccess()/getData()/get() 在键缺失时抛 NPE，改为安全返回
   （isSuccess 视未设置为失败）。deleteGalleryByGid 也显式处理节点无响应(-1)，
   原先该分支不设置 result，末尾 response.get("result") 会 NPE。

5) getWeekUsedAmount() 在配置行缺失/值非法时给默认 0，不再 NPE。

验证：新增/改写回归用例后 143 项测试全过；并用真实数据做了新旧对照实测——
取 downloader=4 且无收藏的任务，以 id=25 用户的授权码删除：
  旧 jar：数据库记录被删（count 0），随后 500；
  新 jar：记录保留（count 1），返回「你不是下载人」并被拒。
覆盖率的 GalleryManageService 由 40.1% 升至 45.8%。
This commit is contained in:
root
2026-09-14 15:23:42 +08:00
parent 213ed4e7f6
commit 576092578d
4 changed files with 216 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/com/lion/lionwebsite/Service/GalleryManageService.java
+61 -12
View File
@@ -60,14 +60,20 @@ public class GalleryManageService {
User user = userMapper.selectUserByAuthCode(AuthCode);
// return Response._failure("调试中，请勿提交任务");
int gid;
try {
gid = Integer.parseInt(link.split("/")[4]);
} catch (NumberFormatException e) {
if (user == null) {
response.failure("授权码无效");
return response.toJSONString();
}
// 段数不足会先抛 ArrayIndexOutOfBoundsException，非数字段抛 NumberFormatException；
// 只捕后者会让畸形链接穿透为 500（本项目无 @ControllerAdvice）。
Integer parsedGid = parseGidFromLink(link);
if (parsedGid == null) {
response.failure("链接错误");
pushService.taskCreateReport(user.getUsername(), "未知任务", response);
return response.toJSONString();
}
int gid = parsedGid;
String taskName = "任务 [" + gid + "]";
if (remoteService.isDead()) {
@@ -126,6 +132,23 @@ public class GalleryManageService {
return response.toJSONString();
}
/**
* 从任务链接中安全提取 gid。任何畸形输入（null、段数不足、非数字）都返回 null，
* 由调用方转成业务失败，避免异常穿透为 500。
*/
static Integer parseGidFromLink(String link) {
if (link == null)
return null;
String[] segments = link.split("/");
if (segments.length <= 4)
return null;
try {
return Integer.parseInt(segments[4]);
} catch (NumberFormatException e) {
return null;
}
}
/**
* 尝试重新连接
* @return 重连结果
@@ -284,21 +307,35 @@ public class GalleryManageService {
response.failure("删除失败，该图片不存在");
return response.toJSONString();
}
if (user == null) {
response.failure("删除失败，授权码无效");
return response.toJSONString();
}
ArrayList<Integer> collector = collectMapper.selectCollectorByGid(gallery.getGid());
if (!(collector.isEmpty() || collector.size() == 1 && collector.getFirst().equals(user.getId()) //判断收藏
&& gallery.getDownloader() == user.getId())) //判断下载
// 拒绝条件与提示文案一致：被别人收藏，或者请求者不是下载人。
// 注意不能写成 collector.isEmpty() || ...：那样在「无任何收藏」时会短路放行，
// 从而完全跳过下载者校验，导致任何有效授权码都能删除他人任务。
boolean collectedByOthers = collector.stream().anyMatch(id -> id != user.getId());
boolean isDownloader = gallery.getDownloader() == user.getId();
if (collectedByOthers || !isDownloader) {
response.failure("删除失败，该图片已被别人收藏或你不是下载人");
else {
log.info("删除图片{}", gallery.getName());
galleryMapper.deleteGalleryByGid(gallery.getGid()); //删除图片记录
log.info("拒绝删除 gid={}：collectedByOthers={} isDownloader={}", gid, collectedByOthers, isDownloader);
return response.toJSONString();
}
// 通过授权后才落库并通知节点，避免被拒请求仍删除节点文件。
log.info("删除图片{}", gallery.getName());
galleryMapper.deleteGalleryByGid(gallery.getGid());
switch (remoteService.deleteGallery(gallery)) {
case ErrorCode.IO_ERROR -> response.failure("图片:" + gallery.getName() + "删除失败，IO错误");
case ErrorCode.FILE_NOT_FOUND -> response.failure("图片:" + gallery.getName() + "删除失败，文件不存在");
case 0 -> response.success();
// 节点无响应/超时会返回 -1 等非枚举值；必须显式判失败，
// 否则 result 保持未设置，末尾的 response.get("result") 会抛 NPE。
default -> response.failure("图片:" + gallery.getName() + "删除失败，节点无响应");
}
if (response.get("result").equals("failure"))
if (!response.isSuccess())
log.info(response.getData());
return response.toJSONString();
}
@@ -313,8 +350,20 @@ public class GalleryManageService {
CustomConfiguration lastResetAmountTime = configurationMapper.selectConfiguration(CustomConfiguration.LAST_RESET_AMOUNT_TIME);
Map<String, String> data = new HashMap<>();
data.put("weekUsedAmount", CustomUtil.fileSizeToString(Long.parseLong(weekUsedAmount.getValue())));
data.put("lastResetAmountTime", lastResetAmountTime.getValue());
// 配置行缺失时给出默认值，避免 NPE 让用量接口整体不可用。
String usedValue = weekUsedAmount == null || weekUsedAmount.getValue() == null
? "0" : weekUsedAmount.getValue();
long used;
try {
used = Long.parseLong(usedValue);
} catch (NumberFormatException e) {
log.warn("每周用量配置值非法，按 0 处理: {}", usedValue);
used = 0L;
}
data.put("weekUsedAmount", CustomUtil.fileSizeToString(used));
data.put("lastResetAmountTime",
lastResetAmountTime == null || lastResetAmountTime.getValue() == null
? "" : lastResetAmountTime.getValue());
response.success(new ObjectMapper().valueToTree(data).toString());
return response.toJSONString();
src/main/java/com/lion/lionwebsite/Util/Response.java
+7 -3
View File
@@ -19,7 +19,8 @@ public class Response {
}
public String get(String key){
return result.get(key).asText();
JsonNode node = result.get(key);
return node == null ? null : node.asText();
}
public void setData(String data){
@@ -57,7 +58,8 @@ public class Response {
}
public String getData(){
return result.get("data").asText();
JsonNode node = result.get("data");
return node == null ? null : node.asText();
}
/**
@@ -69,8 +71,10 @@ public class Response {
return getData();
}
/** 未设置 result 键时视为失败，而不是抛 NPE。 */
public boolean isSuccess(){
return result.get("result").asText().equals("success");
JsonNode node = result.get("result");
return node != null && "success".equals(node.asText());
}
src/test/java/com/lion/lionwebsite/Service/GalleryManageServiceTest.java
+135 -37
View File
@@ -59,18 +59,47 @@ class GalleryManageServiceTest {
}
/**
* ⚠️ 发现（未擅自修改生产逻辑）：段数不足的链接会抛 ArrayIndexOutOfBoundsException。
* createTask 只捕获 NumberFormatException（`link.split("/")[4]` 在段数不足时先抛越界），
* 且项目没有 @ControllerAdvice，因此异常会穿透为 500。
* Controller 层只校验了 null，未校验格式，故该路径可由外部构造的 link 触发。
* 这里记录现状；是否收紧需人工决策。
* 回归：修复前 `link.split("/")[4]` 在段数不足时抛 ArrayIndexOutOfBoundsException，
* 而只捕获 NumberFormatException，且项目无 @ControllerAdvice，会穿透为 500。
* 现在统一转成「链接错误」业务失败。
*/
@Test
void shortLinkThrowsIndexOutOfBoundsInsteadOfFailingGracefully() {
assertThrows(ArrayIndexOutOfBoundsException.class,
() -> service.createTask("https://example.org/g", "original", "code"),
"若此断言失败说明已修复为友好报错，应同步更新该测试");
void shortLinkIsRejectedGracefully() {
String response = assertDoesNotThrow(
() -> service.createTask("https://example.org/g", "original", "code"));
assertTrue(response.contains("链接错误"), "应友好提示链接错误: " + response);
assertFalse(response.contains("\"result\":\"success\""));
verify(galleries, never()).insertGallery(any());
verify(remote, never()).addGalleryToQueue(any());
}
@Test
void nullLinkIsRejectedGracefully() {
String response = assertDoesNotThrow(() -> service.createTask(null, "original", "code"));
assertFalse(response.contains("\"result\":\"success\""));
verify(galleries, never()).insertGallery(any());
}
/** 无效授权码应在解析链接之前就被拒，避免后续 user.getUsername() NPE。 */
@Test
void unknownAuthCodeIsRejectedBeforeParsing() {
User unknown = null;
when(users.selectUserByAuthCode("bogus")).thenReturn(unknown);
String response = assertDoesNotThrow(
() -> service.createTask("https://example.org/g/123/key/", "original", "bogus"));
assertFalse(response.contains("\"result\":\"success\""));
verify(galleries, never()).insertGallery(any());
}
/** parseGidFromLink 的边界：合法/非法输入都应安全返回。 */
@Test
void parseGidFromLinkHandlesMalformedInput() {
assertEquals(123, GalleryManageService.parseGidFromLink("https://exhentai.org/g/123/key/"));
assertNull(GalleryManageService.parseGidFromLink(null));
assertNull(GalleryManageService.parseGidFromLink(""));
assertNull(GalleryManageService.parseGidFromLink("https://example.org/g"));
assertNull(GalleryManageService.parseGidFromLink("https://example.org/a/b/c"));
assertNull(GalleryManageService.parseGidFromLink("https://example.org/g/not-a-number/key/"));
}
/** 节点离线时必须明确告知用户，且不落库。 */
@@ -140,38 +169,32 @@ class GalleryManageServiceTest {
}
/**
* ⚠️ 安全发现（未擅自修改生产逻辑）：当该画廊没有任何收藏记录时，
* collectMapper.selectCollectorByGid 返回空列表，授权判断
* `!(collector.isEmpty() || ...)` 中 collector.isEmpty() 直接为真，
* 使整个条件短路放行，**下载者身份完全未被校验**。
* 即：任意持有有效授权码的用户都能删除他人的任务记录。
* 无收藏正是最常见的情形，故影响面不小。
* 回归：修复前当画廊无任何收藏时，`collector.isEmpty()` 使授权条件短路放行，
* 下载者身份完全未校验，任意有效授权码用户可删除他人任务。现已补上下载者校验。
*/
@Test
void deleteWithoutCollectorsSkipsDownloaderCheck() {
void deleteWithoutCollectorsStillEnforcesDownloaderCheck() {
Gallery gallery = new Gallery();
gallery.setGid(888);
gallery.setName("other-user-task [888]");
gallery.setDownloader(999); // 属于别的用户
when(galleries.selectGalleryByGid(888)).thenReturn(gallery);
when(collectMapper.selectCollectorByGid(888)).thenReturn(new java.util.ArrayList<>());
when(remote.deleteGallery(any())).thenReturn((byte) 0);
String response = service.deleteGalleryByGid(888, "code"); // 请求者是 id=7
assertTrue(response.contains("\"result\":\"success\""),
"当前实现会放行；若此断言失败说明已补上下载者校验，应同步更新该测试");
verify(galleries).deleteGalleryByGid(888);
assertFalse(response.contains("\"result\":\"success\""), "非下载者删除必须被拒: " + response);
verify(galleries, never()).deleteGalleryByGid(anyInt());
verify(remote, never()).deleteGallery(any());
}
/**
* ⚠️ 发现（未擅自修改生产逻辑）：remoteService.deleteGallery(gallery) 位于授权判断的
* if/else **之外**，因此即使授权判定为「拒绝」，仍会向存储节点下发删除文件指令。
* 同时 switch 中的 `case 0 -> response.success()` 会覆盖前面写入的 failure，
* 使被拒的请求对外表现为成功。
* 回归：修复前 remoteService.deleteGallery 位于授权判断之外，
* 被拒请求仍会向节点下发删除指令，且 case 0 的 success 会覆盖 failure。
* 现在授权失败即提前返回，既不落库也不通知节点。
*/
@Test
void deniedDeleteStillNotifiesNodeAndReportsSuccess() {
void deniedDeleteDoesNotTouchDatabaseOrNode() {
Gallery gallery = new Gallery();
gallery.setGid(889);
gallery.setName("collected-by-other [889]");
@@ -180,14 +203,77 @@ class GalleryManageServiceTest {
// 有他人收藏 -> 授权应被拒
when(collectMapper.selectCollectorByGid(889))
.thenReturn(new java.util.ArrayList<>(java.util.List.of(999)));
when(remote.deleteGallery(any())).thenReturn((byte) 0);
String response = service.deleteGalleryByGid(889, "code");
verify(galleries, never()).deleteGalleryByGid(889); // 数据库记录确实没删
verify(remote).deleteGallery(any()); // 但删除指令仍下发了
assertTrue(response.contains("\"result\":\"success\""),
"当前实现会把拒绝结果覆盖为 success；若此断言失败说明已修复，应同步更新该测试");
assertFalse(response.contains("\"result\":\"success\""), "被他人收藏时删除必须被拒");
assertTrue(response.contains("别人收藏") || response.contains("不是下载人"),
"应给出与判定一致的提示: " + response);
verify(galleries, never()).deleteGalleryByGid(anyInt());
verify(remote, never()).deleteGallery(any());
}
/** 无收藏且本人是下载者：正常放行。 */
@Test
void deleteAllowsOwnerWhenNoCollectors() {
Gallery gallery = new Gallery();
gallery.setGid(893);
gallery.setName("mine-no-collect [893]");
gallery.setDownloader(7);
when(galleries.selectGalleryByGid(893)).thenReturn(gallery);
when(collectMapper.selectCollectorByGid(893)).thenReturn(new java.util.ArrayList<>());
when(remote.deleteGallery(any())).thenReturn((byte) 0);
String response = service.deleteGalleryByGid(893, "code");
assertTrue(response.contains("\"result\":\"success\""), "本人任务应可删除: " + response);
verify(galleries).deleteGalleryByGid(893);
}
/** 只有本人收藏时，本人可删除。 */
@Test
void deleteAllowsOwnerWhenOnlySelfCollected() {
Gallery gallery = new Gallery();
gallery.setGid(894);
gallery.setName("mine-self-collect [894]");
gallery.setDownloader(7);
when(galleries.selectGalleryByGid(894)).thenReturn(gallery);
when(collectMapper.selectCollectorByGid(894))
.thenReturn(new java.util.ArrayList<>(java.util.List.of(7)));
when(remote.deleteGallery(any())).thenReturn((byte) 0);
String response = service.deleteGalleryByGid(894, "code");
assertTrue(response.contains("\"result\":\"success\""), "仅本人收藏应可删除: " + response);
verify(galleries).deleteGalleryByGid(894);
}
/** 节点无响应（-1）必须如实报失败，不能被当成成功。 */
@Test
void deleteReportsFailureWhenNodeDoesNotRespond() {
Gallery gallery = new Gallery();
gallery.setGid(895);
gallery.setName("mine [895]");
gallery.setDownloader(7);
when(galleries.selectGalleryByGid(895)).thenReturn(gallery);
when(collectMapper.selectCollectorByGid(895)).thenReturn(new java.util.ArrayList<>());
when(remote.deleteGallery(any())).thenReturn((byte) -1);
String response = service.deleteGalleryByGid(895, "code");
assertFalse(response.contains("\"result\":\"success\""), "节点无响应不应报成功: " + response);
assertTrue(response.contains("节点无响应"), "应说明节点无响应: " + response);
}
/** 授权码无效（查不到用户）应被拒，且不得触发越权判定所需的空指针。 */
@Test
void deleteRejectsUnknownAuthCode() {
Gallery gallery = new Gallery();
gallery.setGid(896);
gallery.setDownloader(7);
when(galleries.selectGalleryByGid(896)).thenReturn(gallery);
String response = service.deleteGalleryByGid(896, "bogus-code");
assertFalse(response.contains("\"result\":\"success\""), "无效授权码应被拒: " + response);
verify(galleries, never()).deleteGalleryByGid(anyInt());
verify(remote, never()).deleteGallery(any());
}
/** 本人任务删除应放行并调用删除。 */
@@ -310,14 +396,26 @@ class GalleryManageServiceTest {
assertTrue(response.contains("2026-09-14 14:23:58"), "应带上次重置时间: " + response);
}
/**
* ⚠️ 发现：配置行缺失时 getWeekUsedAmount 会 NPE（readWeekUsedAmount 未做空值判断）。
* 正常部署下这两行由初始化脚本写入，故未暴露；此处记录现状。
*/
/** 回归：配置行缺失时给出默认值，不再 NPE，保证用量接口始终可用。 */
@Test
void weekUsedAmountThrowsWhenConfigRowsMissing() {
void weekUsedAmountToleratesMissingConfigRows() {
when(configurationMapper.selectConfiguration(anyString())).thenReturn(null);
assertThrows(NullPointerException.class, service::getWeekUsedAmount,
"若此断言失败说明已补空值保护，应同步更新该测试");
String response = assertDoesNotThrow(service::getWeekUsedAmount);
assertTrue(response.contains("\"result\":\"success\""), "应成功返回默认值: " + response);
assertTrue(response.contains("0B"), "缺失时用量应为 0B: " + response);
}
/** 配置值非法（非数字）时按 0 处理，不抛异常。 */
@Test
void weekUsedAmountToleratesMalformedValue() {
com.lion.lionwebsite.Domain.CustomConfiguration used =
new com.lion.lionwebsite.Domain.CustomConfiguration();
used.setValue("not-a-number");
when(configurationMapper.selectConfiguration(
com.lion.lionwebsite.Domain.CustomConfiguration.WEEK_USED_AMOUNT)).thenReturn(used);
String response = assertDoesNotThrow(service::getWeekUsedAmount);
assertTrue(response.contains("0B"), "非法值应按 0 处理: " + response);
}
}
src/test/java/com/lion/lionwebsite/Util/ResponseTest.java
+13 -7
View File
@@ -11,16 +11,22 @@ import static org.junit.jupiter.api.Assertions.*;
class ResponseTest {
/**
* 未标记状态的 Response 上调用 isSuccess() 会抛 NPE：
* isSuccess() 内部直接 result.get("result").asText()，而 "result" 键此时尚不存在。
* 这是既有实现的真实行为（潜在缺陷），此处显式记录而非掩盖；
* 生产路径上 Controller 总会先调 success()/failure()，故未暴露。
* 回归：修复前 isSuccess() 直接 result.get("result").asText()，
* 未设置该键时抛 NPE。现在把「未设置」视为失败。
*/
@Test
void isSuccessOnUnsetStatusThrowsDueToMissingKey() {
void isSuccessOnUnsetStatusIsFalseInsteadOfThrowing() {
Response response = Response.generateResponse();
assertThrows(NullPointerException.class, response::isSuccess,
"若此断言失败说明实现已修复为容忍缺失 result 键，应同步更新该测试");
assertFalse(assertDoesNotThrow(response::isSuccess),
"未设置 result 键应视为失败，而不是抛 NPE");
}
/** 读取不存在的键应返回 null，而不是 NPE。 */
@Test
void getReturnsNullForMissingKey() {
Response response = Response.generateResponse();
assertNull(assertDoesNotThrow(() -> response.get("nope")));
assertNull(assertDoesNotThrow(response::getData));
}
@Test