Skip to content

Commit

Permalink
webview js: Fix type errors in handling null; mark strict-local.
Browse files Browse the repository at this point in the history
We weren't actually checking that the messages we send to React from
inside the webview were of the types we were assuming they were in the
receiving code -- we had an `Object` type disabling the type-checker.

And in fact, this was hiding a series of type errors!  The inner code
pulls various attributes' values off of the elements it looks at and
sends them to the outer code, which blithely goes and assumes the
results are strings -- but if the HTML is for some reason not as we
expect it to be, they could be `null` or `undefined`.  As a result,
we'd end up going wrong in unpredictable ways, in code deep inside the
app's logic.

Instead, check our assumptions right at the edge, and if they fail
then raise an exception so we drop the event and show an error banner.

This is basically another instance of the "crunchy shell" pattern.
  • Loading branch information
gnprice committed Mar 26, 2019
1 parent ad35078 commit dd03939
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 18 deletions.
27 changes: 19 additions & 8 deletions src/webview/js/generatedEs3.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,17 @@ document.addEventListener('message', function (e) {
});
scrollEventsDisabled = false;
});
var requireAttribute = function requireAttribute(e, name) {
var value = e.getAttribute(name);
if (value === null || value === undefined) {
throw new Error("Missing expected attribute " + name);
}
return value;
};
documentBody.addEventListener('click', function (e) {
e.preventDefault();
lastTouchEventTimestamp = 0;
Expand All @@ -421,15 +432,15 @@ documentBody.addEventListener('click', function (e) {
if (target.matches('.avatar-img')) {
sendMessage({
type: 'avatar',
fromEmail: target.getAttribute('data-email')
fromEmail: requireAttribute(target, 'data-emailer')
});
return;
}
if (target.matches('.header')) {
sendMessage({
type: 'narrow',
narrow: target.getAttribute('data-narrow')
narrow: requireAttribute(target, 'data-narrow')
});
return;
}
Expand All @@ -439,7 +450,7 @@ documentBody.addEventListener('click', function (e) {
if (inlineImageLink && !inlineImageLink.closest('.youtube-video, .vimeo-video')) {
sendMessage({
type: 'image',
src: inlineImageLink.getAttribute('href'),
src: requireAttribute(inlineImageLink, 'href'),
messageId: getMessageIdFromNode(inlineImageLink)
});
return;
Expand All @@ -448,7 +459,7 @@ documentBody.addEventListener('click', function (e) {
if (target.matches('a')) {
sendMessage({
type: 'url',
href: target.getAttribute('href'),
href: requireAttribute(target, 'href'),
messageId: getMessageIdFromNode(target)
});
return;
Expand All @@ -457,7 +468,7 @@ documentBody.addEventListener('click', function (e) {
if (target.parentNode instanceof Element && target.parentNode.matches('a')) {
sendMessage({
type: 'url',
href: target.parentNode.getAttribute('href'),
href: requireAttribute(target.parentNode, 'href'),
messageId: getMessageIdFromNode(target.parentNode)
});
return;
Expand All @@ -466,9 +477,9 @@ documentBody.addEventListener('click', function (e) {
if (target.matches('.reaction')) {
sendMessage({
type: 'reaction',
name: target.getAttribute('data-name'),
code: target.getAttribute('data-code'),
reactionType: target.getAttribute('data-type'),
name: requireAttribute(target, 'data-name'),
code: requireAttribute(target, 'data-code'),
reactionType: requireAttribute(target, 'data-type'),
messageId: getMessageIdFromNode(target),
voted: target.classList.contains('self-voted')
});
Expand Down
29 changes: 19 additions & 10 deletions src/webview/js/js.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* @flow */
/* @flow strict-local */
import type { Auth } from '../../types';
import type {
WebviewInputMessage,
Expand All @@ -7,6 +7,7 @@ import type {
MessageInputTyping,
MessageInputReady,
} from '../webViewHandleUpdates';
import type { MessageListEvent } from '../webViewEventHandlers';

/*
* Supported platforms:
Expand Down Expand Up @@ -74,7 +75,7 @@ const escapeHtml = (text: string): string => {
return element.innerHTML;
};

const sendMessage = (msg: Object) => {
const sendMessage = (msg: MessageListEvent) => {
window.postMessage(JSON.stringify(msg), '*');
};

Expand Down Expand Up @@ -545,6 +546,14 @@ document.addEventListener('message', e => {
*
*/

const requireAttribute = (e: Element, name: string): string => {
const value = e.getAttribute(name);
if (value === null || value === undefined) {
throw new Error(`Missing expected attribute ${name}`);
}
return value;
};

documentBody.addEventListener('click', (e: MouseEvent) => {
e.preventDefault();
lastTouchEventTimestamp = 0;
Expand All @@ -563,15 +572,15 @@ documentBody.addEventListener('click', (e: MouseEvent) => {
if (target.matches('.avatar-img')) {
sendMessage({
type: 'avatar',
fromEmail: target.getAttribute('data-email'),
fromEmail: requireAttribute(target, 'data-emailer'),
});
return;
}

if (target.matches('.header')) {
sendMessage({
type: 'narrow',
narrow: target.getAttribute('data-narrow'),
narrow: requireAttribute(target, 'data-narrow'),
});
return;
}
Expand All @@ -587,7 +596,7 @@ documentBody.addEventListener('click', (e: MouseEvent) => {
) {
sendMessage({
type: 'image',
src: inlineImageLink.getAttribute('href'), // TODO: should be `src` / `data-src-fullsize`.
src: requireAttribute(inlineImageLink, 'href'), // TODO: should be `src` / `data-src-fullsize`.
messageId: getMessageIdFromNode(inlineImageLink),
});
return;
Expand All @@ -596,7 +605,7 @@ documentBody.addEventListener('click', (e: MouseEvent) => {
if (target.matches('a')) {
sendMessage({
type: 'url',
href: target.getAttribute('href'),
href: requireAttribute(target, 'href'),
messageId: getMessageIdFromNode(target),
});
return;
Expand All @@ -605,7 +614,7 @@ documentBody.addEventListener('click', (e: MouseEvent) => {
if (target.parentNode instanceof Element && target.parentNode.matches('a')) {
sendMessage({
type: 'url',
href: target.parentNode.getAttribute('href'),
href: requireAttribute(target.parentNode, 'href'),
messageId: getMessageIdFromNode(target.parentNode),
});
return;
Expand All @@ -614,9 +623,9 @@ documentBody.addEventListener('click', (e: MouseEvent) => {
if (target.matches('.reaction')) {
sendMessage({
type: 'reaction',
name: target.getAttribute('data-name'),
code: target.getAttribute('data-code'),
reactionType: target.getAttribute('data-type'),
name: requireAttribute(target, 'data-name'),
code: requireAttribute(target, 'data-code'),
reactionType: requireAttribute(target, 'data-type'),
messageId: getMessageIdFromNode(target),
voted: target.classList.contains('self-voted'),
});
Expand Down

0 comments on commit dd03939

Please sign in to comment.