Skip to content

Commit 0243722

Browse files
alan-agius4AndrewKushnir
alan-agius4
authored and
AndrewKushnir
committed
refactor(core): simplify state transfer escaping (#50201)
This commit removes unnecessary transfer state escaping and updates this process to be done by the means of a `replacer` and `reviver` method as this removes the need to export the escaping and unescaping methods. The only thing that we need to escape is `<script` and `</script` which are done by the browsers, but not Node.js. PR Close #50201
1 parent ea3b5f1 commit 0243722

File tree

7 files changed

+40
-54
lines changed

7 files changed

+40
-54
lines changed

‎packages/core/src/core_private_export.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ export {_sanitizeHtml as ɵ_sanitizeHtml} from './sanitization/html_sanitizer';
3030
export {_sanitizeUrl as ɵ_sanitizeUrl} from './sanitization/url_sanitizer';
3131
export {setAlternateWeakRefImpl as ɵsetAlternateWeakRefImpl} from './signals';
3232
export {TESTABILITY as ɵTESTABILITY, TESTABILITY_GETTER as ɵTESTABILITY_GETTER} from './testability/testability';
33-
export {escapeTransferStateContent as ɵescapeTransferStateContent, unescapeTransferStateContent as ɵunescapeTransferStateContent} from './transfer_state';
3433
export {coerceToBoolean as ɵcoerceToBoolean} from './util/coercion';
3534
export {devModeEqual as ɵdevModeEqual} from './util/comparison';
3635
export {global as ɵglobal} from './util/global';

‎packages/core/src/transfer_state.ts‎

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,28 +11,6 @@ import {inject} from './di/injector_compatibility';
1111
import {ɵɵdefineInjectable} from './di/interface/defs';
1212
import {getDocument} from './render3/interfaces/document';
1313

14-
export function escapeTransferStateContent(text: string): string {
15-
const escapedText: {[k: string]: string} = {
16-
'&': '&a;',
17-
'"': '&q;',
18-
'\'': '&s;',
19-
'<': '&l;',
20-
'>': '&g;',
21-
};
22-
return text.replace(/[&"'<>]/g, s => escapedText[s]);
23-
}
24-
25-
export function unescapeTransferStateContent(text: string): string {
26-
const unescapedText: {[k: string]: string} = {
27-
'&a;': '&',
28-
'&q;': '"',
29-
'&s;': '\'',
30-
'&l;': '<',
31-
'&g;': '>',
32-
};
33-
return text.replace(/&[^;]+;/g, s => unescapedText[s]);
34-
}
35-
3614
/**
3715
* A type-safe key to use with `TransferState`.
3816
*
@@ -70,7 +48,7 @@ export function makeStateKey<T = void>(key: string): StateKey<T> {
7048
return key as StateKey<T>;
7149
}
7250

73-
function initTransferState() {
51+
function initTransferState(): TransferState {
7452
const transferState = new TransferState();
7553
if (inject(PLATFORM_ID) === 'browser') {
7654
transferState.store = retrieveTransferredState(getDocument(), inject(APP_ID));
@@ -132,7 +110,7 @@ export class TransferState {
132110
/**
133111
* Test whether a key exists in the store.
134112
*/
135-
hasKey<T>(key: StateKey<T>) {
113+
hasKey<T>(key: StateKey<T>): boolean {
136114
return this.store.hasOwnProperty(key);
137115
}
138116

@@ -164,7 +142,10 @@ export class TransferState {
164142
}
165143
}
166144
}
167-
return JSON.stringify(this.store);
145+
146+
// Escape script tag to avoid break out of <script> tag in serialized output.
147+
// Encoding of `<` is the same behaviour as G3 script_builders.
148+
return JSON.stringify(this.store).replace(/</g, '\\u003C');
168149
}
169150
}
170151

@@ -175,7 +156,9 @@ function retrieveTransferredState(doc: Document, appId: string): Record<string,
175156
if (script?.textContent) {
176157
try {
177158
// Avoid using any here as it triggers lint errors in google3 (any is not allowed).
178-
return JSON.parse(unescapeTransferStateContent(script.textContent)) as {};
159+
// Decoding of `<` is done of the box by browsers and node.js, same behaviour as G3
160+
// script_builders.
161+
return JSON.parse(script.textContent) as {};
179162
} catch (e) {
180163
console.warn('Exception while restoring TransferState for app ' + appId, e);
181164
}

‎packages/core/test/transfer_state_spec.ts‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,21 @@ import {APP_ID as APP_ID_TOKEN, PLATFORM_ID} from '@angular/core';
1010
import {TestBed} from '@angular/core/testing';
1111

1212
import {getDocument} from '../src/render3/interfaces/document';
13-
import {escapeTransferStateContent, makeStateKey, TransferState, unescapeTransferStateContent} from '../src/transfer_state';
13+
import {makeStateKey, TransferState} from '../src/transfer_state';
1414

15-
(function() {
1615
function removeScriptTag(doc: Document, id: string) {
1716
const existing = doc.getElementById(id);
1817
if (existing) {
1918
doc.body.removeChild(existing);
2019
}
2120
}
2221

23-
function addScriptTag(doc: Document, appId: string, data: {}) {
22+
function addScriptTag(doc: Document, appId: string, data: object|string) {
2423
const script = doc.createElement('script');
2524
const id = appId + '-state';
2625
script.id = id;
2726
script.setAttribute('type', 'application/json');
28-
script.textContent = escapeTransferStateContent(JSON.stringify(data));
27+
script.textContent = typeof data === 'string' ? data : JSON.stringify(data);
2928

3029
// Remove any stale script tags.
3130
removeScriptTag(doc, id);
@@ -129,19 +128,26 @@ describe('TransferState', () => {
129128
transferState.remove(TEST_KEY);
130129
expect(transferState.isEmpty).toBeTrue();
131130
});
132-
});
133131

134-
describe('escape/unescape', () => {
135-
it('works with all escaped characters', () => {
136-
const testString = '</script><script>alert(\'Hello&\' + "World");';
137-
const testObj = {testString};
138-
const escaped = escapeTransferStateContent(JSON.stringify(testObj));
139-
expect(escaped).toBe(
140-
'{&q;testString&q;:&q;&l;/script&g;&l;script&g;' +
141-
'alert(&s;Hello&a;&s; + \\&q;World\\&q;);&q;}');
142-
143-
const unescapedObj = JSON.parse(unescapeTransferStateContent(escaped)) as {testString: string};
144-
expect(unescapedObj['testString']).toBe(testString);
132+
it('should encode `<` to avoid breaking out of <script> tag in serialized output', () => {
133+
const transferState = TestBed.inject(TransferState);
134+
135+
// The state is empty initially.
136+
expect(transferState.isEmpty).toBeTrue();
137+
138+
transferState.set(DELAYED_KEY, '</script><script>alert(\'Hello&\' + "World");');
139+
expect(transferState.toJson())
140+
.toBe(`{"delayed":"\\u003C/script>\\u003Cscript>alert('Hello&' + \\"World\\");"}`);
141+
});
142+
143+
it('should decode `\\u003C` (<) when restoring stating', () => {
144+
const encodedState =
145+
`{"delayed":"\\u003C/script>\\u003Cscript>alert('Hello&' + \\"World\\");"}`;
146+
addScriptTag(doc, APP_ID, encodedState);
147+
const transferState = TestBed.inject(TransferState);
148+
149+
expect(transferState.toJson()).toBe(encodedState);
150+
expect(transferState.get(DELAYED_KEY, null))
151+
.toBe('</script><script>alert(\'Hello&\' + "World");');
145152
});
146153
});
147-
})();

‎packages/platform-server/src/transfer_state.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {DOCUMENT} from '@angular/common';
10-
import {APP_ID, NgModule, Provider, TransferState, ɵescapeTransferStateContent as escapeTransferStateContent} from '@angular/core';
10+
import {APP_ID, NgModule, Provider, TransferState} from '@angular/core';
1111

1212
import {BEFORE_APP_SERIALIZED} from './tokens';
1313

@@ -33,7 +33,7 @@ function serializeTransferStateFactory(doc: Document, appId: string, transferSto
3333
const script = doc.createElement('script');
3434
script.id = appId + '-state';
3535
script.setAttribute('type', 'application/json');
36-
script.textContent = escapeTransferStateContent(content);
36+
script.textContent = content;
3737

3838
// It is intentional that we add the script at the very bottom. Angular CLI script tags for
3939
// bundles are always `type="module"`. These are deferred by default and cause the transfer

‎packages/platform-server/test/hydration_spec.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {ApplicationRef, Component, ComponentRef, createComponent, destroyPlatfor
1414
import {Console} from '@angular/core/src/console';
1515
import {InitialRenderPendingTasks} from '@angular/core/src/initial_render_pending_tasks';
1616
import {getComponentDef} from '@angular/core/src/render3/definition';
17-
import {unescapeTransferStateContent} from '@angular/core/src/transfer_state';
1817
import {NoopNgZone} from '@angular/core/src/zone/ng_zone';
1918
import {TestBed} from '@angular/core/testing';
2019
import {bootstrapApplication, HydrationFeature, HydrationFeatureKind, provideClientHydration, withNoDomReuse} from '@angular/platform-browser';
@@ -185,9 +184,8 @@ function resetTViewsFor(...types: Type<unknown>[]) {
185184
}
186185
}
187186

188-
function getHydrationInfoFromTransferState(input: string): string|null {
189-
const rawContents = input.match(/<script[^>]+>(.*?)<\/script>/)?.[1];
190-
return rawContents ? unescapeTransferStateContent(rawContents) : null;
187+
function getHydrationInfoFromTransferState(input: string): string|undefined {
188+
return input.match(/<script[^>]+>(.*?)<\/script>/)?.[1];
191189
}
192190

193191
function withNoopErrorHandler() {

‎packages/platform-server/test/integration_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ describe('platform-server integration', () => {
842842
renderModule(MyTransferStateModule, options);
843843
const expectedOutput =
844844
'<html><head></head><body><app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other"><div>Works!</div></app>' +
845-
'<script id="ng-state" type="application/json">{&q;some-key&q;:&q;some-value&q;}</script></body></html>';
845+
'<script id="ng-state" type="application/json">{"some-key":"some-value"}</script></body></html>';
846846
const output = await bootstrap;
847847
expect(output).toEqual(expectedOutput);
848848
});

‎packages/platform-server/test/transfer_state_spec.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {renderModule, ServerModule} from '@angular/platform-server';
1212

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

1717
it('adds transfer script tag when using renderModule', async () => {
1818
const STATE_KEY = makeStateKey<number>('test');
@@ -58,8 +58,8 @@ describe('transfer_state', () => {
5858
expect(output).toBe(
5959
'<html><head></head><body><esc-app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</esc-app>' +
6060
'<script id="ng-state" type="application/json">' +
61-
'{&q;testString&q;:&q;&l;/script&g;&l;script&g;' +
62-
'alert(&s;Hello&a;&s; + \\&q;World\\&q;);&q;}</script></body></html>');
61+
`{"testString":"\\u003C/script>\\u003Cscript>alert('Hello&' + \\"World\\");"}` +
62+
'</script></body></html>');
6363
});
6464

6565
it('adds transfer script tag when setting state during onSerialize', async () => {

0 commit comments

Comments
 (0)