Files
wenzi/CODE_REVIEW_REPORT.md
Your Name 91a0b77f7a test(cache): 修复CacheConfigTest边界值测试
- 修改 shouldVerifyCacheManager_withMaximumIntegerTtl 为 shouldVerifyCacheManager_withMaximumAllowedTtl
- 使用正确的最大TTL值(10080分钟,7天)而不是 Integer.MAX_VALUE
- 新增 shouldThrowException_whenTtlExceedsMaximum 测试验证边界检查
- 所有1266个测试用例通过
- 覆盖率: 指令81.89%, 行88.48%, 分支51.55%

docs: 添加项目状态报告
- 生成 PROJECT_STATUS_REPORT.md 详细记录项目当前状态
- 包含质量指标、已完成功能、待办事项和技术债务
2026-03-02 13:31:54 +08:00

18 KiB
Raw Blame History

🦟 蚊子项目代码审查报告 v2.0

项目: Mosquito Propagation System
技术栈: Spring Boot 3.1.5 + Java 17 + PostgreSQL + Redis
审查日期: 2026-01-20
审查工具: code-review, security, testing skills


📊 审查摘要

维度 评分 说明
代码质量 架构清晰,但存在重复代码
安全性 ☆☆ 存在SSRF、限流绕过风险
性能 N+1查询、缓存策略需优化
可维护性 命名规范,分层合理
测试覆盖 JaCoCo强制80%覆盖

🔴 严重安全问题 (必须修复)

1. SSRF漏洞 - 短链接重定向

位置: ShortLinkController.java:32-54

@GetMapping("/r/{code}")
public ResponseEntity<Void> redirect(@PathVariable String code, ...) {
    return shortLinkService.findByCode(code)
        .map(e -> {
            // 直接重定向到原始URL无验证!
            headers.set(HttpHeaders.LOCATION, e.getOriginalUrl());
            return new ResponseEntity<>(headers, HttpStatus.FOUND);
        })

风险等级: 🔴 CRITICAL
影响: 攻击者可利用短链接服务访问内部系统

攻击场景:

# 内部IP访问
POST /api/v1/internal/shorten
{"originalUrl": "http://192.168.1.1/admin"}
GET /r/abc123 → 重定向到内部IP

# SSRF探测
http://169.254.169.254/latest/meta-data/ (AWS metadata)
http://localhost:8080/admin

修复方案:

@GetMapping("/r/{code}")
public ResponseEntity<Void> redirect(@PathVariable String code, HttpServletRequest request) {
    return shortLinkService.findByCode(code)
        .map(e -> {
            // 1. URL白名单验证
            if (!isAllowedUrl(e.getOriginalUrl())) {
                log.warn("Blocked malicious redirect: {}", e.getOriginalUrl());
                return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
            }
            
            // 2. 内部IP检查
            if (isInternalUrl(e.getOriginalUrl())) {
                log.warn("Blocked internal redirect: {}", e.getOriginalUrl());
                return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
            }
            
            HttpHeaders headers = new HttpHeaders();
            headers.set(HttpHeaders.LOCATION, e.getOriginalUrl());
            return new ResponseEntity<>(headers, HttpStatus.FOUND);
        })
private boolean isAllowedUrl(String url) {
    if (url == null) return false;
    try {
        URI uri = URI.create(url);
        // 只允许http/https
        if (!uri.isAbsolute() || 
            (!"http".equalsIgnoreCase(uri.getScheme()) && 
             !"https".equalsIgnoreCase(uri.getScheme()))) {
            return false;
        }
        // 检查内部IP
        InetAddress addr = InetAddress.getByName(uri.getHost());
        return !addr.isSiteLocalAddress() && 
               !addr.isLoopbackAddress() &&
               !addr.isAnyLocalAddress();
    } catch (Exception e) {
        return false;
    }
}

2. API密钥一次性返回 - 无恢复机制

位置: ActivityService.java:129-148

public String generateApiKey(CreateApiKeyRequest request) {
    String rawApiKey = UUID.randomUUID().toString();
    // ... 保存hash
    return rawApiKey;  // 只返回一次!
}

风险等级: 🔴 HIGH
影响: 用户丢失密钥后只能重新创建,造成业务中断

业务影响:

  • 用户需要重新配置所有使用该密钥的系统
  • 旧密钥立即失效可能导致服务中断
  • 没有密钥轮换机制

修复方案:

// 方案1: 加密存储,支持重新显示
public class ApiKeyService {
    private static final String ENCRYPTION_KEY = "..."; // 从配置读取
    
    public String generateApiKey(CreateApiKeyRequest request) {
        String rawApiKey = UUID.randomUUID().toString();
        String encryptedKey = encrypt(rawApiKey, ENCRYPTION_KEY);
        
        ApiKeyEntity entity = new ApiKeyEntity();
        entity.setEncryptedKey(encryptedKey);  // 新增字段
        // ...
        return rawApiKey;
    }
    
    @PostMapping("/{id}/reveal")
    public ResponseEntity<String> revealApiKey(@PathVariable Long id) {
        // 需要额外验证(邮箱/密码)
        String encrypted = entity.getEncryptedKey();
        return decrypt(encrypted, ENCRYPTION_KEY);
    }
}

3. 速率限制可被绕过

位置: RateLimitInterceptor.java:17-44

private final ConcurrentHashMap<String, AtomicInteger> localCounters = new ConcurrentHashMap<>();

public boolean preHandle(HttpServletRequest request, ...) {
    if (redisTemplate != null) {
        // 使用Redis
        Long val = redisTemplate.opsForValue().increment(key);
    } else {
        // 回退到本地计数器 - 可被绕过!
        var counter = localCounters.computeIfAbsent(key, k -> new AtomicInteger(0));
        count = counter.incrementAndGet();
    }
}

风险等级: 🔴 HIGH
影响: 多实例部署时无法正确限流

修复方案:

public RateLimitInterceptor(Environment env) {
    this.perMinuteLimit = Integer.parseInt(env.getProperty("app.rate-limit.per-minute", "100"));
    this.redisTemplateOpt = redisTemplateOpt;
    
    // 生产环境强制使用Redis
    String profile = env.getProperty("spring.profiles.active");
    if ("prod".equals(profile) && redisTemplateOpt.isEmpty()) {
        throw new IllegalStateException("Production requires Redis for rate limiting");
    }
}

4. 异常被静默吞掉

位置: ShortLinkController.java:48

try {
    linkClickRepository.save(click);
} catch (Exception ignore) {}  // BAD!

风险等级: 🔴 HIGH
影响: 无法审计追踪,数据库问题不被发现

修复方案:

try {
    linkClickRepository.save(click);
} catch (Exception e) {
    log.error("Failed to record link click for code {}: {}", code, e.getMessage(), e);
    // 可选: 发送到Sentry/Datadog
    // metrics.increment("link_click.errors");
}

🟠 高优先级问题

5. 数据库设计问题

5.1 缺少外键约束

位置: 多个迁移文件

-- V1__Create_activities_table.sql
CREATE TABLE activities (
    id BIGSERIAL PRIMARY KEY,
    ...
);

-- V7__Add_activity_id_to_api_keys.sql
ALTER TABLE api_keys ADD COLUMN activity_id BIGINT;
-- 没有添加 FOREIGN KEY 约束!

问题:

  • api_keys.activity_id 无外键约束
  • short_links.activity_id 无外键约束
  • user_invites 无活动外键验证

修复方案:

-- 添加外键约束
ALTER TABLE api_keys 
ADD CONSTRAINT fk_api_keys_activity 
FOREIGN KEY (activity_id) REFERENCES activities(id) ON DELETE CASCADE;

ALTER TABLE short_links 
ADD CONSTRAINT fk_short_links_activity 
FOREIGN KEY (activity_id) REFERENCES activities(id) ON DELETE SET NULL;

5.2 缺少复合索引

位置: UserInviteRepository.java

public interface UserInviteRepository extends JpaRepository<UserInviteEntity, Long> {
    List<UserInviteEntity> findByActivityId(Long activityId);
    List<UserInviteEntity> findByActivityIdAndInviterUserId(Long activityId, Long inviterUserId);
}

问题: 没有 (activity_id, invitee_user_id) 的索引

迁移文件:

CREATE INDEX idx_user_invites_activity_invitee 
ON user_invites(activity_id, invitee_user_id);

6. N+1 查询问题

位置: ActivityService.java:287-304

@Cacheable(value = "leaderboards", key = "#activityId")
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
    List<UserInviteEntity> invites = userInviteRepository.findByActivityId(activityId);
    // O(n) 次数据库查询? 不, 这是内存处理
    
    Map<Long, Integer> counts = new HashMap<>();
    for (UserInviteEntity inv : invites) {
        counts.merge(inv.getInviterUserId(), 1, Integer::sum);  // 内存聚合
    }
    // ...
}

当前状态: 已优化,在内存中聚合

建议: 如果数据量超过10万考虑使用SQL聚合:

@Query("SELECT u.inviterUserId, COUNT(u) FROM UserInviteEntity u " +
       "WHERE u.activityId = :activityId GROUP BY u.inviterUserId")
List<Object[]> getInviteCountsByActivityId(@Param("activityId") Long activityId);

7. 缓存策略问题

7.1 缓存没有失效机制

位置: ActivityService.java:287

@Cacheable(value = "leaderboards", key = "#activityId")
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
    // 排行榜更新后,缓存不会失效!
}

修复方案:

@CacheEvict(value = "leaderboards", key = "#activityId")
public LeaderboardEntry recordInvite(...) {
    // 记录邀请后清除缓存
}

@Scheduled(fixedRate = 60000) // 或使用CachePut
public void refreshLeaderboardCache() {
    // 定时刷新
}

7.2 缓存配置缺少序列化安全

位置: CacheConfig.java:24-26

RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig()
    .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(
        new GenericJackson2JsonRedisSerializer()
    ));

问题: GenericJackson2JsonRedisSerializer 使用JDK序列化存在反序列化漏洞

修复方案:

// 使用JSON序列化器
RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig()
    .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(
        new Jackson2JsonRedisSerializer<>(Object.class)
    ));

// 或配置类型信息
ObjectMapper mapper = new ObjectMapper();
mapper.activateDefaultTyping(
    mapper.getPolymorphicTypeValidator(),
    ObjectMapper.DefaultTyping.NON_FINAL
);
RedisCacheConfiguration.defaultCacheConfig()
    .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(
        new Jackson2JsonRedisSerializer<>(mapper, Object.class)
    ));

8. 并发安全问题

8.1 内存中计数器 - StatisticsAggregationJob

位置: StatisticsAggregationJob.java:52-59

public DailyActivityStats aggregateStatsForActivity(Activity activity, LocalDate date) {
    Random random = new Random();  // 每次创建新Random
    stats.setViews(1000 + random.nextInt(500));
    // ...
}

当前状态: 无状态操作,安全

建议: 使用 ThreadLocalRandom 提高性能

import java.util.concurrent.ThreadLocalRandom;

public DailyActivityStats aggregateStatsForActivity(Activity activity, LocalDate date) {
    int views = 1000 + ThreadLocalRandom.current().nextInt(500);
    // ...
}

8.2 ConcurrentHashMap 使用正确

位置: ActivityService.java:41

private final Map<Long, Activity> activities = new ConcurrentHashMap<>();

状态: 正确使用并发集合


9. API设计问题

9.1 缺少版本控制

当前: /api/v1/activities
问题: 未来API变更需要破坏性更新

建议:

# Header版本控制
Accept: application/vnd.mosquito.v1+json

# 或URL版本控制
/api/v2/activities

9.2 响应格式不一致

位置: ActivityController.java:68-89

@GetMapping("/{id}/leaderboard")
public ResponseEntity<List<LeaderboardEntry>> getLeaderboard(...) {
    // 分页返回 List
}

@GetMapping("/{id}/leaderboard/export")
public ResponseEntity<byte[]> exportLeaderboard(...) {
    // 导出返回 CSV bytes
}

建议: 统一响应格式

public class ApiResponse<T> {
    private T data;
    private Meta meta;
    private Error error;
    
    public static <T> ApiResponse<T> success(T data) { ... }
    public static <T> ApiResponse<T> paginated(T data, PaginationMeta meta) { ... }
}

10. 未实现的业务逻辑

位置: ActivityService.java:264-271

public void createReward(Reward reward, boolean skipValidation) {
    if (reward.getRewardType() == RewardType.COUPON && !skipValidation) {
        boolean isValidCouponBatchId = false;  // 永远为false!
        if (!isValidCouponBatchId) {
            throw new InvalidActivityDataException("优惠券批次ID无效。");
        }
    }
}

问题: 验证逻辑被硬编码,功能未实现

建议:

// 方案1: 抛出明确的未实现异常
if (reward.getRewardType() == RewardType.COUPON && !skipValidation) {
    throw new UnsupportedOperationException("Coupon validation not yet implemented");
}

// 方案2: 实现真正的验证
public void createReward(Reward reward, boolean skipValidation) {
    if (reward.getRewardType() == RewardType.COUPON && !skipValidation) {
        CouponBatch batch = couponService.getBatchById(reward.getCouponBatchId());
        if (batch == null || !batch.isActive()) {
            throw new InvalidActivityDataException("优惠券批次ID无效或已禁用");
        }
    }
}

🟡 中等优先级问题

11. 硬编码值

位置 建议
ActivityService.java:39 List.of("image/jpeg", "image/png") 提取到配置
ShortLinkService.java:15 DEFAULT_CODE_LEN = 8 提取到配置
RateLimitInterceptor.java:20 per-minute=100 提取到配置
ActivityService.java:62 rewardCalculationMode = "delta" 使用枚举

建议: 创建 AppConstants 类或使用配置

@Configuration
@ConfigurationProperties(prefix = "app")
public class AppConfig {
    private int defaultCodeLength = 8;
    private int rateLimitPerMinute = 100;
    private List<String> supportedImageTypes = List.of("image/jpeg", "image/png");
    
    // getters and setters
}

12. 重复代码

位置: ActivityService.java

// 重复的existsById检查
private void validateActivityExists(Long activityId) {
    if (!activityRepository.existsById(activityId)) {
        throw new ActivityNotFoundException("活动不存在。");
    }
}

// 在多个方法中使用
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
    if (!activityRepository.existsById(activityId)) {  // 重复
        throw new ActivityNotFoundException("活动不存在。");
    }
    // ...
}

修复方案:

public List<LeaderboardEntry> getLeaderboard(Long activityId) {
    validateActivityExists(activityId);  // 使用私有方法
    // ...
}

private void validateActivityExists(Long activityId) {
    if (!activityRepository.existsById(activityId)) {
        throw new ActivityNotFoundException("活动不存在。");
    }
}

13. 缺少输入长度验证

位置: ShortenRequest.java

public class ShortenRequest {
    @NotBlank
    private String originalUrl;
    // 没有 @Size 验证!
}

修复方案:

public class ShortenRequest {
    @NotBlank
    @Size(min = 10, max = 2048, message = "URL长度必须在10-2048之间")
    private String originalUrl;
}

14. 缺少审计字段

问题: 部分表缺少 created_by, updated_by 字段

影响: 无法追踪数据变更责任人

建议:

ALTER TABLE activities ADD COLUMN created_by BIGINT;
ALTER TABLE activities ADD COLUMN updated_by BIGINT;

使用Spring Data Auditing:

@Entity
@EntityListeners(AuditingEntityListener.class)
public class ActivityEntity {
    @CreatedBy
    private Long createdBy;
    
    @LastModifiedBy
    private Long updatedBy;
}

15. 缺少软删除

当前: 使用 revoked_at 字段模拟软删除

问题:

  • API密钥有软删除
  • 其他数据没有统一处理

建议: 使用Spring Data JPA Soft Delete

@SoftDelete
public interface ActivityRepository extends JpaRepository<ActivityEntity, Long> {
}

🟢 低优先级改进建议

16. 日志格式不统一

位置: 多个文件

// 混杂的中英文日志
log.info("开始执行每日活动数据聚合任务");
log.info("为活动ID {} 聚合了数据", activity.getId());

建议: 统一使用英文或使用日志模板


17. 缺少健康检查端点

建议: 添加 actuator 端点

management.endpoints.web.exposure.include=health,info,metrics
management.endpoint.health.show-details=when_authorized

18. 缺少API文档

建议: 使用SpringDoc OpenAPI

@RestController
@RequestMapping("/api/v1/activities")
@Tag(name = "Activity Management", description = "活动管理API")
public class ActivityController {
    @Operation(summary = "创建活动", description = "创建一个新的推广活动")
    @PostMapping
    public ResponseEntity<Activity> createActivity(...) {
        // ...
    }
}

📈 性能优化建议

19. 数据库连接池

当前: application.properties 无数据库配置

建议:

spring.datasource.hikari.maximum-pool-size=20
spring.datasource.hikari.minimum-idle=5
spring.datasource.hikari.connection-timeout=30000
spring.datasource.hikari.idle-timeout=600000
spring.datasource.hikari.max-lifetime=1800000

20. 批量操作优化

位置: DbRewardQueue.java:13-24

public void enqueueReward(String trackingId, String externalUserId, String payloadJson) {
    RewardJobEntity job = new RewardJobEntity();
    repository.save(job);  // 单条插入
}

建议: 实现批量插入

@Override
public void enqueueRewards(List<RewardJob> jobs) {
    List<RewardJobEntity> entities = jobs.stream()
        .map(this::toEntity)
        .collect(Collectors.toList());
    repository.saveAll(entities);
}

🔒 安全加固清单

必须修复

  • URL白名单验证 (SSRF防护)
  • API密钥恢复机制
  • 异常日志记录
  • 速率限制强制Redis

建议修复

  • 添加数据库外键约束
  • 缓存序列化安全
  • 输入长度验证
  • 审计字段

可选改进

  • API版本控制
  • 统一响应格式
  • OpenAPI文档
  • 健康检查端点

📚 参考资源


📝 审查统计

类别 数量
🔴 严重安全问题 4
🟠 高优先级问题 6
🟡 中等优先级问题 5
🟢 低优先级改进 5
总计 20

报告生成时间: 2026-01-20 使用Skills: code-review, security, database, api-design