Skip to content

Commit

Permalink
fix: restrict thumbnail generation to images in the attachment library (
Browse files Browse the repository at this point in the history
#7079)

#### What type of PR is this?
/kind improvement
/area core
/milestone 2.20.x

#### What this PR does / why we need it:
限制缩略图生成仅针对附件库中的图片,防止任意 URI 的生成行为带来的潜在攻击风险

先 merge #7077 后才能合并此 PR

#### Does this PR introduce a user-facing change?

```release-note
限制缩略图生成仅针对附件库中的图片,防止任意 URI 的生成行为带来的潜在攻击风险
```
  • Loading branch information
guqing authored Nov 26, 2024
1 parent ec5c70f commit 5cefefe
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static String buildSizesAttr() {
*/
public static Mono<String> generateSrcset(URI src, ThumbnailService thumbnailService) {
return Flux.fromArray(ThumbnailSize.values())
.flatMap(size -> thumbnailService.generate(src, size)
.flatMap(size -> thumbnailService.get(src, size)
.map(thumbnail -> thumbnail.toString() + " " + size.getWidth() + "w")
)
.collect(StringBuilder::new, (builder, srcsetValue) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,16 @@ public interface ThumbnailService {
*/
Mono<URI> generate(URI imageUri, ThumbnailSize size);

/**
* <p>Get thumbnail by the given image uri and size.</p>
* <p>It depends on the {@link #generate(URI, ThumbnailSize)} method, currently the thumbnail
* generation is limited to the attachment service, that is, the thumbnail is strongly
* associated with the attachment.</p>
*
* @return if thumbnail exists, return the thumbnail uri, otherwise return the original image
* uri
*/
Mono<URI> get(URI imageUri, ThumbnailSize size);

Mono<Void> delete(URI imageUri);
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ private Mono<URI> doGenerate(URI imageUri, ThumbnailSize size) {
});
}

@Override
public Mono<URI> get(URI imageUri, ThumbnailSize size) {
return fetchThumbnail(imageUri, size)
.map(thumbnail -> URI.create(thumbnail.getSpec().getThumbnailUri()))
.defaultIfEmpty(imageUri);
}

@Override
public Mono<Void> delete(URI imageUri) {
Assert.notNull(imageUri, "Image uri must not be null");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public RouterFunction<ServerResponse> endpoint() {

private Mono<ServerResponse> getThumbnailByUri(ServerRequest request) {
var query = new ThumbnailQuery(request.queryParams());
return thumbnailService.generate(query.getUri(), query.getSize())
return thumbnailService.get(query.getUri(), query.getSize())
.filterWhen(uri -> isAccessible(request, uri))
.defaultIfEmpty(query.getUri())
.flatMap(uri -> ServerResponse.temporaryRedirect(uri).build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class ThumbnailFinderImpl implements ThumbnailFinder {
@Override
public Mono<String> gen(String uriStr, String size) {
return Mono.fromSupplier(() -> URI.create(uriStr))
.flatMap(uri -> thumbnailService.generate(uri, ThumbnailSize.fromName(size)))
.flatMap(uri -> thumbnailService.get(uri, ThumbnailSize.fromName(size)))
.map(URI::toString)
.onErrorResume(Throwable.class, e -> {
log.debug("Failed to generate thumbnail for [{}], error: [{}]", uriStr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void setUp() {

@Test
void thumbnailUriNotAccessible() {
when(thumbnailService.generate(any(), any()))
when(thumbnailService.get(any(), any()))
.thenReturn(Mono.just(URI.create("/thumbnail-not-found.png")));
webClient.get()
.uri("/thumbnails/-/via-uri?size=l&uri=/myavatar.png")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ void shouldNotGenWhenUriIsInvalid() {

@Test
void shouldGenWhenUriIsValid() {
when(thumbnailService.generate(any(), any()))
when(thumbnailService.get(any(), any()))
.thenReturn(Mono.just(URI.create("/test-thumb.jpg")));
thumbnailFinder.gen("/test.jpg", "l")
.as(StepVerifier::create)
.expectNext("/test-thumb.jpg")
.verifyComplete();

verify(thumbnailService).generate(any(), any());
verify(thumbnailService).get(any(), any());
}
}

0 comments on commit 5cefefe

Please sign in to comment.