Skip to content

Commit

Permalink
refactor(core): throw an error when hydration marker is missing from …
Browse files Browse the repository at this point in the history
…DOM (#51170)

non-destructive hydration expects the DOM tree to have the same structure in both places.
With this commit, the app will throw an error if comments are stripped out by the http server (eg by some CDNs).

fixes #51160

PR Close #51170
  • Loading branch information
JeanMeche authored and dylhunn committed Aug 4, 2023
1 parent 6d05a68 commit 0a38dc3
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 46 deletions.
2 changes: 2 additions & 0 deletions goldens/public-api/core/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ export const enum RuntimeErrorCode {
// (undocumented)
MISSING_REQUIRED_INJECTABLE_IN_BOOTSTRAP = 402,
// (undocumented)
MISSING_SSR_CONTENT_INTEGRITY_MARKER = 507,
// (undocumented)
MISSING_ZONEJS = 908,
// (undocumented)
MULTIPLE_COMPONENTS_MATCH = -300,
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export {formatRuntimeError as ɵformatRuntimeError, RuntimeError as ɵRuntimeErr
export {annotateForHydration as ɵannotateForHydration} from './hydration/annotate';
export {withDomHydration as ɵwithDomHydration} from './hydration/api';
export {IS_HYDRATION_DOM_REUSE_ENABLED as ɵIS_HYDRATION_DOM_REUSE_ENABLED} from './hydration/tokens';
export {SSR_CONTENT_INTEGRITY_MARKER as ɵSSR_CONTENT_INTEGRITY_MARKER} from './hydration/utils';
export {CurrencyIndex as ɵCurrencyIndex, ExtraLocaleDataIndex as ɵExtraLocaleDataIndex, findLocaleData as ɵfindLocaleData, getLocaleCurrencyCode as ɵgetLocaleCurrencyCode, getLocalePluralCase as ɵgetLocalePluralCase, LocaleDataIndex as ɵLocaleDataIndex, registerLocaleData as ɵregisterLocaleData, unregisterAllLocaleData as ɵunregisterLocaleData} from './i18n/locale_data_api';
export {DEFAULT_LOCALE_ID as ɵDEFAULT_LOCALE_ID} from './i18n/localization';
export {InitialRenderPendingTasks as ɵInitialRenderPendingTasks} from './initial_render_pending_tasks';
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const enum RuntimeErrorCode {
INVALID_SKIP_HYDRATION_HOST = -504,
MISSING_HYDRATION_ANNOTATIONS = -505,
HYDRATION_STABLE_TIMEDOUT = -506,
MISSING_SSR_CONTENT_INTEGRITY_MARKER = 507,

// Signal Errors
SIGNAL_WRITE_FROM_ILLEGAL_CONTEXT = 600,
Expand Down
42 changes: 37 additions & 5 deletions packages/core/src/hydration/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,27 @@
import {first} from 'rxjs/operators';

import {APP_BOOTSTRAP_LISTENER, ApplicationRef} from '../application_ref';
import {ENABLED_SSR_FEATURES, PLATFORM_ID} from '../application_tokens';
import {ENABLED_SSR_FEATURES} from '../application_tokens';
import {Console} from '../console';
import {ENVIRONMENT_INITIALIZER, EnvironmentProviders, Injector, makeEnvironmentProviders} from '../di';
import {inject} from '../di/injector_compatibility';
import {formatRuntimeError, RuntimeErrorCode} from '../errors';
import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from '../errors';
import {enableLocateOrCreateContainerRefImpl} from '../linker/view_container_ref';
import {enableLocateOrCreateElementNodeImpl} from '../render3/instructions/element';
import {enableLocateOrCreateElementContainerNodeImpl} from '../render3/instructions/element_container';
import {enableApplyRootElementTransformImpl} from '../render3/instructions/shared';
import {enableLocateOrCreateContainerAnchorImpl} from '../render3/instructions/template';
import {enableLocateOrCreateTextNodeImpl} from '../render3/instructions/text';
import {getDocument} from '../render3/interfaces/document';
import {isPlatformBrowser} from '../render3/util/misc_utils';
import {TransferState} from '../transfer_state';
import {NgZone} from '../zone';

import {cleanupDehydratedViews} from './cleanup';
import {IS_HYDRATION_DOM_REUSE_ENABLED, PRESERVE_HOST_CONTENT} from './tokens';
import {enableRetrieveHydrationInfoImpl, NGH_DATA_KEY} from './utils';
import {enableRetrieveHydrationInfoImpl, NGH_DATA_KEY, SSR_CONTENT_INTEGRITY_MARKER} from './utils';
import {enableFindMatchingDehydratedViewImpl} from './views';


/**
* Indicates whether the hydration-related code was added,
* prevents adding it multiple times.
Expand Down Expand Up @@ -150,10 +150,11 @@ export function withDomHydration(): EnvironmentProviders {
useValue: () => {
// Since this function is used across both server and client,
// make sure that the runtime code is only added when invoked
// on the client. Moving forward, the `isBrowser` check should
// on the client. Moving forward, the `isPlatformBrowser` check should
// be replaced with a tree-shakable alternative (e.g. `isServer`
// flag).
if (isPlatformBrowser() && inject(IS_HYDRATION_DOM_REUSE_ENABLED)) {
verifySsrContentsIntegrity();
enableHydrationRuntimeSupport();
}
},
Expand Down Expand Up @@ -209,3 +210,34 @@ function logWarningOnStableTimedout(time: number, console: Console): void {

console.warn(formatRuntimeError(RuntimeErrorCode.HYDRATION_STABLE_TIMEDOUT, message));
}

/**
* Verifies whether the DOM contains a special marker added during SSR time to make sure
* there is no SSR'ed contents transformations happen after SSR is completed. Typically that
* happens either by CDN or during the build process as an optimization to remove comment nodes.
* Hydration process requires comment nodes produced by Angular to locate correct DOM segments.
* When this special marker is *not* present - throw an error and do not proceed with hydration,
* since it will not be able to function correctly.
*
* Note: this function is invoked only on the client, so it's safe to use DOM APIs.
*/
function verifySsrContentsIntegrity(): void {
const doc = getDocument();
let hydrationMarker: Node|undefined;
for (const node of doc.body.childNodes) {
if (node.nodeType === Node.COMMENT_NODE &&
node.textContent?.trim() === SSR_CONTENT_INTEGRITY_MARKER) {
hydrationMarker = node;
break;
}
}
if (!hydrationMarker) {
throw new RuntimeError(
RuntimeErrorCode.MISSING_SSR_CONTENT_INTEGRITY_MARKER,
typeof ngDevMode !== 'undefined' && ngDevMode &&
'Angular hydration logic detected that HTML content of this page was modified after it ' +
'was produced during server side rendering. Make sure that there are no optimizations ' +
'that remove comment nodes from HTML are enabled on your CDN. Angular hydration ' +
'relies on HTML produced by the server, including whitespaces and comment nodes.');
}
}
5 changes: 5 additions & 0 deletions packages/core/src/hydration/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ export const NGH_DATA_KEY = makeStateKey<Array<SerializedView>>(TRANSFER_STATE_T
*/
export const NGH_ATTR_NAME = 'ngh';

/**
* Marker used in a comment node to ensure hydration content integrity
*/
export const SSR_CONTENT_INTEGRITY_MARKER = 'nghm';

export const enum TextNodeMarker {

/**
Expand Down
1 change: 0 additions & 1 deletion packages/platform-browser/src/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ export function provideClientHydration(...features: HydrationFeature<HydrationFe
providers.push(ɵproviders);
}
}

return makeEnvironmentProviders([
(typeof ngDevMode !== 'undefined' && ngDevMode) ? provideZoneJsCompatibilityDetector() : [],
(featuresKind.has(HydrationFeatureKind.NoDomReuseFeature) ? [] : withDomHydration()),
Expand Down
64 changes: 33 additions & 31 deletions packages/platform-browser/test/hydration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {DOCUMENT} from '@angular/common';
import {HttpClient, provideHttpClient} from '@angular/common/http';
import {HttpTestingController, provideHttpClientTesting} from '@angular/common/http/testing';
import {ApplicationRef, Component, Injectable} from '@angular/core';
import {ApplicationRef, Component, Injectable, ɵSSR_CONTENT_INTEGRITY_MARKER as SSR_CONTENT_INTEGRITY_MARKER} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {withBody} from '@angular/private/testing';
import {BehaviorSubject} from 'rxjs';
Expand Down Expand Up @@ -37,23 +37,24 @@ describe('provideClientHydration', () => {
}

describe('default', () => {
beforeEach(withBody('<test-hydrate-app></test-hydrate-app>', () => {
TestBed.resetTestingModule();
beforeEach(withBody(
`<!--${SSR_CONTENT_INTEGRITY_MARKER}--><test-hydrate-app></test-hydrate-app>`, () => {
TestBed.resetTestingModule();

TestBed.configureTestingModule({
declarations: [SomeComponent],
providers: [
{provide: DOCUMENT, useFactory: () => document},
{provide: ApplicationRef, useClass: ApplicationRefPatched},
provideClientHydration(),
provideHttpClient(),
provideHttpClientTesting(),
],
});
TestBed.configureTestingModule({
declarations: [SomeComponent],
providers: [
{provide: DOCUMENT, useFactory: () => document},
{provide: ApplicationRef, useClass: ApplicationRefPatched},
provideClientHydration(),
provideHttpClient(),
provideHttpClientTesting(),
],
});

const appRef = TestBed.inject(ApplicationRef);
appRef.bootstrap(SomeComponent);
}));
const appRef = TestBed.inject(ApplicationRef);
appRef.bootstrap(SomeComponent);
}));

it(`should use cached HTTP calls`, () => {
makeRequestAndExpectOne('/test-1', 'foo');
Expand All @@ -63,23 +64,24 @@ describe('provideClientHydration', () => {
});

describe('withNoHttpTransferCache', () => {
beforeEach(withBody('<test-hydrate-app></test-hydrate-app>', () => {
TestBed.resetTestingModule();
beforeEach(withBody(
`<!--${SSR_CONTENT_INTEGRITY_MARKER}--><test-hydrate-app></test-hydrate-app>`, () => {
TestBed.resetTestingModule();

TestBed.configureTestingModule({
declarations: [SomeComponent],
providers: [
{provide: DOCUMENT, useFactory: () => document},
{provide: ApplicationRef, useClass: ApplicationRefPatched},
provideClientHydration(withNoHttpTransferCache()),
provideHttpClient(),
provideHttpClientTesting(),
],
});
TestBed.configureTestingModule({
declarations: [SomeComponent],
providers: [
{provide: DOCUMENT, useFactory: () => document},
{provide: ApplicationRef, useClass: ApplicationRefPatched},
provideClientHydration(withNoHttpTransferCache()),
provideHttpClient(),
provideHttpClientTesting(),
],
});

const appRef = TestBed.inject(ApplicationRef);
appRef.bootstrap(SomeComponent);
}));
const appRef = TestBed.inject(ApplicationRef);
appRef.bootstrap(SomeComponent);
}));

it(`should not cached HTTP calls`, () => {
makeRequestAndExpectOne('/test-1', 'foo');
Expand Down
19 changes: 17 additions & 2 deletions packages/platform-server/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ApplicationRef, InjectionToken, PlatformRef, Provider, Renderer2, StaticProvider, Type, ɵannotateForHydration as annotateForHydration, ɵENABLED_SSR_FEATURES as ENABLED_SSR_FEATURES, ɵInitialRenderPendingTasks as InitialRenderPendingTasks, ɵIS_HYDRATION_DOM_REUSE_ENABLED as IS_HYDRATION_DOM_REUSE_ENABLED} from '@angular/core';
import {ApplicationRef, InjectionToken, PlatformRef, Provider, Renderer2, StaticProvider, Type, ɵannotateForHydration as annotateForHydration, ɵENABLED_SSR_FEATURES as ENABLED_SSR_FEATURES, ɵIS_HYDRATION_DOM_REUSE_ENABLED as IS_HYDRATION_DOM_REUSE_ENABLED, ɵSSR_CONTENT_INTEGRITY_MARKER as SSR_CONTENT_INTEGRITY_MARKER} from '@angular/core';
import {first} from 'rxjs/operators';

import {PlatformState} from './platform_state';
Expand All @@ -31,6 +31,19 @@ function createServerPlatform(options: PlatformOptions): PlatformRef {
]);
}

/**
* Creates a marker comment node and append it into the `<body>`.
* Some CDNs have mechanisms to remove all comment node from HTML.
* This behaviour breaks hydration, so we'll detect on the client side if this
* marker comment is still available or else throw an error
*/
function appendSsrContentIntegrityMarker(doc: Document) {
// Adding a ng hydration marken comment
const comment = doc.createComment(SSR_CONTENT_INTEGRITY_MARKER);
doc.body.firstChild ? doc.body.insertBefore(comment, doc.body.firstChild) :
doc.body.append(comment);
}

/**
* Adds the `ng-server-context` attribute to host elements of all bootstrapped components
* within a given application.
Expand Down Expand Up @@ -60,7 +73,9 @@ async function _render(platformRef: PlatformRef, applicationRef: ApplicationRef)

const platformState = platformRef.injector.get(PlatformState);
if (applicationRef.injector.get(IS_HYDRATION_DOM_REUSE_ENABLED, false)) {
annotateForHydration(applicationRef, platformState.getDocument());
const doc = platformState.getDocument();
appendSsrContentIntegrityMarker(doc);
annotateForHydration(applicationRef, doc);
}

// Run any BEFORE_APP_SERIALIZED callbacks just before rendering to string.
Expand Down
15 changes: 11 additions & 4 deletions packages/platform-server/test/hydration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {CommonModule, DOCUMENT, isPlatformServer, NgComponentOutlet, NgFor, NgIf
import {MockPlatformLocation} from '@angular/common/testing';
import {afterRender, ApplicationRef, Component, ComponentRef, createComponent, destroyPlatform, Directive, ElementRef, EnvironmentInjector, ErrorHandler, getPlatform, inject, Injectable, Input, NgZone, PLATFORM_ID, Provider, TemplateRef, Type, ViewChild, ViewContainerRef, ViewEncapsulation, ɵsetDocument} from '@angular/core';
import {Console} from '@angular/core/src/console';
import {SSR_CONTENT_INTEGRITY_MARKER} from '@angular/core/src/hydration/utils';
import {getComponentDef} from '@angular/core/src/render3/definition';
import {NoopNgZone} from '@angular/core/src/zone/ng_zone';
import {TestBed} from '@angular/core/testing';
Expand Down Expand Up @@ -89,6 +90,10 @@ function convertHtmlToDom(html: string, doc: Document): HTMLElement {
return container;
}

function stripSsrIntegrityMarker(input: string): string {
return input.replace(`<!--${SSR_CONTENT_INTEGRITY_MARKER}-->`, '');
}

function stripTransferDataScript(input: string): string {
return input.replace(/<script (.*?)<\/script>/s, '');
}
Expand All @@ -105,7 +110,8 @@ function whenStable(appRef: ApplicationRef): Promise<void> {
function verifyClientAndSSRContentsMatch(ssrContents: string, clientAppRootElement: HTMLElement) {
const clientContents =
stripTransferDataScript(stripUtilAttributes(clientAppRootElement.outerHTML, false));
ssrContents = stripTransferDataScript(stripUtilAttributes(ssrContents, false));
ssrContents =
stripSsrIntegrityMarker(stripTransferDataScript(stripUtilAttributes(ssrContents, false)));
expect(clientContents).toBe(ssrContents, 'Client and server contents mismatch');
}

Expand Down Expand Up @@ -280,7 +286,7 @@ describe('platform-server hydration integration', () => {
hydrationFeatures: HydrationFeature<HydrationFeatureKind>[] = []): Promise<ApplicationRef> {
// Get HTML contents of the `<app>`, create a DOM element and append it into the body.
const container = convertHtmlToDom(html, doc);
Array.from(container.children).forEach(node => doc.body.appendChild(node));
Array.from(container.childNodes).forEach(node => doc.body.appendChild(node));

function _document(): any {
ɵsetDocument(doc);
Expand Down Expand Up @@ -4122,14 +4128,15 @@ describe('platform-server hydration integration', () => {
appRef.tick();

const clientRootNode = compRef.location.nativeElement;
const portalRootNode = clientRootNode.ownerDocument.body.firstChild;
const portalRootNode = clientRootNode.ownerDocument.querySelector('portal-app');
verifyAllNodesClaimedForHydration(clientRootNode);
verifyAllNodesClaimedForHydration(portalRootNode.firstChild);
const clientContents = stripUtilAttributes(portalRootNode.outerHTML, false) +
stripUtilAttributes(clientRootNode.outerHTML, false);
expect(clientContents)
.toBe(
stripUtilAttributes(stripTransferDataScript(ssrContents), false),
stripSsrIntegrityMarker(
stripUtilAttributes(stripTransferDataScript(ssrContents), false)),
'Client and server contents mismatch');
});

Expand Down
17 changes: 17 additions & 0 deletions packages/platform-server/test/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {DOCUMENT, isPlatformServer, PlatformLocation, ɵgetDOM as getDOM} from '
import {HTTP_INTERCEPTORS, HttpClient, HttpClientModule, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest} from '@angular/common/http';
import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing';
import {ApplicationConfig, ApplicationRef, Component, destroyPlatform, EnvironmentProviders, getPlatform, HostListener, Inject, inject as coreInject, Injectable, Input, makeStateKey, mergeApplicationConfig, NgModule, NgZone, PLATFORM_ID, Provider, TransferState, Type, ViewEncapsulation} from '@angular/core';
import {SSR_CONTENT_INTEGRITY_MARKER} from '@angular/core/src/hydration/utils';
import {InitialRenderPendingTasks} from '@angular/core/src/initial_render_pending_tasks';
import {TestBed} from '@angular/core/testing';
import {bootstrapApplication, BrowserModule, provideClientHydration, Title, withNoDomReuse, withNoHttpTransferCache} from '@angular/platform-browser';
Expand Down Expand Up @@ -870,6 +871,22 @@ describe('platform-server integration', () => {
expect(output).toMatch(/ng-server-context="other"/);
});

it('appends SSR integrity marker comment when hydration is enabled', async () => {
@Component({
standalone: true,
selector: 'app',
template: ``,
})
class SimpleApp {
}

const bootstrap = renderApplication(
getStandaloneBoostrapFn(SimpleApp, [provideClientHydration()]), {document: doc});
// HttpClient cache and DOM hydration are enabled by default.
const output = await bootstrap;
expect(output).toContain(`<body><!--${SSR_CONTENT_INTEGRITY_MARKER}-->`);
});

it('includes a set of features into `ng-server-context` attribute', async () => {
const options = {
document: doc,
Expand Down
2 changes: 1 addition & 1 deletion packages/tsconfig-build.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"target": "es2022",
// Keep the below in sync with ng_module.bzl
"useDefineForClassFields": false,
"lib": ["es2020", "dom"],
"lib": ["es2020", "dom", "dom.iterable"],
"skipLibCheck": true,
// don't auto-discover @types/node, it results in a ///<reference in the .d.ts output
"types": [],
Expand Down
2 changes: 1 addition & 1 deletion packages/tsconfig-tsec-base.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"extends": "./tsconfig-build.json",
"compilerOptions": {
"noEmit": true,
"lib": ["es2020", "dom"],
"lib": ["es2020", "dom", "dom.iterable"],
"plugins": [{"name": "tsec", "exemptionConfig": "./tsec-exemption.json"}]
}
}
2 changes: 1 addition & 1 deletion packages/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
},
"rootDir": ".",
"inlineSourceMap": true,
"lib": ["es2020", "dom"],
"lib": ["es2020", "dom", "dom.iterable"],
"skipDefaultLibCheck": true,
"skipLibCheck": true,
"types": ["angular"]
Expand Down

0 comments on commit 0a38dc3

Please sign in to comment.