From 32a5ef8c87c537ad19af1ccd51fc3bf732fd0f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Me=C3=9Fmer?= Date: Fri, 12 Jun 2026 08:24:53 +0200 Subject: [PATCH] fix arguments scope bug in fetch header injection callback In the string URL branch of the window.fetch override, the callback passed to setAdditionalHeaders used `arguments[1]` to update the fetch options headers. But inside a regular function() `arguments` is rebound to the callback's own argument list, which only receives `extraHeaders` as arguments[0], making arguments[1] always undefined. This caused a "TypeError: Cannot set properties of undefined (setting 'headers')". The error was caught silently by the try/catch in setAdditionalHeaders but a warning was logged. It only appeared for specific requests because only fetch calls using the Request object form (else branch) guard header injection inside a for...in loop, so they only error when extra headers are actually returned. Now we capture `arguments[1]` into a `fetchOptions` variable in the outer window.fetch scope before the callback and reference that variable inside the callback instead. This commit also adds tests for this particular case. --- app/src/androidTest/assets/test_requests.js | 26 +++++++ .../JavaScriptInterfaceTest.kt | 67 +++++++++++++++++++ .../RequestInspectorJavaScriptInterface.kt | 21 +++++- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/app/src/androidTest/assets/test_requests.js b/app/src/androidTest/assets/test_requests.js index 561c27d..edcfc85 100644 --- a/app/src/androidTest/assets/test_requests.js +++ b/app/src/androidTest/assets/test_requests.js @@ -9,6 +9,32 @@ function triggerFetchStringUrl() { }); } +// Headers passed as a Headers instance — exercises normalizeFetchHeaders() +function triggerFetchStringUrlWithHeadersInstance() { + fetch('https://example.com/api/headers-instance', { + method: 'GET', + headers: new Headers({ + 'X-From-Instance': 'instance-value' + }) + }); +} + +// Headers passed as an array of [name, value] tuples — exercises normalizeFetchHeaders() +function triggerFetchStringUrlWithHeadersArray() { + fetch('https://example.com/api/headers-array', { + method: 'GET', + headers: [ + ['X-From-Array', 'array-value'] + ] + }); +} + +// No options object at all — exercises the arguments-scope fix: arguments[1] starts +// as undefined and is set to {} before fetchOptions captures it. +function triggerFetchStringUrlNoOptions() { + fetch('https://example.com/api/no-options'); +} + function triggerFetchRequestObject() { fetch(new Request('https://example.com/api/request', { method: 'PUT', diff --git a/app/src/androidTest/java/com/acsbendi/requestinspectorwebview/JavaScriptInterfaceTest.kt b/app/src/androidTest/java/com/acsbendi/requestinspectorwebview/JavaScriptInterfaceTest.kt index 74f9349..476e912 100644 --- a/app/src/androidTest/java/com/acsbendi/requestinspectorwebview/JavaScriptInterfaceTest.kt +++ b/app/src/androidTest/java/com/acsbendi/requestinspectorwebview/JavaScriptInterfaceTest.kt @@ -86,6 +86,73 @@ class JavaScriptInterfaceTest { assertTrue("Expected URL to contain 'debug=true', was: $url", url.contains("debug=true")) } + // - fetch (string URL, non-plain-object headers) --------------- + + @Test + fun fetch_stringUrl_headersInstance_recordsHeaders() { + // normalizeFetchHeaders: Headers instance should be converted to a plain object + val latch = matcher.expectRequest() + runJs("triggerFetchStringUrlWithHeadersInstance()") + assertTrue("fetch (Headers instance) not recorded", latch.await(5, TimeUnit.SECONDS)) + + val req = matcher.lastRequest!! + assertEquals(WebViewRequestType.FETCH, req.type) + assertEquals("https://example.com/api/headers-instance", req.url.toString()) + assertEquals("instance-value", req.headers["x-from-instance"]) + } + + @Test + fun fetch_stringUrl_headersInstance_injectsAdditionalHeaders() { + // normalizeFetchHeaders + arguments-scope fix: injection must work when headers is a Headers instance + matcher.additionalHeaders = mapOf("X-Injected" to "injected-value") + val latch = matcher.expectRequest() + runJs("triggerFetchStringUrlWithHeadersInstance()") + assertTrue("fetch (Headers instance) with extra header not recorded", latch.await(5, TimeUnit.SECONDS)) + + val req = matcher.lastRequest!! + assertEquals("instance-value", req.headers["x-from-instance"]) + assertEquals("injected-value", req.headers["x-injected"]) + } + + @Test + fun fetch_stringUrl_headersArray_recordsHeaders() { + // normalizeFetchHeaders: array of [name, value] tuples should be converted to a plain object + val latch = matcher.expectRequest() + runJs("triggerFetchStringUrlWithHeadersArray()") + assertTrue("fetch (headers array) not recorded", latch.await(5, TimeUnit.SECONDS)) + + val req = matcher.lastRequest!! + assertEquals(WebViewRequestType.FETCH, req.type) + assertEquals("https://example.com/api/headers-array", req.url.toString()) + assertEquals("array-value", req.headers["x-from-array"]) + } + + @Test + fun fetch_stringUrl_headersArray_injectsAdditionalHeaders() { + // normalizeFetchHeaders + arguments-scope fix: injection must work when headers is an array + matcher.additionalHeaders = mapOf("X-Injected" to "injected-value") + val latch = matcher.expectRequest() + runJs("triggerFetchStringUrlWithHeadersArray()") + assertTrue("fetch (headers array) with extra header not recorded", latch.await(5, TimeUnit.SECONDS)) + + val req = matcher.lastRequest!! + assertEquals("array-value", req.headers["x-from-array"]) + assertEquals("injected-value", req.headers["x-injected"]) + } + + @Test + fun fetch_stringUrl_noOptions_injectsAdditionalHeaders() { + // arguments-scope fix: when fetch is called with no options object, arguments[1] starts as + // undefined and is initialised to {} in the override — fetchOptions must capture that new + // object so header injection does not throw inside the callback. + matcher.additionalHeaders = mapOf("X-Injected" to "injected-value") + val latch = matcher.expectRequest() + runJs("triggerFetchStringUrlNoOptions()") + assertTrue("fetch (no options) with extra header not recorded", latch.await(5, TimeUnit.SECONDS)) + + assertEquals("injected-value", matcher.lastRequest!!.headers["x-injected"]) + } + // - fetch (request object) ------------------------------------- @Test diff --git a/app/src/main/java/com/acsbendi/requestinspectorwebview/RequestInspectorJavaScriptInterface.kt b/app/src/main/java/com/acsbendi/requestinspectorwebview/RequestInspectorJavaScriptInterface.kt index df7c31d..4655671 100644 --- a/app/src/main/java/com/acsbendi/requestinspectorwebview/RequestInspectorJavaScriptInterface.kt +++ b/app/src/main/java/com/acsbendi/requestinspectorwebview/RequestInspectorJavaScriptInterface.kt @@ -256,6 +256,15 @@ function getFullUrl(url) { } } +// Normalizes the three forms fetch() accepts for headers — a Headers instance, +// an array of [name, value] tuples, or a plain object — into a plain object +// so it can be safely merged with the spread operator. +function normalizeFetchHeaders(headers) { + if (typeof Headers !== 'undefined' && headers instanceof Headers) return Object.fromEntries(headers.entries()); + if (Array.isArray(headers)) return Object.fromEntries(headers); + return headers || {}; +} + function setAdditionalHeaders(url, callback) { try { var extraHeaders = JSON.parse($INTERFACE_NAME.getAdditionalHeaders(url)); @@ -375,10 +384,18 @@ window.fetch = function () { if (!arguments[1]) arguments[1] = {}; method = 'method' in arguments[1] ? arguments[1]['method'] : "GET"; body = 'body' in arguments[1] ? arguments[1]['body'] : ""; + // Normalize headers up-front so JSON.stringify(headers) records correctly + // even if setAdditionalHeaders throws before invoking the callback. headers = 'headers' in arguments[1] ? arguments[1]['headers'] : {}; + headers = normalizeFetchHeaders(headers); + // Capture the options object here — inside the callback `arguments` is + // rebound to the callback's own argument list, so arguments[1] would be undefined. + var fetchOptions = arguments[1]; setAdditionalHeaders(url, function(extraHeaders) { - headers = { ...extraHeaders, ...headers }; - arguments[1].headers = headers; + // extraHeaders are applied first so that any headers already set by + // the caller take precedence over the injected ones. + fetchOptions.headers = { ...extraHeaders, ...headers }; + headers = fetchOptions.headers; }); arguments[0] = url; } else {