Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -16,6 +16,7 @@
import com.iflytek.skillhub.dto.SkillLifecycleVersionResponse;
import com.iflytek.skillhub.dto.SkillVersionDetailResponse;
import com.iflytek.skillhub.dto.SkillVersionResponse;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import com.iflytek.skillhub.ratelimit.RateLimit;
import org.springframework.core.io.InputStreamResource;
import org.springframework.data.domain.Page;
Expand All @@ -39,14 +40,17 @@ public class SkillController extends BaseApiController {

private final SkillQueryService skillQueryService;
private final SkillDownloadService skillDownloadService;
private final SkillHubMetrics metrics;

public SkillController(
SkillQueryService skillQueryService,
SkillDownloadService skillDownloadService,
SkillHubMetrics metrics,
ApiResponseFactory responseFactory) {
super(responseFactory);
this.skillQueryService = skillQueryService;
this.skillDownloadService = skillDownloadService;
this.metrics = metrics;
}

@GetMapping("/{namespace}/{slug}")
Expand Down Expand Up @@ -332,16 +336,24 @@ public ResponseEntity<InputStreamResource> downloadByTag(

private ResponseEntity<InputStreamResource> buildDownloadResponse(HttpServletRequest request, SkillDownloadService.DownloadResult result) {
if (shouldRedirectToPresignedUrl(request, result.presignedUrl())) {
metrics.recordDownloadDelivery("redirect", result.fallbackBundle());
if (result.fallbackBundle()) {
metrics.incrementBundleMissingFallback();
}
return ResponseEntity.status(HttpStatus.FOUND)
.header(HttpHeaders.LOCATION, result.presignedUrl())
.build();
}

metrics.recordDownloadDelivery("stream", result.fallbackBundle());
if (result.fallbackBundle()) {
metrics.incrementBundleMissingFallback();
}
return ResponseEntity.ok()
.header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + result.filename() + "\"")
.contentType(MediaType.parseMediaType(result.contentType()))
.contentLength(result.contentLength())
.body(new InputStreamResource(result.content()));
.body(new InputStreamResource(result.openContent()));
}

private boolean shouldRedirectToPresignedUrl(HttpServletRequest request, String presignedUrl) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import com.iflytek.skillhub.security.SensitiveLogSanitizer;
import com.iflytek.skillhub.storage.StorageAccessException;
import jakarta.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -27,11 +29,14 @@ public class GlobalExceptionHandler {
private static final Logger logger = LoggerFactory.getLogger(GlobalExceptionHandler.class);
private final ApiResponseFactory apiResponseFactory;
private final SensitiveLogSanitizer sensitiveLogSanitizer;
private final SkillHubMetrics metrics;

public GlobalExceptionHandler(ApiResponseFactory apiResponseFactory,
SensitiveLogSanitizer sensitiveLogSanitizer) {
SensitiveLogSanitizer sensitiveLogSanitizer,
SkillHubMetrics metrics) {
this.apiResponseFactory = apiResponseFactory;
this.sensitiveLogSanitizer = sensitiveLogSanitizer;
this.metrics = metrics;
}

@ExceptionHandler(LocalizedException.class)
Expand Down Expand Up @@ -108,6 +113,23 @@ public ResponseEntity<ApiResponse<Void>> handleAccessDenied(AccessDeniedExceptio
apiResponseFactory.error(403, "error.forbidden"));
}

@ExceptionHandler(StorageAccessException.class)
public ResponseEntity<ApiResponse<Void>> handleStorageAccess(StorageAccessException ex, HttpServletRequest request) {
metrics.incrementStorageAccessFailure(ex.getOperation());
logger.warn(
"Object storage unavailable [requestId={}, method={}, path={}, userId={}, operation={}, key={}]",
MDC.get("requestId"),
request.getMethod(),
sensitiveLogSanitizer.sanitizeRequestTarget(request),
resolveUserId(request),
ex.getOperation(),
ex.getKey(),
ex
);
return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(
apiResponseFactory.error(503, "error.storage.unavailable"));
}

@ExceptionHandler(Exception.class)
public ResponseEntity<ApiResponse<Void>> handleGlobalException(Exception ex, HttpServletRequest request) {
logger.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,30 @@ public void incrementSkillPublish(String namespace, String status) {
"status", status
).increment();
}

public void recordDownloadDelivery(String mode, boolean fallbackBundle) {
meterRegistry.counter(
"skillhub.skill.download.delivery",
"mode", mode,
"fallback_bundle", Boolean.toString(fallbackBundle)
).increment();
}

public void incrementBundleMissingFallback() {
meterRegistry.counter("skillhub.skill.download.bundle_missing_fallback").increment();
}

public void incrementRateLimitExceeded(String category) {
meterRegistry.counter(
"skillhub.ratelimit.exceeded",
"category", category
).increment();
}

public void incrementStorageAccessFailure(String operation) {
meterRegistry.counter(
"skillhub.storage.failure",
"operation", operation
).increment();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Component;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.servlet.HandlerInterceptor;
import org.springframework.web.servlet.HandlerMapping;

import java.util.Map;

@Component
public class RateLimitInterceptor implements HandlerInterceptor {
Expand All @@ -19,17 +23,20 @@ public class RateLimitInterceptor implements HandlerInterceptor {
private final AnonymousDownloadIdentityService anonymousDownloadIdentityService;
private final ApiResponseFactory apiResponseFactory;
private final ObjectMapper objectMapper;
private final SkillHubMetrics metrics;

public RateLimitInterceptor(RateLimiter rateLimiter,
ClientIpResolver clientIpResolver,
AnonymousDownloadIdentityService anonymousDownloadIdentityService,
ApiResponseFactory apiResponseFactory,
ObjectMapper objectMapper) {
ObjectMapper objectMapper,
SkillHubMetrics metrics) {
this.rateLimiter = rateLimiter;
this.clientIpResolver = clientIpResolver;
this.anonymousDownloadIdentityService = anonymousDownloadIdentityService;
this.apiResponseFactory = apiResponseFactory;
this.objectMapper = objectMapper;
this.metrics = metrics;
}

@Override
Expand All @@ -51,12 +58,17 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons

// Get limit based on authentication status
int limit = isAuthenticated ? rateLimit.authenticated() : rateLimit.anonymous();
String resourceSuffix = resolveResourceSuffix(rateLimit.category(), request);

boolean allowed = isAuthenticated
? rateLimiter.tryAcquire("ratelimit:" + rateLimit.category() + ":user:" + userId, limit, rateLimit.windowSeconds())
: checkAnonymousLimit(request, response, rateLimit, limit);
? rateLimiter.tryAcquire(
"ratelimit:" + rateLimit.category() + ":user:" + userId + resourceSuffix,
limit,
rateLimit.windowSeconds())
: checkAnonymousLimit(request, response, rateLimit, limit, resourceSuffix);

if (!allowed) {
metrics.incrementRateLimitExceeded(rateLimit.category());
response.setStatus(HttpStatus.TOO_MANY_REQUESTS.value());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
ApiResponse<Void> body = apiResponseFactory.error(429, "error.rateLimit.exceeded");
Expand All @@ -70,10 +82,11 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
private boolean checkAnonymousLimit(HttpServletRequest request,
HttpServletResponse response,
RateLimit rateLimit,
int limit) {
int limit,
String resourceSuffix) {
if (!"download".equals(rateLimit.category())) {
return rateLimiter.tryAcquire(
"ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request),
"ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request) + resourceSuffix,
limit,
rateLimit.windowSeconds()
);
Expand All @@ -82,17 +95,45 @@ private boolean checkAnonymousLimit(HttpServletRequest request,
AnonymousDownloadIdentityService.AnonymousDownloadIdentity identity =
anonymousDownloadIdentityService.resolve(request, response);
boolean ipAllowed = rateLimiter.tryAcquire(
"ratelimit:download:ip:" + identity.ipHash(),
"ratelimit:download:ip:" + identity.ipHash() + resourceSuffix,
limit,
rateLimit.windowSeconds()
);
if (!ipAllowed) {
return false;
}
return rateLimiter.tryAcquire(
"ratelimit:download:anon:" + identity.cookieHash(),
"ratelimit:download:anon:" + identity.cookieHash() + resourceSuffix,
limit,
rateLimit.windowSeconds()
);
}

@SuppressWarnings("unchecked")
private String resolveResourceSuffix(String category, HttpServletRequest request) {
if (!"download".equals(category)) {
return "";
}
Object attribute = request.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
if (!(attribute instanceof Map<?, ?> templateVariables)) {
return "";
}
String namespace = stringValue(templateVariables.get("namespace"));
String slug = stringValue(templateVariables.get("slug"));
String version = stringValue(templateVariables.get("version"));
String tagName = stringValue(templateVariables.get("tagName"));
if (namespace == null || slug == null) {
return "";
}
String target = version != null ? "version:" + version : tagName != null ? "tag:" + tagName : "latest";
return ":ns:" + namespace + ":slug:" + slug + ":" + target;
}

private String stringValue(Object value) {
if (value == null) {
return null;
}
String text = value.toString();
return text.isBlank() ? null : text;
}
}
4 changes: 4 additions & 0 deletions server/skillhub-app/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ skillhub:
force-path-style: ${SKILLHUB_STORAGE_S3_FORCE_PATH_STYLE:true}
auto-create-bucket: ${SKILLHUB_STORAGE_S3_AUTO_CREATE_BUCKET:false}
presign-expiry: ${SKILLHUB_STORAGE_S3_PRESIGN_EXPIRY:PT10M}
max-connections: ${SKILLHUB_STORAGE_S3_MAX_CONNECTIONS:100}
connection-acquisition-timeout: ${SKILLHUB_STORAGE_S3_CONNECTION_ACQUISITION_TIMEOUT:PT2S}
api-call-attempt-timeout: ${SKILLHUB_STORAGE_S3_API_CALL_ATTEMPT_TIMEOUT:PT10S}
api-call-timeout: ${SKILLHUB_STORAGE_S3_API_CALL_TIMEOUT:PT30S}
search:
engine: postgres
rebuild-on-startup: false
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
ALTER TABLE skill_version
ADD COLUMN bundle_ready BOOLEAN NOT NULL DEFAULT FALSE,
ADD COLUMN download_ready BOOLEAN NOT NULL DEFAULT FALSE;

UPDATE skill_version
SET download_ready = CASE
WHEN status = 'PUBLISHED' AND file_count > 0 THEN TRUE
ELSE FALSE
END,
bundle_ready = FALSE;
1 change: 1 addition & 0 deletions server/skillhub-app/src/main/resources/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ error.auth.sessionBootstrap.notAuthenticated=No authenticated external session f
error.badRequest=Invalid request
error.forbidden=Forbidden
error.rateLimit.exceeded=Rate limit exceeded
error.storage.unavailable=Object storage is temporarily unavailable. Please try again later.
error.internal=An unexpected error occurred
error.slug.blank=Slug cannot be blank
error.slug.length=Slug length must be between {0} and {1} characters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ error.auth.sessionBootstrap.notAuthenticated=未检测到已认证的外部会
error.badRequest=请求参数不合法
error.forbidden=没有权限执行该操作
error.rateLimit.exceeded=请求过于频繁,请稍后再试
error.storage.unavailable=对象存储暂时不可用,请稍后再试
error.internal=服务器内部错误
error.slug.blank=slug 不能为空
error.slug.length=slug 长度必须在 {0} 到 {1} 个字符之间
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
Expand Down Expand Up @@ -60,15 +61,17 @@ void anonymousDownloadUsesIpAndSignedCookieBuckets() throws Exception {
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
new ByteArrayInputStream("zip".getBytes()),
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
3L,
"application/zip",
null
null,
false
));

var result = mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.header("X-Forwarded-For", "203.0.113.10"))
.header("X-Forwarded-For", "203.0.113.10")
.with(user("anonymous-test")))
.andExpect(status().isOk())
.andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\""))
.andExpect(header().exists("Set-Cookie"))
Expand All @@ -83,8 +86,8 @@ void anonymousDownloadUsesIpAndSignedCookieBuckets() throws Exception {

ArgumentCaptor<String> keyCaptor = ArgumentCaptor.forClass(String.class);
verify(rateLimiter, times(2)).tryAcquire(keyCaptor.capture(), anyInt(), anyInt());
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:"));
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:"));
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:") && key.endsWith(":ns:global:slug:demo-skill:version:1.0.0"));
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:") && key.endsWith(":ns:global:slug:demo-skill:version:1.0.0"));
}

@Test
Expand All @@ -93,7 +96,8 @@ void anonymousDownloadReturnsTooManyRequestsWhenIpBucketIsExceeded() throws Exce
((String) invocation.getArgument(0)).startsWith("ratelimit:download:ip:") ? false : true);

mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.header("X-Forwarded-For", "203.0.113.10"))
.header("X-Forwarded-For", "203.0.113.10")
.with(user("anonymous-test")))
.andExpect(status().isTooManyRequests())
.andExpect(jsonPath("$.code").value(429));

Expand Down
Loading
Loading