cef: harden popup bridge follow-ups
- commit
- 5ef1bedff8da9316961cb57cc7d20a1cc85150a6
- author
- brice · · 2026-04-23 13:21 UTC
- parent
- 314ad3d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
12 files · +458 -21
M internal/infrastructure/cef/bridge_script_templates.go +17 -1
| @@ -54,6 +54,22 @@ const popupProxyBridgeBodyTemplateJS = ` | |||
| 54 | 54 | return target === '_self' || target === '_top' || target === '_parent'; | |
| 55 | 55 | } | |
| 56 | 56 | ||
| 57 | + | function createPopupProxyID() { | |
| 58 | + | var timestamp = Date.now(); | |
| 59 | + | try { | |
| 60 | + | if (typeof crypto !== 'undefined' && crypto && typeof crypto.getRandomValues === 'function') { | |
| 61 | + | var bytes = new Uint8Array(4); | |
| 62 | + | crypto.getRandomValues(bytes); | |
| 63 | + | var nonce = ''; | |
| 64 | + | for (var i = 0; i < bytes.length; i++) { | |
| 65 | + | nonce += ('0' + bytes[i].toString(16)).slice(-2); | |
| 66 | + | } | |
| 67 | + | return 'popup-' + timestamp + '-' + nonce; | |
| 68 | + | } | |
| 69 | + | } catch (_) {} | |
| 70 | + | return 'popup-' + timestamp + '-' + Math.random().toString(36).slice(2); | |
| 71 | + | } | |
| 72 | + | ||
| 57 | 73 | function createSyntheticPopupProxy(proxyID, initialURL, features) { | |
| 58 | 74 | var href = initialURL || 'about:blank'; | |
| 59 | 75 | var closed = false; | |
| @@ -138,7 +154,7 @@ const popupProxyBridgeBodyTemplateJS = ` | |||
| 138 | 154 | return originalWindowOpen.apply(this, arguments); | |
| 139 | 155 | } | |
| 140 | 156 | ||
| 141 | - | var proxyID = 'popup-' + Date.now() + '-' + Math.random().toString(36).slice(2); | |
| 157 | + | var proxyID = createPopupProxyID(); | |
| 142 | 158 | var resolvedURL = resolvePopupURL(url); | |
| 143 | 159 | var noJavaScriptAccess = popupHasNoOpener(features); | |
| 144 | 160 | var popupProxy = createSyntheticPopupProxy(proxyID, resolvedURL, features); | |
| @@ -54,6 +54,22 @@ const popupProxyBridgeBodyTemplateJS = ` | |||||
| 54 | return target === '_self' || target === '_top' || target === '_parent'; | 54 | return target === '_self' || target === '_top' || target === '_parent'; | ||
| 55 | } | 55 | } | ||
| 56 | 56 | ||||
| 57 | + | function createPopupProxyID() { | |||
| 58 | + | var timestamp = Date.now(); | |||
| 59 | + | try { | |||
| 60 | + | if (typeof crypto !== 'undefined' && crypto && typeof crypto.getRandomValues === 'function') { | |||
| 61 | + | var bytes = new Uint8Array(4); | |||
| 62 | + | crypto.getRandomValues(bytes); | |||
| 63 | + | var nonce = ''; | |||
| 64 | + | for (var i = 0; i < bytes.length; i++) { | |||
| 65 | + | nonce += ('0' + bytes[i].toString(16)).slice(-2); | |||
| 66 | + | } | |||
| 67 | + | return 'popup-' + timestamp + '-' + nonce; | |||
| 68 | + | } | |||
| 69 | + | } catch (_) {} | |||
| 70 | + | return 'popup-' + timestamp + '-' + Math.random().toString(36).slice(2); | |||
| 71 | + | } | |||
| 72 | + | ||||
| 57 | function createSyntheticPopupProxy(proxyID, initialURL, features) { | 73 | function createSyntheticPopupProxy(proxyID, initialURL, features) { | ||
| 58 | var href = initialURL || 'about:blank'; | 74 | var href = initialURL || 'about:blank'; | ||
| 59 | var closed = false; | 75 | var closed = false; | ||
| @@ -138,7 +154,7 @@ const popupProxyBridgeBodyTemplateJS = ` | |||||
| 138 | return originalWindowOpen.apply(this, arguments); | 154 | return originalWindowOpen.apply(this, arguments); | ||
| 139 | } | 155 | } | ||
| 140 | 156 | ||||
| 141 | - | var proxyID = 'popup-' + Date.now() + '-' + Math.random().toString(36).slice(2); | 157 | + | var proxyID = createPopupProxyID(); |
| 142 | var resolvedURL = resolvePopupURL(url); | 158 | var resolvedURL = resolvePopupURL(url); | ||
| 143 | var noJavaScriptAccess = popupHasNoOpener(features); | 159 | var noJavaScriptAccess = popupHasNoOpener(features); | ||
| 144 | var popupProxy = createSyntheticPopupProxy(proxyID, resolvedURL, features); | 160 | var popupProxy = createSyntheticPopupProxy(proxyID, resolvedURL, features); | ||
M internal/infrastructure/cef/content_injector_test.go +2 -0
| @@ -73,6 +73,8 @@ func TestTrustedPageFetchBridgeJS_ShimsWindowOpenToBridgePopupRequests(t *testin | |||
| 73 | 73 | require.Contains(t, script, "dumb:///api/popup-close") | |
| 74 | 74 | require.Contains(t, script, "proxy_id") | |
| 75 | 75 | require.Contains(t, script, "createSyntheticPopupProxy") | |
| 76 | + | require.Contains(t, script, "createPopupProxyID") | |
| 77 | + | require.Contains(t, script, "crypto.getRandomValues") | |
| 76 | 78 | require.Contains(t, script, "no_javascript_access") | |
| 77 | 79 | require.Contains(t, script, "Object.defineProperty(proxy, 'closed'") | |
| 78 | 80 | require.Contains(t, script, "test-bridge-nonce") | |
| @@ -73,6 +73,8 @@ func TestTrustedPageFetchBridgeJS_ShimsWindowOpenToBridgePopupRequests(t *testin | |||||
| 73 | require.Contains(t, script, "dumb:///api/popup-close") | 73 | require.Contains(t, script, "dumb:///api/popup-close") | ||
| 74 | require.Contains(t, script, "proxy_id") | 74 | require.Contains(t, script, "proxy_id") | ||
| 75 | require.Contains(t, script, "createSyntheticPopupProxy") | 75 | require.Contains(t, script, "createSyntheticPopupProxy") | ||
| 76 | + | require.Contains(t, script, "createPopupProxyID") | |||
| 77 | + | require.Contains(t, script, "crypto.getRandomValues") | |||
| 76 | require.Contains(t, script, "no_javascript_access") | 78 | require.Contains(t, script, "no_javascript_access") | ||
| 77 | require.Contains(t, script, "Object.defineProperty(proxy, 'closed'") | 79 | require.Contains(t, script, "Object.defineProperty(proxy, 'closed'") | ||
| 78 | require.Contains(t, script, "test-bridge-nonce") | 80 | require.Contains(t, script, "test-bridge-nonce") | ||
M internal/infrastructure/cef/factory.go +28 -2
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "sync/atomic" | |
| 7 | + | "time" | |
| 7 | 8 | ||
| 8 | 9 | purecef "github.com/bnema/purego-cef/cef" | |
| 9 | 10 | ||
| @@ -44,6 +45,11 @@ type resizeNotifiableBrowserHost interface { | |||
| 44 | 45 | ||
| 45 | 46 | var cefBrowserHostCreateBrowser = purecef.BrowserHostCreateBrowser | |
| 46 | 47 | ||
| 48 | + | const ( | |
| 49 | + | pendingBrowserCreateRetryDelay = 10 * time.Millisecond | |
| 50 | + | pendingBrowserCreateMaxPostRetries = 3 | |
| 51 | + | ) | |
| 52 | + | ||
| 47 | 53 | // newWebViewFactory returns a factory that will create WebViews using the | |
| 48 | 54 | // given GL loader and HiDPI scale factor. | |
| 49 | 55 | func newWebViewFactory(engine *Engine, gl *glLoader, opts webViewFactoryOptions) *WebViewFactory { | |
| @@ -231,7 +237,7 @@ func (f *WebViewFactory) postPendingBrowserCreate(ctx context.Context, wv *WebVi | |||
| 231 | 237 | return | |
| 232 | 238 | } | |
| 233 | 239 | ||
| 234 | - | task := purecef.NewTask(cefTaskFunc(func() { | |
| 240 | + | task := cefNewTask(cefTaskFunc(func() { | |
| 235 | 241 | if wv.destroyed.Load() { | |
| 236 | 242 | log.Debug().Msg("cef: skipping browser creation for destroyed webview") | |
| 237 | 243 | return | |
| @@ -267,17 +273,37 @@ func (f *WebViewFactory) postPendingBrowserCreate(ctx context.Context, wv *WebVi | |||
| 267 | 273 | Msg("cef: BrowserHostCreateBrowser call completed on CEF UI thread") | |
| 268 | 274 | })) | |
| 269 | 275 | ||
| 270 | - | postResult := purecef.PostTask(purecef.ThreadIDTidUi, task) | |
| 276 | + | postResult := cefPostTask(purecef.ThreadIDTidUi, task) | |
| 271 | 277 | if postResult != 1 { | |
| 278 | + | pc.postTaskRetries++ | |
| 272 | 279 | wv.mu.Lock() | |
| 273 | 280 | wv.pendingCreate = pc | |
| 274 | 281 | wv.mu.Unlock() | |
| 275 | 282 | log.Error(). | |
| 276 | 283 | Int32("result", postResult). | |
| 284 | + | Int("retry", pc.postTaskRetries). | |
| 277 | 285 | Msg("cef: failed to post initial browser creation to CEF UI thread") | |
| 286 | + | f.schedulePendingBrowserCreateRetry(ctx, wv, w, h, pc) | |
| 278 | 287 | } | |
| 279 | 288 | } | |
| 280 | 289 | ||
| 290 | + | func (f *WebViewFactory) schedulePendingBrowserCreateRetry( | |
| 291 | + | ctx context.Context, | |
| 292 | + | wv *WebView, | |
| 293 | + | w, h int32, | |
| 294 | + | pc *pendingBrowserCreate, | |
| 295 | + | ) { | |
| 296 | + | if f == nil || wv == nil || pc == nil || wv.destroyed.Load() || pc.postTaskRetries > pendingBrowserCreateMaxPostRetries { | |
| 297 | + | return | |
| 298 | + | } | |
| 299 | + | cefScheduleAfter(pendingBrowserCreateRetryDelay, func() { | |
| 300 | + | if wv.destroyed.Load() { | |
| 301 | + | return | |
| 302 | + | } | |
| 303 | + | f.postPendingBrowserCreate(ctx, wv, w, h) | |
| 304 | + | }) | |
| 305 | + | } | |
| 306 | + | ||
| 281 | 307 | func notifyBrowserResize(host resizeNotifiableBrowserHost) { | |
| 282 | 308 | if host == nil { | |
| 283 | 309 | return | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "sync/atomic" | 6 | "sync/atomic" | ||
| 7 | + | "time" | |||
| 7 | 8 | ||||
| 8 | purecef "github.com/bnema/purego-cef/cef" | 9 | purecef "github.com/bnema/purego-cef/cef" | ||
| 9 | 10 | ||||
| @@ -44,6 +45,11 @@ type resizeNotifiableBrowserHost interface { | |||||
| 44 | 45 | ||||
| 45 | var cefBrowserHostCreateBrowser = purecef.BrowserHostCreateBrowser | 46 | var cefBrowserHostCreateBrowser = purecef.BrowserHostCreateBrowser | ||
| 46 | 47 | ||||
| 48 | + | const ( | |||
| 49 | + | pendingBrowserCreateRetryDelay = 10 * time.Millisecond | |||
| 50 | + | pendingBrowserCreateMaxPostRetries = 3 | |||
| 51 | + | ) | |||
| 52 | + | ||||
| 47 | // newWebViewFactory returns a factory that will create WebViews using the | 53 | // newWebViewFactory returns a factory that will create WebViews using the | ||
| 48 | // given GL loader and HiDPI scale factor. | 54 | // given GL loader and HiDPI scale factor. | ||
| 49 | func newWebViewFactory(engine *Engine, gl *glLoader, opts webViewFactoryOptions) *WebViewFactory { | 55 | func newWebViewFactory(engine *Engine, gl *glLoader, opts webViewFactoryOptions) *WebViewFactory { | ||
| @@ -231,7 +237,7 @@ func (f *WebViewFactory) postPendingBrowserCreate(ctx context.Context, wv *WebVi | |||||
| 231 | return | 237 | return | ||
| 232 | } | 238 | } | ||
| 233 | 239 | ||||
| 234 | - | task := purecef.NewTask(cefTaskFunc(func() { | 240 | + | task := cefNewTask(cefTaskFunc(func() { |
| 235 | if wv.destroyed.Load() { | 241 | if wv.destroyed.Load() { | ||
| 236 | log.Debug().Msg("cef: skipping browser creation for destroyed webview") | 242 | log.Debug().Msg("cef: skipping browser creation for destroyed webview") | ||
| 237 | return | 243 | return | ||
| @@ -267,17 +273,37 @@ func (f *WebViewFactory) postPendingBrowserCreate(ctx context.Context, wv *WebVi | |||||
| 267 | Msg("cef: BrowserHostCreateBrowser call completed on CEF UI thread") | 273 | Msg("cef: BrowserHostCreateBrowser call completed on CEF UI thread") | ||
| 268 | })) | 274 | })) | ||
| 269 | 275 | ||||
| 270 | - | postResult := purecef.PostTask(purecef.ThreadIDTidUi, task) | 276 | + | postResult := cefPostTask(purecef.ThreadIDTidUi, task) |
| 271 | if postResult != 1 { | 277 | if postResult != 1 { | ||
| 278 | + | pc.postTaskRetries++ | |||
| 272 | wv.mu.Lock() | 279 | wv.mu.Lock() | ||
| 273 | wv.pendingCreate = pc | 280 | wv.pendingCreate = pc | ||
| 274 | wv.mu.Unlock() | 281 | wv.mu.Unlock() | ||
| 275 | log.Error(). | 282 | log.Error(). | ||
| 276 | Int32("result", postResult). | 283 | Int32("result", postResult). | ||
| 284 | + | Int("retry", pc.postTaskRetries). | |||
| 277 | Msg("cef: failed to post initial browser creation to CEF UI thread") | 285 | Msg("cef: failed to post initial browser creation to CEF UI thread") | ||
| 286 | + | f.schedulePendingBrowserCreateRetry(ctx, wv, w, h, pc) | |||
| 278 | } | 287 | } | ||
| 279 | } | 288 | } | ||
| 280 | 289 | ||||
| 290 | + | func (f *WebViewFactory) schedulePendingBrowserCreateRetry( | |||
| 291 | + | ctx context.Context, | |||
| 292 | + | wv *WebView, | |||
| 293 | + | w, h int32, | |||
| 294 | + | pc *pendingBrowserCreate, | |||
| 295 | + | ) { | |||
| 296 | + | if f == nil || wv == nil || pc == nil || wv.destroyed.Load() || pc.postTaskRetries > pendingBrowserCreateMaxPostRetries { | |||
| 297 | + | return | |||
| 298 | + | } | |||
| 299 | + | cefScheduleAfter(pendingBrowserCreateRetryDelay, func() { | |||
| 300 | + | if wv.destroyed.Load() { | |||
| 301 | + | return | |||
| 302 | + | } | |||
| 303 | + | f.postPendingBrowserCreate(ctx, wv, w, h) | |||
| 304 | + | }) | |||
| 305 | + | } | |||
| 306 | + | ||||
| 281 | func notifyBrowserResize(host resizeNotifiableBrowserHost) { | 307 | func notifyBrowserResize(host resizeNotifiableBrowserHost) { | ||
| 282 | if host == nil { | 308 | if host == nil { | ||
| 283 | return | 309 | return | ||
M internal/infrastructure/cef/factory_resize_test.go +70 -0
| @@ -1,7 +1,9 @@ | |||
| 1 | 1 | package cef | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "testing" | |
| 6 | + | "time" | |
| 5 | 7 | ||
| 6 | 8 | purecef "github.com/bnema/purego-cef/cef" | |
| 7 | 9 | "github.com/stretchr/testify/require" | |
| @@ -26,3 +28,71 @@ func TestNotifyBrowserResize_CallsWasResizedThenInvalidate(t *testing.T) { | |||
| 26 | 28 | ||
| 27 | 29 | require.Equal(t, []string{"WasResized", "Invalidate"}, host.calls) | |
| 28 | 30 | } | |
| 31 | + | ||
| 32 | + | func TestPostPendingBrowserCreate_RetriesWhenPostingTaskFails(t *testing.T) { | |
| 33 | + | oldNewTask := cefNewTask | |
| 34 | + | oldPostTask := cefPostTask | |
| 35 | + | oldScheduleAfter := cefScheduleAfter | |
| 36 | + | oldCreateBrowser := cefBrowserHostCreateBrowser | |
| 37 | + | defer func() { | |
| 38 | + | cefNewTask = oldNewTask | |
| 39 | + | cefPostTask = oldPostTask | |
| 40 | + | cefScheduleAfter = oldScheduleAfter | |
| 41 | + | cefBrowserHostCreateBrowser = oldCreateBrowser | |
| 42 | + | }() | |
| 43 | + | ||
| 44 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 45 | + | ||
| 46 | + | postCalls := 0 | |
| 47 | + | cefPostTask = func(threadID purecef.ThreadID, task purecef.Task) int32 { | |
| 48 | + | require.Equal(t, purecef.ThreadIDTidUi, threadID) | |
| 49 | + | require.NotNil(t, task) | |
| 50 | + | postCalls++ | |
| 51 | + | if postCalls == 1 { | |
| 52 | + | return 0 | |
| 53 | + | } | |
| 54 | + | task.Execute() | |
| 55 | + | return 1 | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | retried := false | |
| 59 | + | cefScheduleAfter = func(delay time.Duration, fn func()) { | |
| 60 | + | require.Equal(t, pendingBrowserCreateRetryDelay, delay) | |
| 61 | + | retried = true | |
| 62 | + | fn() | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | createCalls := 0 | |
| 66 | + | cefBrowserHostCreateBrowser = func( | |
| 67 | + | windowInfo *purecef.WindowInfo, | |
| 68 | + | _ purecef.RawClient, | |
| 69 | + | url string, | |
| 70 | + | settings *purecef.BrowserSettings, | |
| 71 | + | _ purecef.DictionaryValue, | |
| 72 | + | _ purecef.RequestContext, | |
| 73 | + | ) int32 { | |
| 74 | + | createCalls++ | |
| 75 | + | require.NotNil(t, windowInfo) | |
| 76 | + | require.NotNil(t, settings) | |
| 77 | + | require.Equal(t, "about:blank", url) | |
| 78 | + | return 1 | |
| 79 | + | } | |
| 80 | + | ||
| 81 | + | windowInfo := purecef.NewWindowInfo() | |
| 82 | + | settings := purecef.NewBrowserSettings() | |
| 83 | + | wv := &WebView{ | |
| 84 | + | ctx: context.Background(), | |
| 85 | + | pendingCreate: &pendingBrowserCreate{ | |
| 86 | + | windowInfo: &windowInfo, | |
| 87 | + | settings: &settings, | |
| 88 | + | }, | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | factory := &WebViewFactory{} | |
| 92 | + | factory.postPendingBrowserCreate(context.Background(), wv, 640, 480) | |
| 93 | + | ||
| 94 | + | require.True(t, retried) | |
| 95 | + | require.Equal(t, 2, postCalls) | |
| 96 | + | require.Equal(t, 1, createCalls) | |
| 97 | + | require.Nil(t, wv.takePendingCreate()) | |
| 98 | + | } | |
| @@ -1,7 +1,9 @@ | |||||
| 1 | package cef | 1 | package cef | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "testing" | 5 | "testing" | ||
| 6 | + | "time" | |||
| 5 | 7 | ||||
| 6 | purecef "github.com/bnema/purego-cef/cef" | 8 | purecef "github.com/bnema/purego-cef/cef" | ||
| 7 | "github.com/stretchr/testify/require" | 9 | "github.com/stretchr/testify/require" | ||
| @@ -26,3 +28,71 @@ func TestNotifyBrowserResize_CallsWasResizedThenInvalidate(t *testing.T) { | |||||
| 26 | 28 | ||||
| 27 | require.Equal(t, []string{"WasResized", "Invalidate"}, host.calls) | 29 | require.Equal(t, []string{"WasResized", "Invalidate"}, host.calls) | ||
| 28 | } | 30 | } | ||
| 31 | + | ||||
| 32 | + | func TestPostPendingBrowserCreate_RetriesWhenPostingTaskFails(t *testing.T) { | |||
| 33 | + | oldNewTask := cefNewTask | |||
| 34 | + | oldPostTask := cefPostTask | |||
| 35 | + | oldScheduleAfter := cefScheduleAfter | |||
| 36 | + | oldCreateBrowser := cefBrowserHostCreateBrowser | |||
| 37 | + | defer func() { | |||
| 38 | + | cefNewTask = oldNewTask | |||
| 39 | + | cefPostTask = oldPostTask | |||
| 40 | + | cefScheduleAfter = oldScheduleAfter | |||
| 41 | + | cefBrowserHostCreateBrowser = oldCreateBrowser | |||
| 42 | + | }() | |||
| 43 | + | ||||
| 44 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 45 | + | ||||
| 46 | + | postCalls := 0 | |||
| 47 | + | cefPostTask = func(threadID purecef.ThreadID, task purecef.Task) int32 { | |||
| 48 | + | require.Equal(t, purecef.ThreadIDTidUi, threadID) | |||
| 49 | + | require.NotNil(t, task) | |||
| 50 | + | postCalls++ | |||
| 51 | + | if postCalls == 1 { | |||
| 52 | + | return 0 | |||
| 53 | + | } | |||
| 54 | + | task.Execute() | |||
| 55 | + | return 1 | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | retried := false | |||
| 59 | + | cefScheduleAfter = func(delay time.Duration, fn func()) { | |||
| 60 | + | require.Equal(t, pendingBrowserCreateRetryDelay, delay) | |||
| 61 | + | retried = true | |||
| 62 | + | fn() | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | createCalls := 0 | |||
| 66 | + | cefBrowserHostCreateBrowser = func( | |||
| 67 | + | windowInfo *purecef.WindowInfo, | |||
| 68 | + | _ purecef.RawClient, | |||
| 69 | + | url string, | |||
| 70 | + | settings *purecef.BrowserSettings, | |||
| 71 | + | _ purecef.DictionaryValue, | |||
| 72 | + | _ purecef.RequestContext, | |||
| 73 | + | ) int32 { | |||
| 74 | + | createCalls++ | |||
| 75 | + | require.NotNil(t, windowInfo) | |||
| 76 | + | require.NotNil(t, settings) | |||
| 77 | + | require.Equal(t, "about:blank", url) | |||
| 78 | + | return 1 | |||
| 79 | + | } | |||
| 80 | + | ||||
| 81 | + | windowInfo := purecef.NewWindowInfo() | |||
| 82 | + | settings := purecef.NewBrowserSettings() | |||
| 83 | + | wv := &WebView{ | |||
| 84 | + | ctx: context.Background(), | |||
| 85 | + | pendingCreate: &pendingBrowserCreate{ | |||
| 86 | + | windowInfo: &windowInfo, | |||
| 87 | + | settings: &settings, | |||
| 88 | + | }, | |||
| 89 | + | } | |||
| 90 | + | ||||
| 91 | + | factory := &WebViewFactory{} | |||
| 92 | + | factory.postPendingBrowserCreate(context.Background(), wv, 640, 480) | |||
| 93 | + | ||||
| 94 | + | require.True(t, retried) | |||
| 95 | + | require.Equal(t, 2, postCalls) | |||
| 96 | + | require.Equal(t, 1, createCalls) | |||
| 97 | + | require.Nil(t, wv.takePendingCreate()) | |||
| 98 | + | } | |||
M internal/infrastructure/cef/popup_opener_bridge.go +14 -8
| @@ -338,16 +338,16 @@ func (wv *WebView) handlePopupOpenerPostMessage(payload popupOpenerPostMessagePa | |||
| 338 | 338 | } | |
| 339 | 339 | return | |
| 340 | 340 | } | |
| 341 | + | sourceOrigin, sourceHref := popupOpenerSourceMetadata(wv) | |
| 341 | 342 | if wv.ctx != nil { | |
| 342 | 343 | logging.FromContext(wv.ctx).Debug(). | |
| 343 | 344 | Uint64("webview_id", uint64(wv.id)). | |
| 344 | 345 | Str("target_origin", payload.TargetOrigin). | |
| 345 | - | Str("source_origin", payload.SourceOrigin). | |
| 346 | - | Str("source_href", logging.TruncateURL(payload.SourceHref, logging.PermissionLogURLMaxLen)). | |
| 346 | + | Str("source_origin", sourceOrigin). | |
| 347 | + | Str("source_href", logging.TruncateURL(sourceHref, logging.PermissionLogURLMaxLen)). | |
| 347 | 348 | Msg("cef: popup opener postMessage received") | |
| 348 | 349 | } | |
| 349 | 350 | ||
| 350 | - | sourceOrigin := popupSourceOrigin(payload) | |
| 351 | 351 | var dataExpr string | |
| 352 | 352 | switch strings.ToLower(strings.TrimSpace(payload.DataKind)) { | |
| 353 | 353 | case "json": | |
| @@ -358,7 +358,6 @@ func (wv *WebView) handlePopupOpenerPostMessage(payload popupOpenerPostMessagePa | |||
| 358 | 358 | dataExpr = fmt.Sprintf("'%s'", webutil.EscapeForJSString(payload.Data)) | |
| 359 | 359 | } | |
| 360 | 360 | ||
| 361 | - | sourceHref := strings.TrimSpace(payload.SourceHref) | |
| 362 | 361 | script := fmt.Sprintf(`(function() { | |
| 363 | 362 | try { | |
| 364 | 363 | var data; | |
| @@ -405,11 +404,18 @@ func targetOriginMatchesPopupOpener(targetOrigin, openerURI string) bool { | |||
| 405 | 404 | return targetCanonical != "" && targetCanonical == canonicalOrigin(openerURI) | |
| 406 | 405 | } | |
| 407 | 406 | ||
| 408 | - | func popupSourceOrigin(payload popupOpenerPostMessagePayload) string { | |
| 409 | - | if origin := strings.TrimSpace(payload.SourceOrigin); origin != "" { | |
| 410 | - | return origin | |
| 407 | + | func popupOpenerSourceMetadata(wv *WebView) (origin, href string) { | |
| 408 | + | if wv == nil { | |
| 409 | + | return "", "" | |
| 410 | + | } | |
| 411 | + | href = strings.TrimSpace(wv.URI()) | |
| 412 | + | if href == "" { | |
| 413 | + | href = strings.TrimSpace(wv.pendingNavigationURI()) | |
| 414 | + | } | |
| 415 | + | if href == "" { | |
| 416 | + | return "", "" | |
| 411 | 417 | } | |
| 412 | - | return originFromURL(payload.SourceHref) | |
| 418 | + | return originFromURL(href), href | |
| 413 | 419 | } | |
| 414 | 420 | ||
| 415 | 421 | func originFromURL(rawURL string) string { | |
| @@ -338,16 +338,16 @@ func (wv *WebView) handlePopupOpenerPostMessage(payload popupOpenerPostMessagePa | |||||
| 338 | } | 338 | } | ||
| 339 | return | 339 | return | ||
| 340 | } | 340 | } | ||
| 341 | + | sourceOrigin, sourceHref := popupOpenerSourceMetadata(wv) | |||
| 341 | if wv.ctx != nil { | 342 | if wv.ctx != nil { | ||
| 342 | logging.FromContext(wv.ctx).Debug(). | 343 | logging.FromContext(wv.ctx).Debug(). | ||
| 343 | Uint64("webview_id", uint64(wv.id)). | 344 | Uint64("webview_id", uint64(wv.id)). | ||
| 344 | Str("target_origin", payload.TargetOrigin). | 345 | Str("target_origin", payload.TargetOrigin). | ||
| 345 | - | Str("source_origin", payload.SourceOrigin). | 346 | + | Str("source_origin", sourceOrigin). |
| 346 | - | Str("source_href", logging.TruncateURL(payload.SourceHref, logging.PermissionLogURLMaxLen)). | 347 | + | Str("source_href", logging.TruncateURL(sourceHref, logging.PermissionLogURLMaxLen)). |
| 347 | Msg("cef: popup opener postMessage received") | 348 | Msg("cef: popup opener postMessage received") | ||
| 348 | } | 349 | } | ||
| 349 | 350 | ||||
| 350 | - | sourceOrigin := popupSourceOrigin(payload) | |||
| 351 | var dataExpr string | 351 | var dataExpr string | ||
| 352 | switch strings.ToLower(strings.TrimSpace(payload.DataKind)) { | 352 | switch strings.ToLower(strings.TrimSpace(payload.DataKind)) { | ||
| 353 | case "json": | 353 | case "json": | ||
| @@ -358,7 +358,6 @@ func (wv *WebView) handlePopupOpenerPostMessage(payload popupOpenerPostMessagePa | |||||
| 358 | dataExpr = fmt.Sprintf("'%s'", webutil.EscapeForJSString(payload.Data)) | 358 | dataExpr = fmt.Sprintf("'%s'", webutil.EscapeForJSString(payload.Data)) | ||
| 359 | } | 359 | } | ||
| 360 | 360 | ||||
| 361 | - | sourceHref := strings.TrimSpace(payload.SourceHref) | |||
| 362 | script := fmt.Sprintf(`(function() { | 361 | script := fmt.Sprintf(`(function() { | ||
| 363 | try { | 362 | try { | ||
| 364 | var data; | 363 | var data; | ||
| @@ -405,11 +404,18 @@ func targetOriginMatchesPopupOpener(targetOrigin, openerURI string) bool { | |||||
| 405 | return targetCanonical != "" && targetCanonical == canonicalOrigin(openerURI) | 404 | return targetCanonical != "" && targetCanonical == canonicalOrigin(openerURI) | ||
| 406 | } | 405 | } | ||
| 407 | 406 | ||||
| 408 | - | func popupSourceOrigin(payload popupOpenerPostMessagePayload) string { | 407 | + | func popupOpenerSourceMetadata(wv *WebView) (origin, href string) { |
| 409 | - | if origin := strings.TrimSpace(payload.SourceOrigin); origin != "" { | 408 | + | if wv == nil { |
| 410 | - | return origin | 409 | + | return "", "" |
| 410 | + | } | |||
| 411 | + | href = strings.TrimSpace(wv.URI()) | |||
| 412 | + | if href == "" { | |||
| 413 | + | href = strings.TrimSpace(wv.pendingNavigationURI()) | |||
| 414 | + | } | |||
| 415 | + | if href == "" { | |||
| 416 | + | return "", "" | |||
| 411 | } | 417 | } | ||
| 412 | - | return originFromURL(payload.SourceHref) | 418 | + | return originFromURL(href), href |
| 413 | } | 419 | } | ||
| 414 | 420 | ||||
| 415 | func originFromURL(rawURL string) string { | 421 | func originFromURL(rawURL string) string { | ||
M internal/infrastructure/cef/popup_opener_bridge_test.go +30 -0
| @@ -1,8 +1,12 @@ | |||
| 1 | 1 | package cef | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 5 | + | "strings" | |
| 4 | 6 | "testing" | |
| 5 | 7 | ||
| 8 | + | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 6 | 10 | "github.com/stretchr/testify/require" | |
| 7 | 11 | ) | |
| 8 | 12 | ||
| @@ -39,3 +43,29 @@ func TestOriginFromURL_NormalizesInternalConceptualURLs(t *testing.T) { | |||
| 39 | 43 | func TestTargetOriginMatchesPopupOpener_NormalizesDefaultPorts(t *testing.T) { | |
| 40 | 44 | require.True(t, targetOriginMatchesPopupOpener("https://example.com:443", "https://example.com/callback")) | |
| 41 | 45 | } | |
| 46 | + | ||
| 47 | + | func TestHandlePopupOpenerPostMessage_UsesPopupCommittedURIForSourceMetadata(t *testing.T) { | |
| 48 | + | frame := cefmocks.NewMockFrame(t) | |
| 49 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |
| 50 | + | return strings.Contains(script, "https://popup.example") && | |
| 51 | + | strings.Contains(script, "https://popup.example/oauth/callback") && | |
| 52 | + | !strings.Contains(script, "https://evil.example") | |
| 53 | + | }), "", int32(0)).Once() | |
| 54 | + | browser := cefmocks.NewMockBrowser(t) | |
| 55 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |
| 56 | + | ||
| 57 | + | opener := &WebView{ctx: context.Background(), browser: browser, uri: "https://app.example/session"} | |
| 58 | + | popup := &WebView{ | |
| 59 | + | ctx: context.Background(), | |
| 60 | + | uri: "https://popup.example/oauth/callback", | |
| 61 | + | popupOpenerBridgeParent: opener, | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | popup.handlePopupOpenerPostMessage(popupOpenerPostMessagePayload{ | |
| 65 | + | Data: `{"ok":true}`, | |
| 66 | + | DataKind: "json", | |
| 67 | + | TargetOrigin: "https://app.example", | |
| 68 | + | SourceOrigin: "https://evil.example", | |
| 69 | + | SourceHref: "https://evil.example/fake", | |
| 70 | + | }) | |
| 71 | + | } | |
| @@ -1,8 +1,12 @@ | |||||
| 1 | package cef | 1 | package cef | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 5 | + | "strings" | |||
| 4 | "testing" | 6 | "testing" | ||
| 5 | 7 | ||||
| 8 | + | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 6 | "github.com/stretchr/testify/require" | 10 | "github.com/stretchr/testify/require" | ||
| 7 | ) | 11 | ) | ||
| 8 | 12 | ||||
| @@ -39,3 +43,29 @@ func TestOriginFromURL_NormalizesInternalConceptualURLs(t *testing.T) { | |||||
| 39 | func TestTargetOriginMatchesPopupOpener_NormalizesDefaultPorts(t *testing.T) { | 43 | func TestTargetOriginMatchesPopupOpener_NormalizesDefaultPorts(t *testing.T) { | ||
| 40 | require.True(t, targetOriginMatchesPopupOpener("https://example.com:443", "https://example.com/callback")) | 44 | require.True(t, targetOriginMatchesPopupOpener("https://example.com:443", "https://example.com/callback")) | ||
| 41 | } | 45 | } | ||
| 46 | + | ||||
| 47 | + | func TestHandlePopupOpenerPostMessage_UsesPopupCommittedURIForSourceMetadata(t *testing.T) { | |||
| 48 | + | frame := cefmocks.NewMockFrame(t) | |||
| 49 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |||
| 50 | + | return strings.Contains(script, "https://popup.example") && | |||
| 51 | + | strings.Contains(script, "https://popup.example/oauth/callback") && | |||
| 52 | + | !strings.Contains(script, "https://evil.example") | |||
| 53 | + | }), "", int32(0)).Once() | |||
| 54 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 55 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |||
| 56 | + | ||||
| 57 | + | opener := &WebView{ctx: context.Background(), browser: browser, uri: "https://app.example/session"} | |||
| 58 | + | popup := &WebView{ | |||
| 59 | + | ctx: context.Background(), | |||
| 60 | + | uri: "https://popup.example/oauth/callback", | |||
| 61 | + | popupOpenerBridgeParent: opener, | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | popup.handlePopupOpenerPostMessage(popupOpenerPostMessagePayload{ | |||
| 65 | + | Data: `{"ok":true}`, | |||
| 66 | + | DataKind: "json", | |||
| 67 | + | TargetOrigin: "https://app.example", | |||
| 68 | + | SourceOrigin: "https://evil.example", | |||
| 69 | + | SourceHref: "https://evil.example/fake", | |||
| 70 | + | }) | |||
| 71 | + | } | |||
M internal/infrastructure/cef/renderer_bridge_test.go +48 -0
| @@ -164,6 +164,54 @@ func TestDecodeRendererBridgePopupClosePayload(t *testing.T) { | |||
| 164 | 164 | require.Equal(t, "popup-1", req.ProxyID) | |
| 165 | 165 | } | |
| 166 | 166 | ||
| 167 | + | func TestDecodeRendererBridgePopupPayloadErrors(t *testing.T) { | |
| 168 | + | tests := []struct { | |
| 169 | + | name string | |
| 170 | + | decode func([]byte) error | |
| 171 | + | }{ | |
| 172 | + | { | |
| 173 | + | name: "open", | |
| 174 | + | decode: func(payload []byte) error { | |
| 175 | + | _, err := decodeRendererBridgePopupOpenPayload(payload) | |
| 176 | + | return err | |
| 177 | + | }, | |
| 178 | + | }, | |
| 179 | + | { | |
| 180 | + | name: "navigate", | |
| 181 | + | decode: func(payload []byte) error { | |
| 182 | + | _, err := decodeRendererBridgePopupNavigatePayload(payload) | |
| 183 | + | return err | |
| 184 | + | }, | |
| 185 | + | }, | |
| 186 | + | { | |
| 187 | + | name: "close", | |
| 188 | + | decode: func(payload []byte) error { | |
| 189 | + | _, err := decodeRendererBridgePopupClosePayload(payload) | |
| 190 | + | return err | |
| 191 | + | }, | |
| 192 | + | }, | |
| 193 | + | } | |
| 194 | + | ||
| 195 | + | payloads := []struct { | |
| 196 | + | name string | |
| 197 | + | payload []byte | |
| 198 | + | }{ | |
| 199 | + | {name: "empty", payload: nil}, | |
| 200 | + | {name: "invalid json", payload: []byte(`{invalid`)}, | |
| 201 | + | {name: "missing proxy id", payload: []byte(`{"url":"https://example.com"}`)}, | |
| 202 | + | } | |
| 203 | + | ||
| 204 | + | for _, test := range tests { | |
| 205 | + | t.Run(test.name, func(t *testing.T) { | |
| 206 | + | for _, payload := range payloads { | |
| 207 | + | t.Run(payload.name, func(t *testing.T) { | |
| 208 | + | require.Error(t, test.decode(payload.payload)) | |
| 209 | + | }) | |
| 210 | + | } | |
| 211 | + | }) | |
| 212 | + | } | |
| 213 | + | } | |
| 214 | + | ||
| 167 | 215 | func parseCEFPackageFiles(t *testing.T) []*ast.File { | |
| 168 | 216 | t.Helper() | |
| 169 | 217 | entries, err := os.ReadDir(".") | |
| @@ -164,6 +164,54 @@ func TestDecodeRendererBridgePopupClosePayload(t *testing.T) { | |||||
| 164 | require.Equal(t, "popup-1", req.ProxyID) | 164 | require.Equal(t, "popup-1", req.ProxyID) | ||
| 165 | } | 165 | } | ||
| 166 | 166 | ||||
| 167 | + | func TestDecodeRendererBridgePopupPayloadErrors(t *testing.T) { | |||
| 168 | + | tests := []struct { | |||
| 169 | + | name string | |||
| 170 | + | decode func([]byte) error | |||
| 171 | + | }{ | |||
| 172 | + | { | |||
| 173 | + | name: "open", | |||
| 174 | + | decode: func(payload []byte) error { | |||
| 175 | + | _, err := decodeRendererBridgePopupOpenPayload(payload) | |||
| 176 | + | return err | |||
| 177 | + | }, | |||
| 178 | + | }, | |||
| 179 | + | { | |||
| 180 | + | name: "navigate", | |||
| 181 | + | decode: func(payload []byte) error { | |||
| 182 | + | _, err := decodeRendererBridgePopupNavigatePayload(payload) | |||
| 183 | + | return err | |||
| 184 | + | }, | |||
| 185 | + | }, | |||
| 186 | + | { | |||
| 187 | + | name: "close", | |||
| 188 | + | decode: func(payload []byte) error { | |||
| 189 | + | _, err := decodeRendererBridgePopupClosePayload(payload) | |||
| 190 | + | return err | |||
| 191 | + | }, | |||
| 192 | + | }, | |||
| 193 | + | } | |||
| 194 | + | ||||
| 195 | + | payloads := []struct { | |||
| 196 | + | name string | |||
| 197 | + | payload []byte | |||
| 198 | + | }{ | |||
| 199 | + | {name: "empty", payload: nil}, | |||
| 200 | + | {name: "invalid json", payload: []byte(`{invalid`)}, | |||
| 201 | + | {name: "missing proxy id", payload: []byte(`{"url":"https://example.com"}`)}, | |||
| 202 | + | } | |||
| 203 | + | ||||
| 204 | + | for _, test := range tests { | |||
| 205 | + | t.Run(test.name, func(t *testing.T) { | |||
| 206 | + | for _, payload := range payloads { | |||
| 207 | + | t.Run(payload.name, func(t *testing.T) { | |||
| 208 | + | require.Error(t, test.decode(payload.payload)) | |||
| 209 | + | }) | |||
| 210 | + | } | |||
| 211 | + | }) | |||
| 212 | + | } | |||
| 213 | + | } | |||
| 214 | + | ||||
| 167 | func parseCEFPackageFiles(t *testing.T) []*ast.File { | 215 | func parseCEFPackageFiles(t *testing.T) []*ast.File { | ||
| 168 | t.Helper() | 216 | t.Helper() | ||
| 169 | entries, err := os.ReadDir(".") | 217 | entries, err := os.ReadDir(".") | ||
M internal/infrastructure/cef/scheme_handler_test.go +97 -0
| @@ -374,6 +374,103 @@ func TestSchemeHandler_APIPopupCloseInvokesPopupCallback(t *testing.T) { | |||
| 374 | 374 | require.Equal(t, "popup-1", got.ProxyID) | |
| 375 | 375 | } | |
| 376 | 376 | ||
| 377 | + | func TestSchemeHandler_APIPopupBridgeRejectsInvalidPayloads(t *testing.T) { | |
| 378 | + | oldNewResourceHandler := cefNewResourceHandler | |
| 379 | + | cefNewResourceHandler = func(impl purecef.ResourceHandler) purecef.ResourceHandler { return impl } | |
| 380 | + | defer func() { cefNewResourceHandler = oldNewResourceHandler }() | |
| 381 | + | ||
| 382 | + | tests := []struct { | |
| 383 | + | name string | |
| 384 | + | path string | |
| 385 | + | body string | |
| 386 | + | wire func(*dumbSchemeHandler, *bool) | |
| 387 | + | }{ | |
| 388 | + | { | |
| 389 | + | name: "open malformed json", | |
| 390 | + | path: "/api/popup-open", | |
| 391 | + | body: `{invalid`, | |
| 392 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |
| 393 | + | h.onPopupOpen = func(purecef.Browser, rendererBridgePopupOpenPayload) { *called = true } | |
| 394 | + | }, | |
| 395 | + | }, | |
| 396 | + | { | |
| 397 | + | name: "open missing proxy id", | |
| 398 | + | path: "/api/popup-open", | |
| 399 | + | body: `{"url":"https://example.com/login"}`, | |
| 400 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |
| 401 | + | h.onPopupOpen = func(purecef.Browser, rendererBridgePopupOpenPayload) { *called = true } | |
| 402 | + | }, | |
| 403 | + | }, | |
| 404 | + | { | |
| 405 | + | name: "navigate malformed json", | |
| 406 | + | path: "/api/popup-navigate", | |
| 407 | + | body: `{invalid`, | |
| 408 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |
| 409 | + | h.onPopupNavigate = func(purecef.Browser, rendererBridgePopupNavigatePayload) { *called = true } | |
| 410 | + | }, | |
| 411 | + | }, | |
| 412 | + | { | |
| 413 | + | name: "navigate missing proxy id", | |
| 414 | + | path: "/api/popup-navigate", | |
| 415 | + | body: `{"url":"https://example.com/callback"}`, | |
| 416 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |
| 417 | + | h.onPopupNavigate = func(purecef.Browser, rendererBridgePopupNavigatePayload) { *called = true } | |
| 418 | + | }, | |
| 419 | + | }, | |
| 420 | + | { | |
| 421 | + | name: "close malformed json", | |
| 422 | + | path: "/api/popup-close", | |
| 423 | + | body: `{invalid`, | |
| 424 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |
| 425 | + | h.onPopupClose = func(purecef.Browser, rendererBridgePopupClosePayload) { *called = true } | |
| 426 | + | }, | |
| 427 | + | }, | |
| 428 | + | { | |
| 429 | + | name: "close missing proxy id", | |
| 430 | + | path: "/api/popup-close", | |
| 431 | + | body: `{"url":"https://example.com/callback"}`, | |
| 432 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |
| 433 | + | h.onPopupClose = func(purecef.Browser, rendererBridgePopupClosePayload) { *called = true } | |
| 434 | + | }, | |
| 435 | + | }, | |
| 436 | + | } | |
| 437 | + | ||
| 438 | + | for _, test := range tests { | |
| 439 | + | t.Run(test.name, func(t *testing.T) { | |
| 440 | + | h, err := newDumbSchemeHandler(context.Background(), nil, nil, func() ([]byte, error) { return []byte(`{}`), nil }, func() ([]byte, error) { return []byte(`{}`), nil }) | |
| 441 | + | require.NoError(t, err) | |
| 442 | + | ||
| 443 | + | const bridgeNonce = "bridge-nonce" | |
| 444 | + | browser := cefmocks.NewMockBrowser(t) | |
| 445 | + | h.bridgeNonceValidator = func(got purecef.Browser, gotNonce string) bool { | |
| 446 | + | return got == browser && gotNonce == bridgeNonce | |
| 447 | + | } | |
| 448 | + | ||
| 449 | + | called := false | |
| 450 | + | test.wire(h, &called) | |
| 451 | + | ||
| 452 | + | request := cefmocks.NewMockRequest(t) | |
| 453 | + | request.EXPECT().GetHeaderByName(dumberBridgeNonceHeaderName).Return(bridgeNonce).Once() | |
| 454 | + | encoded := base64.StdEncoding.EncodeToString([]byte(test.body)) | |
| 455 | + | request.EXPECT().GetHeaderByName(dumberBodyHeaderName).Return(encoded).Once() | |
| 456 | + | ||
| 457 | + | response := cefmocks.NewMockResponse(t) | |
| 458 | + | response.EXPECT().SetStatus(int32(http.StatusBadRequest)).Once() | |
| 459 | + | response.EXPECT().SetStatusText(http.StatusText(http.StatusBadRequest)).Once() | |
| 460 | + | response.EXPECT().SetMimeType("application/json").Once() | |
| 461 | + | expectAPIResponseHeaders(response) | |
| 462 | + | ||
| 463 | + | handler := h.handleAPI(browser, http.MethodPost, test.path, request) | |
| 464 | + | require.NotNil(t, handler) | |
| 465 | + | ||
| 466 | + | var responseLength int64 | |
| 467 | + | handler.GetResponseHeaders(response, &responseLength, 0) | |
| 468 | + | require.Positive(t, responseLength) | |
| 469 | + | require.False(t, called) | |
| 470 | + | }) | |
| 471 | + | } | |
| 472 | + | } | |
| 473 | + | ||
| 377 | 474 | func TestSchemeHandler_APIOptionsReturnsCORSPreflightHeaders(t *testing.T) { | |
| 378 | 475 | oldNewResourceHandler := cefNewResourceHandler | |
| 379 | 476 | cefNewResourceHandler = func(impl purecef.ResourceHandler) purecef.ResourceHandler { return impl } | |
| @@ -374,6 +374,103 @@ func TestSchemeHandler_APIPopupCloseInvokesPopupCallback(t *testing.T) { | |||||
| 374 | require.Equal(t, "popup-1", got.ProxyID) | 374 | require.Equal(t, "popup-1", got.ProxyID) | ||
| 375 | } | 375 | } | ||
| 376 | 376 | ||||
| 377 | + | func TestSchemeHandler_APIPopupBridgeRejectsInvalidPayloads(t *testing.T) { | |||
| 378 | + | oldNewResourceHandler := cefNewResourceHandler | |||
| 379 | + | cefNewResourceHandler = func(impl purecef.ResourceHandler) purecef.ResourceHandler { return impl } | |||
| 380 | + | defer func() { cefNewResourceHandler = oldNewResourceHandler }() | |||
| 381 | + | ||||
| 382 | + | tests := []struct { | |||
| 383 | + | name string | |||
| 384 | + | path string | |||
| 385 | + | body string | |||
| 386 | + | wire func(*dumbSchemeHandler, *bool) | |||
| 387 | + | }{ | |||
| 388 | + | { | |||
| 389 | + | name: "open malformed json", | |||
| 390 | + | path: "/api/popup-open", | |||
| 391 | + | body: `{invalid`, | |||
| 392 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |||
| 393 | + | h.onPopupOpen = func(purecef.Browser, rendererBridgePopupOpenPayload) { *called = true } | |||
| 394 | + | }, | |||
| 395 | + | }, | |||
| 396 | + | { | |||
| 397 | + | name: "open missing proxy id", | |||
| 398 | + | path: "/api/popup-open", | |||
| 399 | + | body: `{"url":"https://example.com/login"}`, | |||
| 400 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |||
| 401 | + | h.onPopupOpen = func(purecef.Browser, rendererBridgePopupOpenPayload) { *called = true } | |||
| 402 | + | }, | |||
| 403 | + | }, | |||
| 404 | + | { | |||
| 405 | + | name: "navigate malformed json", | |||
| 406 | + | path: "/api/popup-navigate", | |||
| 407 | + | body: `{invalid`, | |||
| 408 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |||
| 409 | + | h.onPopupNavigate = func(purecef.Browser, rendererBridgePopupNavigatePayload) { *called = true } | |||
| 410 | + | }, | |||
| 411 | + | }, | |||
| 412 | + | { | |||
| 413 | + | name: "navigate missing proxy id", | |||
| 414 | + | path: "/api/popup-navigate", | |||
| 415 | + | body: `{"url":"https://example.com/callback"}`, | |||
| 416 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |||
| 417 | + | h.onPopupNavigate = func(purecef.Browser, rendererBridgePopupNavigatePayload) { *called = true } | |||
| 418 | + | }, | |||
| 419 | + | }, | |||
| 420 | + | { | |||
| 421 | + | name: "close malformed json", | |||
| 422 | + | path: "/api/popup-close", | |||
| 423 | + | body: `{invalid`, | |||
| 424 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |||
| 425 | + | h.onPopupClose = func(purecef.Browser, rendererBridgePopupClosePayload) { *called = true } | |||
| 426 | + | }, | |||
| 427 | + | }, | |||
| 428 | + | { | |||
| 429 | + | name: "close missing proxy id", | |||
| 430 | + | path: "/api/popup-close", | |||
| 431 | + | body: `{"url":"https://example.com/callback"}`, | |||
| 432 | + | wire: func(h *dumbSchemeHandler, called *bool) { | |||
| 433 | + | h.onPopupClose = func(purecef.Browser, rendererBridgePopupClosePayload) { *called = true } | |||
| 434 | + | }, | |||
| 435 | + | }, | |||
| 436 | + | } | |||
| 437 | + | ||||
| 438 | + | for _, test := range tests { | |||
| 439 | + | t.Run(test.name, func(t *testing.T) { | |||
| 440 | + | h, err := newDumbSchemeHandler(context.Background(), nil, nil, func() ([]byte, error) { return []byte(`{}`), nil }, func() ([]byte, error) { return []byte(`{}`), nil }) | |||
| 441 | + | require.NoError(t, err) | |||
| 442 | + | ||||
| 443 | + | const bridgeNonce = "bridge-nonce" | |||
| 444 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 445 | + | h.bridgeNonceValidator = func(got purecef.Browser, gotNonce string) bool { | |||
| 446 | + | return got == browser && gotNonce == bridgeNonce | |||
| 447 | + | } | |||
| 448 | + | ||||
| 449 | + | called := false | |||
| 450 | + | test.wire(h, &called) | |||
| 451 | + | ||||
| 452 | + | request := cefmocks.NewMockRequest(t) | |||
| 453 | + | request.EXPECT().GetHeaderByName(dumberBridgeNonceHeaderName).Return(bridgeNonce).Once() | |||
| 454 | + | encoded := base64.StdEncoding.EncodeToString([]byte(test.body)) | |||
| 455 | + | request.EXPECT().GetHeaderByName(dumberBodyHeaderName).Return(encoded).Once() | |||
| 456 | + | ||||
| 457 | + | response := cefmocks.NewMockResponse(t) | |||
| 458 | + | response.EXPECT().SetStatus(int32(http.StatusBadRequest)).Once() | |||
| 459 | + | response.EXPECT().SetStatusText(http.StatusText(http.StatusBadRequest)).Once() | |||
| 460 | + | response.EXPECT().SetMimeType("application/json").Once() | |||
| 461 | + | expectAPIResponseHeaders(response) | |||
| 462 | + | ||||
| 463 | + | handler := h.handleAPI(browser, http.MethodPost, test.path, request) | |||
| 464 | + | require.NotNil(t, handler) | |||
| 465 | + | ||||
| 466 | + | var responseLength int64 | |||
| 467 | + | handler.GetResponseHeaders(response, &responseLength, 0) | |||
| 468 | + | require.Positive(t, responseLength) | |||
| 469 | + | require.False(t, called) | |||
| 470 | + | }) | |||
| 471 | + | } | |||
| 472 | + | } | |||
| 473 | + | ||||
| 377 | func TestSchemeHandler_APIOptionsReturnsCORSPreflightHeaders(t *testing.T) { | 474 | func TestSchemeHandler_APIOptionsReturnsCORSPreflightHeaders(t *testing.T) { | ||
| 378 | oldNewResourceHandler := cefNewResourceHandler | 475 | oldNewResourceHandler := cefNewResourceHandler | ||
| 379 | cefNewResourceHandler = func(impl purecef.ResourceHandler) purecef.ResourceHandler { return impl } | 476 | cefNewResourceHandler = func(impl purecef.ResourceHandler) purecef.ResourceHandler { return impl } | ||
M internal/infrastructure/cef/webview.go +20 -10
| @@ -177,10 +177,11 @@ type WebView struct { | |||
| 177 | 177 | // pendingBrowserCreate holds the parameters needed to create a CEF browser, | |
| 178 | 178 | // deferred until the GL area has a non-zero size. | |
| 179 | 179 | type pendingBrowserCreate struct { | |
| 180 | - | windowInfo *purecef.WindowInfo | |
| 181 | - | client purecef.RawClient | |
| 182 | - | settings *purecef.BrowserSettings | |
| 183 | - | extraInfo purecef.DictionaryValue | |
| 180 | + | windowInfo *purecef.WindowInfo | |
| 181 | + | client purecef.RawClient | |
| 182 | + | settings *purecef.BrowserSettings | |
| 183 | + | extraInfo purecef.DictionaryValue | |
| 184 | + | postTaskRetries int | |
| 184 | 185 | } | |
| 185 | 186 | ||
| 186 | 187 | type cefTaskFunc func() | |
| @@ -685,14 +686,14 @@ func (wv *WebView) stopNativePopupFallbackTimer() { | |||
| 685 | 686 | } | |
| 686 | 687 | ||
| 687 | 688 | func (wv *WebView) scheduleNativePopupFallback(delay time.Duration, fn func()) { | |
| 688 | - | if wv == nil || fn == nil { | |
| 689 | + | if wv == nil || fn == nil || wv.destroyed.Load() { | |
| 689 | 690 | return | |
| 690 | 691 | } | |
| 691 | 692 | wv.stopNativePopupFallbackTimer() | |
| 692 | 693 | var timer stoppableTimer | |
| 693 | 694 | timer = wv.nativePopupFallbackScheduler()(delay, func() { | |
| 694 | 695 | wv.mu.Lock() | |
| 695 | - | if wv.nativePopupFallbackTimer != timer { | |
| 696 | + | if wv.destroyed.Load() || wv.nativePopupFallbackTimer != timer { | |
| 696 | 697 | wv.mu.Unlock() | |
| 697 | 698 | return | |
| 698 | 699 | } | |
| @@ -701,6 +702,13 @@ func (wv *WebView) scheduleNativePopupFallback(delay time.Duration, fn func()) { | |||
| 701 | 702 | fn() | |
| 702 | 703 | }) | |
| 703 | 704 | wv.mu.Lock() | |
| 705 | + | if wv.destroyed.Load() { | |
| 706 | + | wv.mu.Unlock() | |
| 707 | + | if timer != nil { | |
| 708 | + | timer.Stop() | |
| 709 | + | } | |
| 710 | + | return | |
| 711 | + | } | |
| 704 | 712 | wv.nativePopupFallbackTimer = timer | |
| 705 | 713 | wv.mu.Unlock() | |
| 706 | 714 | } | |
| @@ -711,7 +719,7 @@ func (wv *WebView) preparePopupShellDirectBrowserCreation() bool { | |||
| 711 | 719 | } | |
| 712 | 720 | wv.stopNativePopupFallbackTimer() | |
| 713 | 721 | wv.mu.Lock() | |
| 714 | - | if wv.browser != nil || wv.nativePopupFallbackStarted || wv.pendingCreate == nil { | |
| 722 | + | if wv.destroyed.Load() || wv.browser != nil || wv.nativePopupFallbackStarted || wv.pendingCreate == nil { | |
| 715 | 723 | wv.mu.Unlock() | |
| 716 | 724 | return false | |
| 717 | 725 | } | |
| @@ -721,7 +729,7 @@ func (wv *WebView) preparePopupShellDirectBrowserCreation() bool { | |||
| 721 | 729 | wv.nativePopupCandidate = false | |
| 722 | 730 | wv.nativePopupParent = nil | |
| 723 | 731 | wv.nativePopupID = 0 | |
| 724 | - | if !wv.popupNoJavaScriptAccess && parent != nil { | |
| 732 | + | if !wv.popupNoJavaScriptAccess && parent != nil && !parent.destroyed.Load() { | |
| 725 | 733 | wv.popupOpenerBridgeParent = parent | |
| 726 | 734 | wv.popupOpenerBridgeParentURI = parent.URI() | |
| 727 | 735 | } else { | |
| @@ -741,9 +749,10 @@ func (wv *WebView) startNativePopupFallback() bool { | |||
| 741 | 749 | return false | |
| 742 | 750 | } | |
| 743 | 751 | wv.mu.RLock() | |
| 752 | + | destroyed := wv.destroyed.Load() | |
| 744 | 753 | alreadyStarted := wv.nativePopupFallbackStarted | |
| 745 | 754 | wv.mu.RUnlock() | |
| 746 | - | if alreadyStarted { | |
| 755 | + | if destroyed || alreadyStarted { | |
| 747 | 756 | return false | |
| 748 | 757 | } | |
| 749 | 758 | return wv.preparePopupShellDirectBrowserCreation() | |
| @@ -986,7 +995,6 @@ func (wv *WebView) Destroy() { | |||
| 986 | 995 | wv.syntheticPopups = nil | |
| 987 | 996 | wv.syntheticPopupMu.Unlock() | |
| 988 | 997 | wv.mu.Lock() | |
| 989 | - | wv.closeCallbacks = nil | |
| 990 | 998 | wv.navigationCallbacks = nil | |
| 991 | 999 | wv.openerMessageCallbacks = nil | |
| 992 | 1000 | wv.openerNavigationCallbacks = nil | |
| @@ -1011,6 +1019,8 @@ func (wv *WebView) Destroy() { | |||
| 1011 | 1019 | wv.mu.RUnlock() | |
| 1012 | 1020 | if host != nil { | |
| 1013 | 1021 | host.CloseBrowser(1) | |
| 1022 | + | } else { | |
| 1023 | + | wv.runCloseCallbacks() | |
| 1014 | 1024 | } | |
| 1015 | 1025 | if wv.pipeline != nil { | |
| 1016 | 1026 | wv.pipeline.destroy() | |
| @@ -177,10 +177,11 @@ type WebView struct { | |||||
| 177 | // pendingBrowserCreate holds the parameters needed to create a CEF browser, | 177 | // pendingBrowserCreate holds the parameters needed to create a CEF browser, | ||
| 178 | // deferred until the GL area has a non-zero size. | 178 | // deferred until the GL area has a non-zero size. | ||
| 179 | type pendingBrowserCreate struct { | 179 | type pendingBrowserCreate struct { | ||
| 180 | - | windowInfo *purecef.WindowInfo | 180 | + | windowInfo *purecef.WindowInfo |
| 181 | - | client purecef.RawClient | 181 | + | client purecef.RawClient |
| 182 | - | settings *purecef.BrowserSettings | 182 | + | settings *purecef.BrowserSettings |
| 183 | - | extraInfo purecef.DictionaryValue | 183 | + | extraInfo purecef.DictionaryValue |
| 184 | + | postTaskRetries int | |||
| 184 | } | 185 | } | ||
| 185 | 186 | ||||
| 186 | type cefTaskFunc func() | 187 | type cefTaskFunc func() | ||
| @@ -685,14 +686,14 @@ func (wv *WebView) stopNativePopupFallbackTimer() { | |||||
| 685 | } | 686 | } | ||
| 686 | 687 | ||||
| 687 | func (wv *WebView) scheduleNativePopupFallback(delay time.Duration, fn func()) { | 688 | func (wv *WebView) scheduleNativePopupFallback(delay time.Duration, fn func()) { | ||
| 688 | - | if wv == nil || fn == nil { | 689 | + | if wv == nil || fn == nil || wv.destroyed.Load() { |
| 689 | return | 690 | return | ||
| 690 | } | 691 | } | ||
| 691 | wv.stopNativePopupFallbackTimer() | 692 | wv.stopNativePopupFallbackTimer() | ||
| 692 | var timer stoppableTimer | 693 | var timer stoppableTimer | ||
| 693 | timer = wv.nativePopupFallbackScheduler()(delay, func() { | 694 | timer = wv.nativePopupFallbackScheduler()(delay, func() { | ||
| 694 | wv.mu.Lock() | 695 | wv.mu.Lock() | ||
| 695 | - | if wv.nativePopupFallbackTimer != timer { | 696 | + | if wv.destroyed.Load() || wv.nativePopupFallbackTimer != timer { |
| 696 | wv.mu.Unlock() | 697 | wv.mu.Unlock() | ||
| 697 | return | 698 | return | ||
| 698 | } | 699 | } | ||
| @@ -701,6 +702,13 @@ func (wv *WebView) scheduleNativePopupFallback(delay time.Duration, fn func()) { | |||||
| 701 | fn() | 702 | fn() | ||
| 702 | }) | 703 | }) | ||
| 703 | wv.mu.Lock() | 704 | wv.mu.Lock() | ||
| 705 | + | if wv.destroyed.Load() { | |||
| 706 | + | wv.mu.Unlock() | |||
| 707 | + | if timer != nil { | |||
| 708 | + | timer.Stop() | |||
| 709 | + | } | |||
| 710 | + | return | |||
| 711 | + | } | |||
| 704 | wv.nativePopupFallbackTimer = timer | 712 | wv.nativePopupFallbackTimer = timer | ||
| 705 | wv.mu.Unlock() | 713 | wv.mu.Unlock() | ||
| 706 | } | 714 | } | ||
| @@ -711,7 +719,7 @@ func (wv *WebView) preparePopupShellDirectBrowserCreation() bool { | |||||
| 711 | } | 719 | } | ||
| 712 | wv.stopNativePopupFallbackTimer() | 720 | wv.stopNativePopupFallbackTimer() | ||
| 713 | wv.mu.Lock() | 721 | wv.mu.Lock() | ||
| 714 | - | if wv.browser != nil || wv.nativePopupFallbackStarted || wv.pendingCreate == nil { | 722 | + | if wv.destroyed.Load() || wv.browser != nil || wv.nativePopupFallbackStarted || wv.pendingCreate == nil { |
| 715 | wv.mu.Unlock() | 723 | wv.mu.Unlock() | ||
| 716 | return false | 724 | return false | ||
| 717 | } | 725 | } | ||
| @@ -721,7 +729,7 @@ func (wv *WebView) preparePopupShellDirectBrowserCreation() bool { | |||||
| 721 | wv.nativePopupCandidate = false | 729 | wv.nativePopupCandidate = false | ||
| 722 | wv.nativePopupParent = nil | 730 | wv.nativePopupParent = nil | ||
| 723 | wv.nativePopupID = 0 | 731 | wv.nativePopupID = 0 | ||
| 724 | - | if !wv.popupNoJavaScriptAccess && parent != nil { | 732 | + | if !wv.popupNoJavaScriptAccess && parent != nil && !parent.destroyed.Load() { |
| 725 | wv.popupOpenerBridgeParent = parent | 733 | wv.popupOpenerBridgeParent = parent | ||
| 726 | wv.popupOpenerBridgeParentURI = parent.URI() | 734 | wv.popupOpenerBridgeParentURI = parent.URI() | ||
| 727 | } else { | 735 | } else { | ||
| @@ -741,9 +749,10 @@ func (wv *WebView) startNativePopupFallback() bool { | |||||
| 741 | return false | 749 | return false | ||
| 742 | } | 750 | } | ||
| 743 | wv.mu.RLock() | 751 | wv.mu.RLock() | ||
| 752 | + | destroyed := wv.destroyed.Load() | |||
| 744 | alreadyStarted := wv.nativePopupFallbackStarted | 753 | alreadyStarted := wv.nativePopupFallbackStarted | ||
| 745 | wv.mu.RUnlock() | 754 | wv.mu.RUnlock() | ||
| 746 | - | if alreadyStarted { | 755 | + | if destroyed || alreadyStarted { |
| 747 | return false | 756 | return false | ||
| 748 | } | 757 | } | ||
| 749 | return wv.preparePopupShellDirectBrowserCreation() | 758 | return wv.preparePopupShellDirectBrowserCreation() | ||
| @@ -986,7 +995,6 @@ func (wv *WebView) Destroy() { | |||||
| 986 | wv.syntheticPopups = nil | 995 | wv.syntheticPopups = nil | ||
| 987 | wv.syntheticPopupMu.Unlock() | 996 | wv.syntheticPopupMu.Unlock() | ||
| 988 | wv.mu.Lock() | 997 | wv.mu.Lock() | ||
| 989 | - | wv.closeCallbacks = nil | |||
| 990 | wv.navigationCallbacks = nil | 998 | wv.navigationCallbacks = nil | ||
| 991 | wv.openerMessageCallbacks = nil | 999 | wv.openerMessageCallbacks = nil | ||
| 992 | wv.openerNavigationCallbacks = nil | 1000 | wv.openerNavigationCallbacks = nil | ||
| @@ -1011,6 +1019,8 @@ func (wv *WebView) Destroy() { | |||||
| 1011 | wv.mu.RUnlock() | 1019 | wv.mu.RUnlock() | ||
| 1012 | if host != nil { | 1020 | if host != nil { | ||
| 1013 | host.CloseBrowser(1) | 1021 | host.CloseBrowser(1) | ||
| 1022 | + | } else { | |||
| 1023 | + | wv.runCloseCallbacks() | |||
| 1014 | } | 1024 | } | ||
| 1015 | if wv.pipeline != nil { | 1025 | if wv.pipeline != nil { | ||
| 1016 | wv.pipeline.destroy() | 1026 | wv.pipeline.destroy() | ||
M internal/infrastructure/cef/webview_handlers_test.go +73 -0
| @@ -101,6 +101,10 @@ func (f stubFrame) CreateUrlrequest(purecef.Request, purecef.UrlrequestClient) p | |||
| 101 | 101 | return nil | |
| 102 | 102 | } | |
| 103 | 103 | ||
| 104 | + | type stubStoppableTimer struct{} | |
| 105 | + | ||
| 106 | + | func (stubStoppableTimer) Stop() bool { return true } | |
| 107 | + | ||
| 104 | 108 | func TestStartNativePopupFallback_AllowsPopupShellAwaitingArm(t *testing.T) { | |
| 105 | 109 | wv := &WebView{ | |
| 106 | 110 | nativePopupCandidate: true, | |
| @@ -125,6 +129,66 @@ func TestPreparePopupShellDirectBrowserCreation_EnablesSyntheticOpenerBridge(t * | |||
| 125 | 129 | require.Equal(t, "https://example.com/login", wv.popupOpenerBridgeParentURI) | |
| 126 | 130 | } | |
| 127 | 131 | ||
| 132 | + | func TestPreparePopupShellDirectBrowserCreation_SkipsDestroyedParentOpenerBridge(t *testing.T) { | |
| 133 | + | parent := &WebView{ctx: context.Background()} | |
| 134 | + | parent.updateURI("https://example.com/login") | |
| 135 | + | parent.destroyed.Store(true) | |
| 136 | + | wv := &WebView{ | |
| 137 | + | pendingCreate: &pendingBrowserCreate{}, | |
| 138 | + | nativePopupParent: parent, | |
| 139 | + | } | |
| 140 | + | ||
| 141 | + | require.True(t, wv.preparePopupShellDirectBrowserCreation()) | |
| 142 | + | require.Nil(t, wv.popupOpenerBridgeParent) | |
| 143 | + | require.Empty(t, wv.popupOpenerBridgeParentURI) | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | func TestScheduleNativePopupFallback_SkipsDestroyedWebView(t *testing.T) { | |
| 147 | + | called := false | |
| 148 | + | wv := &WebView{} | |
| 149 | + | wv.destroyed.Store(true) | |
| 150 | + | wv.nativePopupFallbackSchedule = func(time.Duration, func()) stoppableTimer { | |
| 151 | + | called = true | |
| 152 | + | return stubStoppableTimer{} | |
| 153 | + | } | |
| 154 | + | ||
| 155 | + | wv.scheduleNativePopupFallback(time.Millisecond, func() { | |
| 156 | + | called = true | |
| 157 | + | }) | |
| 158 | + | ||
| 159 | + | require.False(t, called) | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | func TestScheduleNativePopupFallback_DoesNotFireAfterDestroy(t *testing.T) { | |
| 163 | + | called := false | |
| 164 | + | var scheduled func() | |
| 165 | + | wv := &WebView{} | |
| 166 | + | wv.nativePopupFallbackSchedule = func(_ time.Duration, fn func()) stoppableTimer { | |
| 167 | + | scheduled = fn | |
| 168 | + | return stubStoppableTimer{} | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | wv.scheduleNativePopupFallback(time.Millisecond, func() { | |
| 172 | + | called = true | |
| 173 | + | }) | |
| 174 | + | require.NotNil(t, scheduled) | |
| 175 | + | ||
| 176 | + | wv.destroyed.Store(true) | |
| 177 | + | scheduled() | |
| 178 | + | ||
| 179 | + | require.False(t, called) | |
| 180 | + | } | |
| 181 | + | ||
| 182 | + | func TestStartNativePopupFallback_RejectsDestroyedWebView(t *testing.T) { | |
| 183 | + | wv := &WebView{ | |
| 184 | + | nativePopupCandidate: true, | |
| 185 | + | pendingCreate: &pendingBrowserCreate{}, | |
| 186 | + | } | |
| 187 | + | wv.destroyed.Store(true) | |
| 188 | + | ||
| 189 | + | require.False(t, wv.startNativePopupFallback()) | |
| 190 | + | } | |
| 191 | + | ||
| 128 | 192 | func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testing.T) { | |
| 129 | 193 | closed := false | |
| 130 | 194 | parent := &WebView{} | |
| @@ -371,6 +435,15 @@ func TestOnTextSelectionChanged_SuppressesAutoCopyWhenFocusedNodeEditableAndResu | |||
| 371 | 435 | require.Equal(t, "free selection", recorder.selection.Text) | |
| 372 | 436 | } | |
| 373 | 437 | ||
| 438 | + | func TestDestroy_WithoutHostRunsCloseCallbacks(t *testing.T) { | |
| 439 | + | called := false | |
| 440 | + | wv := &WebView{closeCallbacks: []func(){func() { called = true }}} | |
| 441 | + | ||
| 442 | + | wv.Destroy() | |
| 443 | + | ||
| 444 | + | require.True(t, called) | |
| 445 | + | } | |
| 446 | + | ||
| 374 | 447 | func TestOnTextSelectionChanged_DoesNotEmitLateDebouncedUpdateAfterDestroy(t *testing.T) { | |
| 375 | 448 | scheduler := &controlledSelectionScheduler{} | |
| 376 | 449 | orchestrator := portmocks.NewMockClipboardTextOrchestrator(t) | |
| @@ -101,6 +101,10 @@ func (f stubFrame) CreateUrlrequest(purecef.Request, purecef.UrlrequestClient) p | |||||
| 101 | return nil | 101 | return nil | ||
| 102 | } | 102 | } | ||
| 103 | 103 | ||||
| 104 | + | type stubStoppableTimer struct{} | |||
| 105 | + | ||||
| 106 | + | func (stubStoppableTimer) Stop() bool { return true } | |||
| 107 | + | ||||
| 104 | func TestStartNativePopupFallback_AllowsPopupShellAwaitingArm(t *testing.T) { | 108 | func TestStartNativePopupFallback_AllowsPopupShellAwaitingArm(t *testing.T) { | ||
| 105 | wv := &WebView{ | 109 | wv := &WebView{ | ||
| 106 | nativePopupCandidate: true, | 110 | nativePopupCandidate: true, | ||
| @@ -125,6 +129,66 @@ func TestPreparePopupShellDirectBrowserCreation_EnablesSyntheticOpenerBridge(t * | |||||
| 125 | require.Equal(t, "https://example.com/login", wv.popupOpenerBridgeParentURI) | 129 | require.Equal(t, "https://example.com/login", wv.popupOpenerBridgeParentURI) | ||
| 126 | } | 130 | } | ||
| 127 | 131 | ||||
| 132 | + | func TestPreparePopupShellDirectBrowserCreation_SkipsDestroyedParentOpenerBridge(t *testing.T) { | |||
| 133 | + | parent := &WebView{ctx: context.Background()} | |||
| 134 | + | parent.updateURI("https://example.com/login") | |||
| 135 | + | parent.destroyed.Store(true) | |||
| 136 | + | wv := &WebView{ | |||
| 137 | + | pendingCreate: &pendingBrowserCreate{}, | |||
| 138 | + | nativePopupParent: parent, | |||
| 139 | + | } | |||
| 140 | + | ||||
| 141 | + | require.True(t, wv.preparePopupShellDirectBrowserCreation()) | |||
| 142 | + | require.Nil(t, wv.popupOpenerBridgeParent) | |||
| 143 | + | require.Empty(t, wv.popupOpenerBridgeParentURI) | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | func TestScheduleNativePopupFallback_SkipsDestroyedWebView(t *testing.T) { | |||
| 147 | + | called := false | |||
| 148 | + | wv := &WebView{} | |||
| 149 | + | wv.destroyed.Store(true) | |||
| 150 | + | wv.nativePopupFallbackSchedule = func(time.Duration, func()) stoppableTimer { | |||
| 151 | + | called = true | |||
| 152 | + | return stubStoppableTimer{} | |||
| 153 | + | } | |||
| 154 | + | ||||
| 155 | + | wv.scheduleNativePopupFallback(time.Millisecond, func() { | |||
| 156 | + | called = true | |||
| 157 | + | }) | |||
| 158 | + | ||||
| 159 | + | require.False(t, called) | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | func TestScheduleNativePopupFallback_DoesNotFireAfterDestroy(t *testing.T) { | |||
| 163 | + | called := false | |||
| 164 | + | var scheduled func() | |||
| 165 | + | wv := &WebView{} | |||
| 166 | + | wv.nativePopupFallbackSchedule = func(_ time.Duration, fn func()) stoppableTimer { | |||
| 167 | + | scheduled = fn | |||
| 168 | + | return stubStoppableTimer{} | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | wv.scheduleNativePopupFallback(time.Millisecond, func() { | |||
| 172 | + | called = true | |||
| 173 | + | }) | |||
| 174 | + | require.NotNil(t, scheduled) | |||
| 175 | + | ||||
| 176 | + | wv.destroyed.Store(true) | |||
| 177 | + | scheduled() | |||
| 178 | + | ||||
| 179 | + | require.False(t, called) | |||
| 180 | + | } | |||
| 181 | + | ||||
| 182 | + | func TestStartNativePopupFallback_RejectsDestroyedWebView(t *testing.T) { | |||
| 183 | + | wv := &WebView{ | |||
| 184 | + | nativePopupCandidate: true, | |||
| 185 | + | pendingCreate: &pendingBrowserCreate{}, | |||
| 186 | + | } | |||
| 187 | + | wv.destroyed.Store(true) | |||
| 188 | + | ||||
| 189 | + | require.False(t, wv.startNativePopupFallback()) | |||
| 190 | + | } | |||
| 191 | + | ||||
| 128 | func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testing.T) { | 192 | func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testing.T) { | ||
| 129 | closed := false | 193 | closed := false | ||
| 130 | parent := &WebView{} | 194 | parent := &WebView{} | ||
| @@ -371,6 +435,15 @@ func TestOnTextSelectionChanged_SuppressesAutoCopyWhenFocusedNodeEditableAndResu | |||||
| 371 | require.Equal(t, "free selection", recorder.selection.Text) | 435 | require.Equal(t, "free selection", recorder.selection.Text) | ||
| 372 | } | 436 | } | ||
| 373 | 437 | ||||
| 438 | + | func TestDestroy_WithoutHostRunsCloseCallbacks(t *testing.T) { | |||
| 439 | + | called := false | |||
| 440 | + | wv := &WebView{closeCallbacks: []func(){func() { called = true }}} | |||
| 441 | + | ||||
| 442 | + | wv.Destroy() | |||
| 443 | + | ||||
| 444 | + | require.True(t, called) | |||
| 445 | + | } | |||
| 446 | + | ||||
| 374 | func TestOnTextSelectionChanged_DoesNotEmitLateDebouncedUpdateAfterDestroy(t *testing.T) { | 447 | func TestOnTextSelectionChanged_DoesNotEmitLateDebouncedUpdateAfterDestroy(t *testing.T) { | ||
| 375 | scheduler := &controlledSelectionScheduler{} | 448 | scheduler := &controlledSelectionScheduler{} | ||
| 376 | orchestrator := portmocks.NewMockClipboardTextOrchestrator(t) | 449 | orchestrator := portmocks.NewMockClipboardTextOrchestrator(t) | ||
M internal/ui/coordinator/content/oauth.go +3 -0
| @@ -334,6 +334,9 @@ func (c *Coordinator) setupOAuthAutoClose( | |||
| 334 | 334 | }) | |
| 335 | 335 | if opener, ok := wv.(port.PopupOpenerCapable); ok { | |
| 336 | 336 | opener.AddOpenerNavigationCallback(func(uri string) { | |
| 337 | + | if !ShouldAutoClose(uri) { | |
| 338 | + | return | |
| 339 | + | } | |
| 337 | 340 | c.capturePopupOAuthState(popupID, uri) | |
| 338 | 341 | requestOAuthClose("opener-navigation") | |
| 339 | 342 | }) | |
| @@ -334,6 +334,9 @@ func (c *Coordinator) setupOAuthAutoClose( | |||||
| 334 | }) | 334 | }) | ||
| 335 | if opener, ok := wv.(port.PopupOpenerCapable); ok { | 335 | if opener, ok := wv.(port.PopupOpenerCapable); ok { | ||
| 336 | opener.AddOpenerNavigationCallback(func(uri string) { | 336 | opener.AddOpenerNavigationCallback(func(uri string) { | ||
| 337 | + | if !ShouldAutoClose(uri) { | |||
| 338 | + | return | |||
| 339 | + | } | |||
| 337 | c.capturePopupOAuthState(popupID, uri) | 340 | c.capturePopupOAuthState(popupID, uri) | ||
| 338 | requestOAuthClose("opener-navigation") | 341 | requestOAuthClose("opener-navigation") | ||
| 339 | }) | 342 | }) | ||
M internal/ui/coordinator/content/oauth_test.go +56 -0
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | ||
| 8 | 8 | "github.com/stretchr/testify/assert" | |
| 9 | 9 | "github.com/stretchr/testify/mock" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 10 | 11 | ||
| 11 | 12 | "github.com/bnema/dumber/internal/application/port" | |
| 12 | 13 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| @@ -24,6 +25,37 @@ func (s *popupOpenerBridgeStateStub) AddOpenerNavigationCallback(func(string)) { | |||
| 24 | 25 | } | |
| 25 | 26 | func (s *popupOpenerBridgeStateStub) HasActivePopupOpenerBridge() bool { return s.active } | |
| 26 | 27 | ||
| 28 | + | type popupOAuthAutoCloseWebViewStub struct { | |
| 29 | + | *portmocks.MockWebView | |
| 30 | + | active bool | |
| 31 | + | navigationCallbacks []func(string) | |
| 32 | + | closeCallbacks []func() | |
| 33 | + | openerMessageCallbacks []func() | |
| 34 | + | openerNavigateCallbacks []func(string) | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | func (s *popupOAuthAutoCloseWebViewStub) AddNavigationCallback(fn func(string)) { | |
| 38 | + | s.navigationCallbacks = append(s.navigationCallbacks, fn) | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | func (s *popupOAuthAutoCloseWebViewStub) AddCloseCallback(fn func()) { | |
| 42 | + | s.closeCallbacks = append(s.closeCallbacks, fn) | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | func (*popupOAuthAutoCloseWebViewStub) Close() {} | |
| 46 | + | ||
| 47 | + | func (*popupOAuthAutoCloseWebViewStub) EnablePopupOpenerBridge(port.WebView, bool) {} | |
| 48 | + | ||
| 49 | + | func (s *popupOAuthAutoCloseWebViewStub) AddOpenerMessageCallback(fn func()) { | |
| 50 | + | s.openerMessageCallbacks = append(s.openerMessageCallbacks, fn) | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func (s *popupOAuthAutoCloseWebViewStub) AddOpenerNavigationCallback(fn func(string)) { | |
| 54 | + | s.openerNavigateCallbacks = append(s.openerNavigateCallbacks, fn) | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | func (s *popupOAuthAutoCloseWebViewStub) HasActivePopupOpenerBridge() bool { return s.active } | |
| 58 | + | ||
| 27 | 59 | func TestPopupUsesSyntheticOpenerSignals_DetectsActiveBridge(t *testing.T) { | |
| 28 | 60 | wv := &popupOpenerBridgeStateStub{MockWebView: portmocks.NewMockWebView(t), active: true} | |
| 29 | 61 | assert.True(t, popupUsesSyntheticOpenerSignals(wv)) | |
| @@ -87,6 +119,30 @@ func TestCapturePopupOAuthMessage_MarksPopupSeenAndSuccessful(t *testing.T) { | |||
| 87 | 119 | assert.Equal(t, "postmessage://oauth-complete", state.CallbackURI) | |
| 88 | 120 | } | |
| 89 | 121 | ||
| 122 | + | func TestSetupOAuthAutoClose_OpenerNavigationIgnoresNonTerminalURI(t *testing.T) { | |
| 123 | + | popupID := port.WebViewID(1000) | |
| 124 | + | wv := &popupOAuthAutoCloseWebViewStub{ | |
| 125 | + | MockWebView: portmocks.NewMockWebView(t), | |
| 126 | + | active: true, | |
| 127 | + | } | |
| 128 | + | c := &Coordinator{popups: newPopupManager()} | |
| 129 | + | c.trackOAuthPopup(popupID, entity.PaneID("parent-pane"), "https://example.com/start") | |
| 130 | + | ||
| 131 | + | c.setupOAuthAutoClose(context.Background(), entity.PaneID("popup-pane"), popupID, wv) | |
| 132 | + | require.Len(t, wv.openerNavigateCallbacks, 1) | |
| 133 | + | ||
| 134 | + | wv.openerNavigateCallbacks[0]("https://example.com/intermediate") | |
| 135 | + | ||
| 136 | + | state, ok := c.popups.popupOAuth[popupID] | |
| 137 | + | require.True(t, ok) | |
| 138 | + | assert.False(t, state.Seen) | |
| 139 | + | assert.Empty(t, state.CallbackURI) | |
| 140 | + | ||
| 141 | + | for _, fn := range wv.closeCallbacks { | |
| 142 | + | fn() | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | ||
| 90 | 146 | func TestHandlePopupOAuthClose_SuccessSchedulesParentResume(t *testing.T) { | |
| 91 | 147 | parentPaneID := entity.PaneID("parent-pane") | |
| 92 | 148 | popupID := port.WebViewID(101) | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | 7 | ||||
| 8 | "github.com/stretchr/testify/assert" | 8 | "github.com/stretchr/testify/assert" | ||
| 9 | "github.com/stretchr/testify/mock" | 9 | "github.com/stretchr/testify/mock" | ||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 10 | 11 | ||||
| 11 | "github.com/bnema/dumber/internal/application/port" | 12 | "github.com/bnema/dumber/internal/application/port" | ||
| 12 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | 13 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | ||
| @@ -24,6 +25,37 @@ func (s *popupOpenerBridgeStateStub) AddOpenerNavigationCallback(func(string)) { | |||||
| 24 | } | 25 | } | ||
| 25 | func (s *popupOpenerBridgeStateStub) HasActivePopupOpenerBridge() bool { return s.active } | 26 | func (s *popupOpenerBridgeStateStub) HasActivePopupOpenerBridge() bool { return s.active } | ||
| 26 | 27 | ||||
| 28 | + | type popupOAuthAutoCloseWebViewStub struct { | |||
| 29 | + | *portmocks.MockWebView | |||
| 30 | + | active bool | |||
| 31 | + | navigationCallbacks []func(string) | |||
| 32 | + | closeCallbacks []func() | |||
| 33 | + | openerMessageCallbacks []func() | |||
| 34 | + | openerNavigateCallbacks []func(string) | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | func (s *popupOAuthAutoCloseWebViewStub) AddNavigationCallback(fn func(string)) { | |||
| 38 | + | s.navigationCallbacks = append(s.navigationCallbacks, fn) | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | func (s *popupOAuthAutoCloseWebViewStub) AddCloseCallback(fn func()) { | |||
| 42 | + | s.closeCallbacks = append(s.closeCallbacks, fn) | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | func (*popupOAuthAutoCloseWebViewStub) Close() {} | |||
| 46 | + | ||||
| 47 | + | func (*popupOAuthAutoCloseWebViewStub) EnablePopupOpenerBridge(port.WebView, bool) {} | |||
| 48 | + | ||||
| 49 | + | func (s *popupOAuthAutoCloseWebViewStub) AddOpenerMessageCallback(fn func()) { | |||
| 50 | + | s.openerMessageCallbacks = append(s.openerMessageCallbacks, fn) | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | func (s *popupOAuthAutoCloseWebViewStub) AddOpenerNavigationCallback(fn func(string)) { | |||
| 54 | + | s.openerNavigateCallbacks = append(s.openerNavigateCallbacks, fn) | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | func (s *popupOAuthAutoCloseWebViewStub) HasActivePopupOpenerBridge() bool { return s.active } | |||
| 58 | + | ||||
| 27 | func TestPopupUsesSyntheticOpenerSignals_DetectsActiveBridge(t *testing.T) { | 59 | func TestPopupUsesSyntheticOpenerSignals_DetectsActiveBridge(t *testing.T) { | ||
| 28 | wv := &popupOpenerBridgeStateStub{MockWebView: portmocks.NewMockWebView(t), active: true} | 60 | wv := &popupOpenerBridgeStateStub{MockWebView: portmocks.NewMockWebView(t), active: true} | ||
| 29 | assert.True(t, popupUsesSyntheticOpenerSignals(wv)) | 61 | assert.True(t, popupUsesSyntheticOpenerSignals(wv)) | ||
| @@ -87,6 +119,30 @@ func TestCapturePopupOAuthMessage_MarksPopupSeenAndSuccessful(t *testing.T) { | |||||
| 87 | assert.Equal(t, "postmessage://oauth-complete", state.CallbackURI) | 119 | assert.Equal(t, "postmessage://oauth-complete", state.CallbackURI) | ||
| 88 | } | 120 | } | ||
| 89 | 121 | ||||
| 122 | + | func TestSetupOAuthAutoClose_OpenerNavigationIgnoresNonTerminalURI(t *testing.T) { | |||
| 123 | + | popupID := port.WebViewID(1000) | |||
| 124 | + | wv := &popupOAuthAutoCloseWebViewStub{ | |||
| 125 | + | MockWebView: portmocks.NewMockWebView(t), | |||
| 126 | + | active: true, | |||
| 127 | + | } | |||
| 128 | + | c := &Coordinator{popups: newPopupManager()} | |||
| 129 | + | c.trackOAuthPopup(popupID, entity.PaneID("parent-pane"), "https://example.com/start") | |||
| 130 | + | ||||
| 131 | + | c.setupOAuthAutoClose(context.Background(), entity.PaneID("popup-pane"), popupID, wv) | |||
| 132 | + | require.Len(t, wv.openerNavigateCallbacks, 1) | |||
| 133 | + | ||||
| 134 | + | wv.openerNavigateCallbacks[0]("https://example.com/intermediate") | |||
| 135 | + | ||||
| 136 | + | state, ok := c.popups.popupOAuth[popupID] | |||
| 137 | + | require.True(t, ok) | |||
| 138 | + | assert.False(t, state.Seen) | |||
| 139 | + | assert.Empty(t, state.CallbackURI) | |||
| 140 | + | ||||
| 141 | + | for _, fn := range wv.closeCallbacks { | |||
| 142 | + | fn() | |||
| 143 | + | } | |||
| 144 | + | } | |||
| 145 | + | ||||
| 90 | func TestHandlePopupOAuthClose_SuccessSchedulesParentResume(t *testing.T) { | 146 | func TestHandlePopupOAuthClose_SuccessSchedulesParentResume(t *testing.T) { | ||
| 91 | parentPaneID := entity.PaneID("parent-pane") | 147 | parentPaneID := entity.PaneID("parent-pane") | ||
| 92 | popupID := port.WebViewID(101) | 148 | popupID := port.WebViewID(101) | ||