Refactor iframe load callback to fix race condition (#1536)

* Refactor iframe load callback to fix race condition

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

src/components/editor-page/renderer-pane/hooks/use-force-render-page-url-on-iframe-load-callback.ts

@@ -0,0 +1,51 @@
+/*
+ * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { RefObject, useCallback, useEffect, useMemo, useRef } from 'react'
+import { Logger } from '../../../../utils/logger'
+
+const log = new Logger('IframeLoader')
+
+/**
+ * Generates a callback for a iframe load handler, that enforces a given URL if frame navigates away.
+ *
+ * @param iFrameReference A reference to the iframe react dom element.
+ * @param rendererOrigin The base url that should be enforced.
+ * @param onNavigateAway An optional callback that is executed when the iframe leaves the enforced URL.
+ */
+export const useForceRenderPageUrlOnIframeLoadCallback = (
+  iFrameReference: RefObject<HTMLIFrameElement>,
+  rendererOrigin: string,
+  onNavigateAway?: () => void
+): (() => void) => {
+  const forcedUrl = useMemo(() => `${rendererOrigin}render`, [rendererOrigin])
+  const redirectionInProgress = useRef<boolean>(false)
+
+  const loadCallback = useCallback(() => {
+    const frame = iFrameReference.current
+
+    if (!frame) {
+      log.debug('No frame in reference')
+      return
+    }
+
+    if (redirectionInProgress.current) {
+      redirectionInProgress.current = false
+      log.debug('Redirect complete')
+    } else {
+      log.warn(`Navigated away from unknown URL. Forcing back to ${forcedUrl}`)
+      onNavigateAway?.()
+      redirectionInProgress.current = true
+      frame.src = forcedUrl
+    }
+  }, [iFrameReference, onNavigateAway, forcedUrl])
+
+  useEffect(() => {
+    loadCallback()
+  }, [loadCallback])
+
+  return loadCallback
+}

src/components/editor-page/renderer-pane/hooks/use-on-iframe-load.ts

@@ -1,40 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-import { RefObject, useCallback, useRef } from 'react'
-import { EditorToRendererCommunicator } from '../../../render-page/window-post-message-communicator/editor-to-renderer-communicator'
-import { Logger } from '../../../../utils/logger'
-
-const log = new Logger('IframeLoader')
-
-export const useOnIframeLoad = (
-  frameReference: RefObject<HTMLIFrameElement>,
-  iframeCommunicator: EditorToRendererCommunicator,
-  rendererOrigin: string,
-  renderPageUrl: string,
-  onNavigateAway: () => void
-): (() => void) => {
-  const sendToRenderPage = useRef<boolean>(true)
-
-  return useCallback(() => {
-    const frame = frameReference.current
-    if (!frame || !frame.contentWindow) {
-      iframeCommunicator.unsetMessageTarget()
-      return
-    }
-
-    if (sendToRenderPage.current) {
-      iframeCommunicator.setMessageTarget(frame.contentWindow, rendererOrigin)
-      sendToRenderPage.current = false
-      return
-    } else {
-      onNavigateAway()
-      log.error('Navigated away from unknown URL')
-      frame.src = renderPageUrl
-      sendToRenderPage.current = true
-    }
-  }, [frameReference, iframeCommunicator, onNavigateAway, renderPageUrl, rendererOrigin])
-}

src/components/editor-page/renderer-pane/render-iframe.tsx

@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 import React, { Fragment, useCallback, useEffect, useRef, useState } from 'react'
-import { useApplicationState } from '../../../hooks/common/use-application-state'
 import { isTestMode } from '../../../utils/test-modes'
 import { RendererProps } from '../../render-page/markdown-document'
 import {
@@ -16,7 +15,7 @@ import {
   SetScrollStateMessage
 } from '../../render-page/window-post-message-communicator/rendering-message'
 import { useEditorToRendererCommunicator } from '../render-context/editor-to-renderer-communicator-context-provider'
-import { useOnIframeLoad } from './hooks/use-on-iframe-load'
+import { useForceRenderPageUrlOnIframeLoadCallback } from './hooks/use-force-render-page-url-on-iframe-load-callback'
 import { CommunicatorImageLightbox } from './communicator-image-lightbox'
 import { setRendererStatus } from '../../../redux/renderer-status/methods'
 import { useEditorReceiveHandler } from '../../render-page/window-post-message-communicator/hooks/use-editor-receive-handler'
@@ -24,6 +23,8 @@ import { useIsRendererReady } from '../../render-page/window-post-message-commun
 import { useSendDarkModeStatusToRenderer } from './hooks/use-send-dark-mode-status-to-renderer'
 import { useSendMarkdownToRenderer } from './hooks/use-send-markdown-to-renderer'
 import { useSendScrollState } from './hooks/use-send-scroll-state'
+import { useApplicationState } from '../../../hooks/common/use-application-state'
+import { Logger } from '../../../utils/logger'
 
 export interface RenderIframeProps extends RendererProps {
   rendererType: RendererType
@@ -31,6 +32,8 @@ export interface RenderIframeProps extends RendererProps {
   frameClasses?: string
 }
 
+const log = new Logger('RenderIframe')
+
 export const RenderIframe: React.FC<RenderIframeProps> = ({
   markdownContent,
   onTaskCheckedChange,
@@ -44,17 +47,14 @@ export const RenderIframe: React.FC<RenderIframeProps> = ({
 }) => {
   const frameReference = useRef<HTMLIFrameElement>(null)
   const rendererOrigin = useApplicationState((state) => state.config.iframeCommunication.rendererOrigin)
-  const renderPageUrl = `${rendererOrigin}render`
-  const resetRendererReady = useCallback(() => setRendererStatus(false), [])
   const iframeCommunicator = useEditorToRendererCommunicator()
+  const resetRendererReady = useCallback(() => {
+    log.debug('Reset render status')
+    iframeCommunicator.unsetMessageTarget()
+    setRendererStatus(false)
+  }, [iframeCommunicator])
   const rendererReady = useIsRendererReady()
-  const onIframeLoad = useOnIframeLoad(
-    frameReference,
-    iframeCommunicator,
-    rendererOrigin,
-    renderPageUrl,
-    resetRendererReady
-  )
+  const onIframeLoad = useForceRenderPageUrlOnIframeLoadCallback(frameReference, rendererOrigin, resetRendererReady)
   const [frameHeight, setFrameHeight] = useState<number>(0)
 
   useEffect(
@@ -99,6 +99,18 @@ export const RenderIframe: React.FC<RenderIframeProps> = ({
   useEditorReceiveHandler(
     CommunicationMessageType.RENDERER_READY,
     useCallback(() => {
+      const frame = frameReference.current
+      if (!frame) {
+        log.error('Load triggered without frame in ref')
+        return
+      }
+      const otherWindow = frame.contentWindow
+      if (!otherWindow) {
+        log.error('Load triggered without content window')
+        return
+      }
+      log.debug(`Set iframecommunicator window with origin ${rendererOrigin}`)
+      iframeCommunicator.setMessageTarget(otherWindow, rendererOrigin)
       iframeCommunicator.enableCommunication()
       iframeCommunicator.sendMessageToOtherSide({
         type: CommunicationMessageType.SET_BASE_CONFIGURATION,
@@ -108,7 +120,7 @@ export const RenderIframe: React.FC<RenderIframeProps> = ({
         }
       })
       setRendererStatus(true)
-    }, [iframeCommunicator, rendererType])
+    }, [iframeCommunicator, rendererOrigin, rendererType])
   )
 
   useSendScrollState(scrollState)
@@ -123,7 +135,6 @@ export const RenderIframe: React.FC<RenderIframeProps> = ({
         data-cy={'documentIframe'}
         onLoad={onIframeLoad}
         title='render'
-        src={renderPageUrl}
         {...(isTestMode() ? {} : { sandbox: 'allow-downloads allow-same-origin allow-scripts allow-popups' })}
         ref={frameReference}
         className={`border-0 ${frameClasses ?? ''}`}