Skip to content

Commit

Permalink
fix(platform-server): call onSerialize when state is empty (#47888)
Browse files Browse the repository at this point in the history 
Commit a0b2d36#diff-3975e0ee5aa3e06ecbcd76f5fa5134612f7fd2e6802ca7d370973bd410aab55cR25-R31 changed the serialization phase logic so
that when the state is empty the script tag is not added to the document. As a side effect, this caused the `toJson` not called which caused the `onSerialize` callbacks also not to be called.
These callbacks are used to provide the value for a key when `toJson` is called. Example: ngrx/platform#101 (comment)

Closes #47172

PR Close #47888
  • Loading branch information
@alan-agius4 @alxhub
alan-agius4 authored and alxhub committed Oct 28, 2022
1 parent 8707475 commit d2d9bbf
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 104 deletions.
7 changes: 6 additions & 1 deletion packages/platform-server/src/transfer_state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,20 @@ export const TRANSFER_STATE_SERIALIZATION_PROVIDERS: Provider[] = [{

function serializeTransferStateFactory(doc: Document, appId: string, transferStore: TransferState) {
return () => {
// The `.toJSON` here causes the `onSerialize` callbacks to be called.
// These callbacks can be used to provide the value for a given key.
const content = transferStore.toJson();

if (transferStore.isEmpty) {
// The state is empty, nothing to transfer,
// avoid creating an extra `<script>` tag in this case.
return;
}

const script = doc.createElement('script');
script.id = appId + '-state';
script.setAttribute('type', 'application/json');
script.textContent = escapeHtml(transferStore.toJson());
script.textContent = escapeHtml(content);
doc.body.appendChild(script);
};
}
Expand Down
105 changes: 3 additions & 102 deletions packages/platform-server/test/integration_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import {animate, AnimationBuilder, state, style, transition, trigger} from '@ang
import {DOCUMENT, isPlatformServer, PlatformLocation, ɵgetDOM as getDOM} from '@angular/common';
import {HTTP_INTERCEPTORS, HttpClient, HttpClientModule, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest} from '@angular/common/http';
import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing';
import {ApplicationRef, CompilerFactory, Component, destroyPlatform, getPlatform, HostBinding, HostListener, importProvidersFrom, Inject, Injectable, Input, NgModule, NgZone, OnInit, PLATFORM_ID, PlatformRef, Type, ViewEncapsulation} from '@angular/core';
import {ApplicationRef, CompilerFactory, Component, destroyPlatform, getPlatform, HostListener, Inject, Injectable, Input, NgModule, NgZone, PLATFORM_ID, PlatformRef, ViewEncapsulation} from '@angular/core';
import {inject, TestBed, waitForAsync} from '@angular/core/testing';
import {BrowserModule, makeStateKey, Title, TransferState} from '@angular/platform-browser';
import {BEFORE_APP_SERIALIZED, INITIAL_CONFIG, platformDynamicServer, PlatformState, renderModule, renderModuleFactory, ServerModule, ServerTransferStateModule} from '@angular/platform-server';
import {BrowserModule, Title} from '@angular/platform-browser';
import {BEFORE_APP_SERIALIZED, INITIAL_CONFIG, platformDynamicServer, PlatformState, renderModule, renderModuleFactory, ServerModule} from '@angular/platform-server';
import {Observable} from 'rxjs';
import {first} from 'rxjs/operators';

Expand Down Expand Up @@ -389,19 +389,6 @@ const [MyHostComponentStandalone, _] = createFalseAttributesComponents(true);
class FalseAttributesModule {
}

@Component({selector: 'app', template: '<div [innerText]="foo"></div>'})
class InnerTextComponent {
foo = 'Some text';
}

@NgModule({
declarations: [InnerTextComponent],
bootstrap: [InnerTextComponent],
imports: [ServerModule, BrowserModule.withServerTransition({appId: 'inner-text'})]
})
class InnerTextModule {
}

function createMyInputComponent(standalone: boolean) {
@Component({
standalone,
Expand Down Expand Up @@ -449,47 +436,6 @@ const HTMLTypesAppStandalone = createHTMLTypesApp(true);
class HTMLTypesModule {
}

const TEST_KEY = makeStateKey<number>('test');
const STRING_KEY = makeStateKey<string>('testString');

@Component({selector: 'app', template: 'Works!'})
class TransferComponent {
constructor(private transferStore: TransferState) {}
ngOnInit() {
this.transferStore.set(TEST_KEY, 10);
}
}

@Component({selector: 'esc-app', template: 'Works!'})
class EscapedComponent {
constructor(private transferStore: TransferState) {}
ngOnInit() {
this.transferStore.set(STRING_KEY, '</script><script>alert(\'Hello&\' + "World");');
}
}

@NgModule({
bootstrap: [TransferComponent],
declarations: [TransferComponent],
imports: [
BrowserModule.withServerTransition({appId: 'transfer'}),
ServerModule,
]
})
class TransferStoreModule {
}

@NgModule({
bootstrap: [EscapedComponent],
declarations: [EscapedComponent],
imports: [
BrowserModule.withServerTransition({appId: 'transfer'}),
ServerModule,
]
})
class EscapedTransferStoreModule {
}

function createMyHiddenComponent(standalone: boolean) {
@Component({
standalone,
Expand Down Expand Up @@ -1310,50 +1256,5 @@ describe('platform-server integration', () => {
});
});
});

describe('ServerTransferStoreModule', () => {
let called = false;
const defaultExpectedOutput =
'<html><head></head><body><app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</app><script id="transfer-state" type="application/json">{&q;test&q;:10}</script></body></html>';

beforeEach(() => {
called = false;
});
afterEach(() => {
expect(called).toBe(true);
});

it('adds transfer script tag when using renderModule', waitForAsync(() => {
renderModule(TransferStoreModule, {document: '<app></app>'}).then(output => {
expect(output).toBe(defaultExpectedOutput);
called = true;
});
}));

it('adds transfer script tag when using renderModuleFactory',
waitForAsync(inject([PlatformRef], (defaultPlatform: PlatformRef) => {
const compilerFactory: CompilerFactory =
defaultPlatform.injector.get(CompilerFactory, null)!;
const moduleFactory =
compilerFactory.createCompiler().compileModuleSync(TransferStoreModule);
renderModuleFactory(moduleFactory, {document: '<app></app>'}).then(output => {
expect(output).toBe(defaultExpectedOutput);
called = true;
});
})));

it('cannot break out of <script> tag in serialized output', waitForAsync(() => {
renderModule(EscapedTransferStoreModule, {
document: '<esc-app></esc-app>'
}).then(output => {
expect(output).toBe(
'<html><head></head><body><esc-app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</esc-app>' +
'<script id="transfer-state" type="application/json">' +
'{&q;testString&q;:&q;&l;/script&g;&l;script&g;' +
'alert(&s;Hello&a;&s; + \\&q;World\\&q;);&q;}</script></body></html>');
called = true;
});
}));
});
});
})();
86 changes: 86 additions & 0 deletions packages/platform-server/test/transfer_state_spec.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {Component, NgModule,} from '@angular/core';
import {BrowserModule, makeStateKey, TransferState} from '@angular/platform-browser';
import {renderModule, ServerModule} from '@angular/platform-server';

describe('transfer_state', () => {
const defaultExpectedOutput =
'<html><head></head><body><app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</app><script id="transfer-state" type="application/json">{&q;test&q;:10}</script></body></html>';

it('adds transfer script tag when using renderModule', async () => {
const STATE_KEY = makeStateKey<number>('test');

@Component({selector: 'app', template: 'Works!'})
class TransferComponent {
constructor(private transferStore: TransferState) {
this.transferStore.set(STATE_KEY, 10);
}
}

@NgModule({
bootstrap: [TransferComponent],
declarations: [TransferComponent],
imports: [BrowserModule.withServerTransition({appId: 'transfer'}), ServerModule],
})
class TransferStoreModule {
}

const output = await renderModule(TransferStoreModule, {document: '<app></app>'});
expect(output).toBe(defaultExpectedOutput);
});

it('cannot break out of <script> tag in serialized output', async () => {
const STATE_KEY = makeStateKey<string>('testString');

@Component({selector: 'esc-app', template: 'Works!'})
class EscapedComponent {
constructor(private transferStore: TransferState) {
this.transferStore.set(STATE_KEY, '</script><script>alert(\'Hello&\' + "World");');
}
}
@NgModule({
bootstrap: [EscapedComponent],
declarations: [EscapedComponent],
imports: [BrowserModule.withServerTransition({appId: 'transfer'}), ServerModule],
})
class EscapedTransferStoreModule {
}

const output =
await renderModule(EscapedTransferStoreModule, {document: '<esc-app></esc-app>'});
expect(output).toBe(
'<html><head></head><body><esc-app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</esc-app>' +
'<script id="transfer-state" type="application/json">' +
'{&q;testString&q;:&q;&l;/script&g;&l;script&g;' +
'alert(&s;Hello&a;&s; + \\&q;World\\&q;);&q;}</script></body></html>');
});

it('adds transfer script tag when setting state during onSerialize', async () => {
const STATE_KEY = makeStateKey<number>('test');

@Component({selector: 'app', template: 'Works!'})
class TransferComponent {
constructor(private transferStore: TransferState) {
this.transferStore.onSerialize(STATE_KEY, () => 10);
}
}

@NgModule({
bootstrap: [TransferComponent],
declarations: [TransferComponent],
imports: [BrowserModule.withServerTransition({appId: 'transfer'}), ServerModule],
})
class TransferStoreModule {
}

const output = await renderModule(TransferStoreModule, {document: '<app></app>'});
expect(output).toBe(defaultExpectedOutput);
});
});
2 changes: 1 addition & 1 deletion packages/platform-server/testing/src/server.ts
View file
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 {createPlatformFactory, NgModule, PlatformRef, StaticProvider} from '@angular/core';
import {createPlatformFactory, NgModule} from '@angular/core';
import {BrowserDynamicTestingModule, ɵplatformCoreDynamicTesting as platformCoreDynamicTesting} from '@angular/platform-browser-dynamic/testing';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {ɵINTERNAL_SERVER_PLATFORM_PROVIDERS as INTERNAL_SERVER_PLATFORM_PROVIDERS, ɵSERVER_RENDER_PROVIDERS as SERVER_RENDER_PROVIDERS} from '@angular/platform-server';
Expand Down

0 comments on commit d2d9bbf

Please sign in to comment.