From 8e52ca271496b0feebf66b2dc7c8f396b73d61a0 Mon Sep 17 00:00:00 2001 From: Alex Castle Date: Tue, 8 Nov 2022 14:09:21 -0800 Subject: [PATCH] fix(common): Don't generate srcsets with very large sources (#47997) Fix an issue where users could inadvertently generate very large source images in ngOptimizedImage PR Close #47997 --- .../ng_optimized_image/ng_optimized_image.ts | 16 +++++++++++-- .../directives/ng_optimized_image_spec.ts | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts index 3a25e75fedcce..e3dd65adc65ec 100644 --- a/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts +++ b/packages/common/src/directives/ng_optimized_image/ng_optimized_image.ts @@ -75,6 +75,14 @@ const ASPECT_RATIO_TOLERANCE = .1; */ const OVERSIZED_IMAGE_TOLERANCE = 1000; +/** + * Used to limit automatic srcset generation of very large sources for + * fixed-size images. In pixels. + */ +const FIXED_SRCSET_WIDTH_LIMIT = 1920; +const FIXED_SRCSET_HEIGHT_LIMIT = 1080; + + /** Info about built-in loaders we can test for. */ export const BUILT_IN_LOADERS = [imgixLoaderInfo, imageKitLoaderInfo, cloudinaryLoaderInfo]; @@ -422,8 +430,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { if (this.ngSrcset) { rewrittenSrcset = this.getRewrittenSrcset(); - } else if ( - !this._disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader) { + } else if (this.shouldGenerateAutomaticSrcset()) { rewrittenSrcset = this.getAutomaticSrcset(); } @@ -517,6 +524,11 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy { return finalSrcs.join(', '); } + private shouldGenerateAutomaticSrcset(): boolean { + return !this._disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader && + !(this.width! > FIXED_SRCSET_WIDTH_LIMIT || this.height! > FIXED_SRCSET_HEIGHT_LIMIT); + } + /** @nodoc */ ngOnDestroy() { if (ngDevMode) { diff --git a/packages/common/test/directives/ng_optimized_image_spec.ts b/packages/common/test/directives/ng_optimized_image_spec.ts index f92661cec12bf..3a42b21505648 100644 --- a/packages/common/test/directives/ng_optimized_image_spec.ts +++ b/packages/common/test/directives/ng_optimized_image_spec.ts @@ -1496,6 +1496,30 @@ describe('Image directive', () => { .toBe(`${IMG_BASE_URL}/img?w=100 1x, ${IMG_BASE_URL}/img?w=200 2x`); }); + it('should not add a fixed srcset to the img element if height is too large', () => { + setupTestingModule({imageLoader}); + + const template = ``; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + const nativeElement = fixture.nativeElement as HTMLElement; + const img = nativeElement.querySelector('img')!; + expect(img.getAttribute('srcset')).toBeNull(); + }); + + it('should not add a fixed srcset to the img element if width is too large', () => { + setupTestingModule({imageLoader}); + + const template = ``; + const fixture = createTestComponent(template); + fixture.detectChanges(); + + const nativeElement = fixture.nativeElement as HTMLElement; + const img = nativeElement.querySelector('img')!; + expect(img.getAttribute('srcset')).toBeNull(); + }); + it('should use a custom breakpoint set if one is provided', () => { const imageConfig = { breakpoints: [16, 32, 48, 64, 96, 128, 256, 384, 640, 1280, 3840],