Skip to content

Commit

Permalink
fix(common): Don't generate srcsets with very large sources (#47997)
Browse files Browse the repository at this point in the history
Fix an issue where users could inadvertently generate very large source images in ngOptimizedImage

PR Close #47997
  • Loading branch information
atcastle authored and AndrewKushnir committed Dec 5, 2022
1 parent 9214e58 commit 50b1c2b
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
Expand Up @@ -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];

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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) {
Expand Down
24 changes: 24 additions & 0 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -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 = `<img ngSrc="img" width="1100" height="2400">`;
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 = `<img ngSrc="img" width="3000" height="400">`;
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],
Expand Down

0 comments on commit 50b1c2b

Please sign in to comment.