Skip to content

Commit 3fa72d9

Browse files
author
Mat Brown
committed
Don’t navigate to data URIs at the top level
Chrome no longer supports navigation to data URIs in the top level window, for security reasons. So, for the preview as well as the spinner that is displayed until gist export completes, we instead populate the new window using good old `document.write`. There is a minor concern here about security—in particular, we don’t want malicious imported code to be able to access the Popcode environment and attempt to e.g. hijack the user’s GitHub session. I *think* the implementation herein prevents such an attack by removing the `opener` property on the new window before populating its content, in principle making it impossible for the opened window to access the Popcode environment. This approach is discussed e.g. [in a Chromium thread discussing `window.opener` security risks](https://bugs.chromium.org/p/chromium/issues/detail?id=168988#c14) and seems to be generally agreed to be sufficient. I could not find a way to access `opener` from a window opened by Popcode.
1 parent 4ecf983 commit 3fa72d9

3 files changed

Lines changed: 19 additions & 50 deletions

File tree

src/sagas/ui.js

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
import {all, call, put, take, takeEvery} from 'redux-saga/effects';
22
import debounceFor from 'redux-saga-debounce-effect/src/debounceFor';
3-
import {TextEncoder} from 'text-encoding';
4-
import base64 from 'base64-js';
53
import {userDoneTyping as userDoneTypingAction} from '../actions/ui';
64
import {
75
gistExportDisplayed,
86
gistExportNotDisplayed,
97
repoExportDisplayed,
108
repoExportNotDisplayed,
119
} from '../actions/clients';
12-
import {openWindowWithWorkaroundForChromeClosingBug} from '../util';
10+
import {openWindowWithContent} from '../util';
1311
import generatePreview from '../util/generatePreview';
14-
import {spinnerPage} from '../templates';
12+
import spinnerPageHtml from '../../templates/github-export.html';
1513

1614
export function* userDoneTyping() {
1715
yield put(userDoneTypingAction());
@@ -22,10 +20,7 @@ function* githubExport(
2220
failureAction,
2321
notDisplayedAction,
2422
displayedAction) {
25-
const exportWindow = yield call(
26-
openWindowWithWorkaroundForChromeClosingBug,
27-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
28-
);
23+
const exportWindow = yield call(openWindowWithContent, spinnerPageHtml);
2924
const {type, payload: url} =
3025
yield take([successAction, failureAction]);
3126

@@ -52,10 +47,7 @@ export function* exportGist() {
5247

5348
export function* popOutProject({payload: project}) {
5449
const preview = yield call(generatePreview, project);
55-
const uint8array = new TextEncoder('utf-8').encode(preview);
56-
const base64encoded = base64.fromByteArray(uint8array);
57-
const url = `data:text/html;charset=utf-8;base64,${base64encoded}`;
58-
yield call(openWindowWithWorkaroundForChromeClosingBug, url);
50+
yield call(openWindowWithContent, preview);
5951
}
6052

6153
export function* exportRepo() {

src/util/index.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
export function openWindowWithWorkaroundForChromeClosingBug(
2-
location, name = '_blank',
3-
) {
1+
export function openWindowWithContent(content, name = '_blank') {
42
const newWindow = open('about:blank', name);
5-
newWindow.location.href = location;
3+
Reflect.deleteProperty(newWindow, 'opener');
4+
const {document} = newWindow;
5+
document.open();
6+
document.write(content);
7+
document.close();
68
return newWindow;
79
}

test/unit/sagas/ui.js

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import test from 'tape';
22
import {testSaga} from 'redux-saga-test-plan';
3-
import {TextEncoder} from 'text-encoding';
4-
import base64 from 'base64-js';
53
import {
64
userDoneTyping as userDoneTypingSaga,
75
exportGist as exportGistSaga,
@@ -19,8 +17,8 @@ import {
1917
repoExportError,
2018
repoExportNotDisplayed,
2119
} from '../../../src/actions/clients';
22-
import {openWindowWithWorkaroundForChromeClosingBug} from '../../../src/util';
23-
import {spinnerPage} from '../../../src/templates';
20+
import {openWindowWithContent} from '../../../src/util';
21+
import spinnerPageHtml from '../../../templates/github-export.html';
2422
import generatePreview from '../../../src/util/generatePreview';
2523

2624
test('userDoneTyping', (assert) => {
@@ -36,10 +34,7 @@ test('exportGist', (t) => {
3634
const mockWindow = {closed: false, location: {}};
3735
const url = 'https://gist.github.com/abc123';
3836
testSaga(exportGistSaga).
39-
next().call(
40-
openWindowWithWorkaroundForChromeClosingBug,
41-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
42-
).
37+
next().call(openWindowWithContent, spinnerPageHtml).
4338
next(mockWindow).take(['GIST_EXPORTED', 'GIST_EXPORT_ERROR']).
4439
next(gistExported(url)).put(gistExportDisplayed()).
4540
next().isDone();
@@ -53,10 +48,7 @@ test('exportGist', (t) => {
5348
const mockWindow = {closed: true, location: {}};
5449
const url = 'https://gist.github.com/abc123';
5550
testSaga(exportGistSaga).
56-
next().call(
57-
openWindowWithWorkaroundForChromeClosingBug,
58-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
59-
).
51+
next().call(openWindowWithContent, spinnerPageHtml).
6052
next(mockWindow).take(['GIST_EXPORTED', 'GIST_EXPORT_ERROR']).
6153
next(gistExported(url)).put(gistExportNotDisplayed(url)).
6254
next().isDone();
@@ -69,10 +61,7 @@ test('exportGist', (t) => {
6961
t.test('with gist export error', (assert) => {
7062
const mockWindow = {closed: false, close() { }};
7163
testSaga(exportGistSaga).
72-
next().call(
73-
openWindowWithWorkaroundForChromeClosingBug,
74-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
75-
).
64+
next().call(openWindowWithContent, spinnerPageHtml).
7665
next(mockWindow).take(['GIST_EXPORTED', 'GIST_EXPORT_ERROR']).
7766
next(gistExportError(new Error())).call([mockWindow, 'close']).
7867
next().isDone();
@@ -86,14 +75,9 @@ test('popOutProject', (assert) => {
8675
const mockWindow = {closed: false, close() { }};
8776
const project = {};
8877
const preview = '<html></html>';
89-
const uint8array = new TextEncoder('utf-8').encode(preview);
90-
const base64encoded = base64.fromByteArray(uint8array);
9178
testSaga(popOutProjectSaga, popOutProject(project)).
9279
next().call(generatePreview, project).
93-
next(preview).call(
94-
openWindowWithWorkaroundForChromeClosingBug,
95-
`data:text/html;charset=utf-8;base64,${base64encoded}`,
96-
).
80+
next(preview).call(openWindowWithContent, preview).
9781
next(mockWindow).isDone();
9882
assert.end();
9983
});
@@ -103,10 +87,7 @@ test('exportRepo', (t) => {
10387
const mockWindow = {closed: false, location: {}};
10488
const url = 'https://popcode-mat.github.io/my-popcode-repo';
10589
testSaga(exportRepoSaga).
106-
next().call(
107-
openWindowWithWorkaroundForChromeClosingBug,
108-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
109-
).
90+
next().call(openWindowWithContent, spinnerPageHtml).
11091
next(mockWindow).take(['REPO_EXPORTED', 'REPO_EXPORT_ERROR']).
11192
next(repoExported(url)).put(repoExportDisplayed()).
11293
next().isDone();
@@ -120,10 +101,7 @@ test('exportRepo', (t) => {
120101
const mockWindow = {closed: true, location: {}};
121102
const url = 'https://popcode-mat.github.io/my-popcode-repo';
122103
testSaga(exportRepoSaga).
123-
next().call(
124-
openWindowWithWorkaroundForChromeClosingBug,
125-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
126-
).
104+
next().call(openWindowWithContent, spinnerPageHtml).
127105
next(mockWindow).take(['REPO_EXPORTED', 'REPO_EXPORT_ERROR']).
128106
next(repoExported(url)).put(repoExportNotDisplayed(url)).
129107
next().isDone();
@@ -136,10 +114,7 @@ test('exportRepo', (t) => {
136114
t.test('with repo export error', (assert) => {
137115
const mockWindow = {closed: false, close() { }};
138116
testSaga(exportRepoSaga).
139-
next().call(
140-
openWindowWithWorkaroundForChromeClosingBug,
141-
`data:text/html;charset=utf-8;base64,${spinnerPage}`,
142-
).
117+
next().call(openWindowWithContent, spinnerPageHtml).
143118
next(mockWindow).take(['REPO_EXPORTED', 'REPO_EXPORT_ERROR']).
144119
next(repoExportError(new Error())).call([mockWindow, 'close']).
145120
next().isDone();

0 commit comments

Comments
 (0)