Skip to content

Commit 6a8ce02

Browse files
crisbetommalerba
authored andcommitted
fix(overlay): validate that ConnectedPositionStrategy positions are passed in correctly at runtime (#9466)
Since we have some cases where we pass in positions verbatim to the connected position strategy (the connected position directive), we could end up in a situation where the consumer passed in a set of invalid positions that don't break necessarily, but don't work correctly either. These changes add some sanity checks at runtime to make debugging easier.
1 parent b68d38c commit 6a8ce02

File tree

3 files changed

+102
-3
lines changed

3 files changed

+102
-3
lines changed

src/cdk/overlay/position/connected-position-strategy.spec.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,60 @@ describe('ConnectedPositionStrategy', () => {
793793

794794
});
795795

796+
describe('validations', () => {
797+
let overlayElement: HTMLElement;
798+
let originElement: HTMLElement;
799+
let strategy: ConnectedPositionStrategy;
800+
801+
beforeEach(() => {
802+
overlayElement = createPositionedBlockElement();
803+
overlayContainerElement.appendChild(overlayElement);
804+
originElement = createBlockElement();
805+
806+
strategy = positionBuilder.connectedTo(
807+
new ElementRef(originElement),
808+
{originX: 'start', originY: 'bottom'},
809+
{overlayX: 'start', overlayY: 'top'});
810+
strategy.attach(fakeOverlayRef(overlayElement));
811+
});
812+
813+
afterEach(() => {
814+
strategy.dispose();
815+
});
816+
817+
it('should throw when attaching without any positions', () => {
818+
strategy.withPositions([]);
819+
expect(() => strategy.apply()).toThrow();
820+
});
821+
822+
it('should throw when passing in something that is missing a connection point', () => {
823+
strategy.withPositions([{originY: 'top', overlayX: 'start', overlayY: 'top'} as any]);
824+
expect(() => strategy.apply()).toThrow();
825+
});
826+
827+
it('should throw when passing in something that has an invalid X position', () => {
828+
strategy.withPositions([{
829+
originX: 'left',
830+
originY: 'top',
831+
overlayX: 'left',
832+
overlayY: 'top'
833+
} as any]);
834+
835+
expect(() => strategy.apply()).toThrow();
836+
});
837+
838+
it('should throw when passing in something that has an invalid Y position', () => {
839+
strategy.withPositions([{
840+
originX: 'start',
841+
originY: 'middle',
842+
overlayX: 'start',
843+
overlayY: 'middle'
844+
} as any]);
845+
846+
expect(() => strategy.apply()).toThrow();
847+
});
848+
});
849+
796850
});
797851

798852
/** Creates an absolutely positioned, display: block element with a default size. */

src/cdk/overlay/position/connected-position-strategy.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
OverlayConnectionPosition,
1616
ConnectedOverlayPositionChange,
1717
ScrollingVisibility,
18+
validateHorizontalPosition,
19+
validateVerticalPosition,
1820
} from './connected-position';
1921
import {Subject} from 'rxjs/Subject';
2022
import {Subscription} from 'rxjs/Subscription';
@@ -131,6 +133,7 @@ export class ConnectedPositionStrategy implements PositionStrategy {
131133
return;
132134
}
133135

136+
this._validatePositions();
134137
this._applied = true;
135138

136139
// We need the bounding rects for the origin and the overlay to determine how to position
@@ -184,6 +187,8 @@ export class ConnectedPositionStrategy implements PositionStrategy {
184187
return;
185188
}
186189

190+
this._validatePositions();
191+
187192
const originRect = this._origin.getBoundingClientRect();
188193
const overlayRect = this._pane.getBoundingClientRect();
189194
const viewportSize = this._viewportRuler.getViewportSize();
@@ -439,14 +444,28 @@ export class ConnectedPositionStrategy implements PositionStrategy {
439444
this._onPositionChange.next(positionChange);
440445
}
441446

442-
/**
443-
* Subtracts the amount that an element is overflowing on an axis from it's length.
444-
*/
447+
/** Subtracts the amount that an element is overflowing on an axis from it's length. */
445448
private _subtractOverflows(length: number, ...overflows: number[]): number {
446449
return overflows.reduce((currentValue: number, currentOverflow: number) => {
447450
return currentValue - Math.max(currentOverflow, 0);
448451
}, length);
449452
}
453+
454+
/** Validates that the current position match the expected values. */
455+
private _validatePositions(): void {
456+
if (!this._preferredPositions.length) {
457+
throw Error('ConnectedPositionStrategy: At least one position is required.');
458+
}
459+
460+
// TODO(crisbeto): remove these once Angular's template type
461+
// checking is advanced enough to catch these cases.
462+
this._preferredPositions.forEach(pair => {
463+
validateHorizontalPosition('originX', pair.originX);
464+
validateVerticalPosition('originY', pair.originY);
465+
validateHorizontalPosition('overlayX', pair.overlayX);
466+
validateVerticalPosition('overlayY', pair.overlayY);
467+
});
468+
}
450469
}
451470

452471
/** A simple (x, y) coordinate. */

src/cdk/overlay/position/connected-position.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,29 @@ export class ConnectedOverlayPositionChange {
9090
/** @docs-private */
9191
@Optional() public scrollableViewProperties: ScrollingVisibility) {}
9292
}
93+
94+
/**
95+
* Validates whether a vertical position property matches the expected values.
96+
* @param property Name of the property being validated.
97+
* @param value Value of the property being validated.
98+
* @docs-private
99+
*/
100+
export function validateVerticalPosition(property: string, value: VerticalConnectionPos) {
101+
if (value !== 'top' && value !== 'bottom' && value !== 'center') {
102+
throw Error(`ConnectedPosition: Invalid ${property} "${value}". ` +
103+
`Expected "top", "bottom" or "center".`);
104+
}
105+
}
106+
107+
/**
108+
* Validates whether a horizontal position property matches the expected values.
109+
* @param property Name of the property being validated.
110+
* @param value Value of the property being validated.
111+
* @docs-private
112+
*/
113+
export function validateHorizontalPosition(property: string, value: HorizontalConnectionPos) {
114+
if (value !== 'start' && value !== 'end' && value !== 'center') {
115+
throw Error(`ConnectedPosition: Invalid ${property} "${value}". ` +
116+
`Expected "start", "end" or "center".`);
117+
}
118+
}

0 commit comments

Comments
 (0)