From 0a38dc3c267562a842f4514092f32a4b0720b53c Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Wed, 26 Jul 2023 00:16:47 +0200 Subject: [PATCH] refactor(core): throw an error when hydration marker is missing from 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 --- goldens/public-api/core/errors.md | 2 + packages/core/src/core_private_export.ts | 1 + packages/core/src/errors.ts | 1 + packages/core/src/hydration/api.ts | 42 ++++++++++-- packages/core/src/hydration/utils.ts | 5 ++ packages/platform-browser/src/hydration.ts | 1 - .../platform-browser/test/hydration_spec.ts | 64 ++++++++++--------- packages/platform-server/src/utils.ts | 19 +++++- .../platform-server/test/hydration_spec.ts | 15 +++-- .../platform-server/test/integration_spec.ts | 17 +++++ packages/tsconfig-build.json | 2 +- packages/tsconfig-tsec-base.json | 2 +- packages/tsconfig.json | 2 +- 13 files changed, 127 insertions(+), 46 deletions(-) diff --git a/goldens/public-api/core/errors.md b/goldens/public-api/core/errors.md index dce5a8b3e5f7a..ae441f35e971d 100644 --- a/goldens/public-api/core/errors.md +++ b/goldens/public-api/core/errors.md @@ -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, diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index df8b889bfd7fe..ad42ce266a011 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -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'; diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 074571c07d4fa..6009350296b2e 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -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, diff --git a/packages/core/src/hydration/api.ts b/packages/core/src/hydration/api.ts index b726b95f27ddf..3155210aa9d22 100644 --- a/packages/core/src/hydration/api.ts +++ b/packages/core/src/hydration/api.ts @@ -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. @@ -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(); } }, @@ -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.'); + } +} diff --git a/packages/core/src/hydration/utils.ts b/packages/core/src/hydration/utils.ts index a72fbd420d173..900e59d53b1b8 100644 --- a/packages/core/src/hydration/utils.ts +++ b/packages/core/src/hydration/utils.ts @@ -36,6 +36,11 @@ export const NGH_DATA_KEY = makeStateKey>(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 { /** diff --git a/packages/platform-browser/src/hydration.ts b/packages/platform-browser/src/hydration.ts index 54865f9645289..5d60091b1855f 100644 --- a/packages/platform-browser/src/hydration.ts +++ b/packages/platform-browser/src/hydration.ts @@ -169,7 +169,6 @@ export function provideClientHydration(...features: HydrationFeature { } describe('default', () => { - beforeEach(withBody('', () => { - TestBed.resetTestingModule(); + beforeEach(withBody( + ``, () => { + 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'); @@ -63,23 +64,24 @@ describe('provideClientHydration', () => { }); describe('withNoHttpTransferCache', () => { - beforeEach(withBody('', () => { - TestBed.resetTestingModule(); + beforeEach(withBody( + ``, () => { + 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'); diff --git a/packages/platform-server/src/utils.ts b/packages/platform-server/src/utils.ts index 78ed21c4a3c02..51e56ca980308 100644 --- a/packages/platform-server/src/utils.ts +++ b/packages/platform-server/src/utils.ts @@ -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'; @@ -31,6 +31,19 @@ function createServerPlatform(options: PlatformOptions): PlatformRef { ]); } +/** + * Creates a marker comment node and append it into the ``. + * 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. @@ -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. diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts index 88675099e44c0..c49d3eec9d769 100644 --- a/packages/platform-server/test/hydration_spec.ts +++ b/packages/platform-server/test/hydration_spec.ts @@ -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'; @@ -89,6 +90,10 @@ function convertHtmlToDom(html: string, doc: Document): HTMLElement { return container; } +function stripSsrIntegrityMarker(input: string): string { + return input.replace(``, ''); +} + function stripTransferDataScript(input: string): string { return input.replace(/