Skip to content

Android: unset the request's header env vars after each dispatch - #337

Open
inoha-kudo wants to merge 1 commit into
NativePHP:mainfrom
inoha-kudo:fix/android-header-env-leak
Open

Android: unset the request's header env vars after each dispatch#337
inoha-kudo wants to merge 1 commit into
NativePHP:mainfrom
inoha-kudo:fix/android-header-env-leak

Conversation

@inoha-kudo

@inoha-kudo inoha-kudo commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Fixes #324. Follow-up to #119.

Headers reach PHP as HTTP_* environment variables, and the environment outlives a dispatch, so a request inherits every header of the one before it: a POST's Content-Type is still set when the next GET is dispatched, and so are X-Requested-With, Referer, Origin, and anything a JS library sends on only some requests.

#119 addressed the visible symptom by stripping HTTP_* and CONTENT_* from $_SERVER at the top of each dispatch, but the same eval re-imports the environment twenty lines later:

foreach (getenv() as $__k => $__v) { $_SERVER[$__k] = $__v; }

so the stale keys return immediately. The Kotlin band-aid in PHPBridge.kt covers this for X-Inertia only, and by blanking rather than removing.

iOS already does the right thing — PersistentPHPRuntime.swift records the keys it sets and unsetenvs them after the dispatch. This brings Android to the same behaviour.

What changed

handleLaravelRequest() records every header env var it sets (headerEnvKeys), runs the dispatch inside a try, and unsets them in finally — so a failed dispatch, including the native early returns (php_embed_init() failure, "runtime not initialized"), cannot leave its headers behind either. The list is local to the request on the single phpExecutor thread, so there is no shared state and nothing to lock. The C side gains one small JNI method, nativeUnsetEnv, since System.getenv() is a JVM snapshot and Kotlin otherwise has no way to remove a process variable.

Track-and-remove rather than sweeping environ for anything matching HTTP_*, for three reasons: pre-existing HTTP_* variables whose names the current request does not set are left alone (and a request that does send a Proxy: header has its HTTP_PROXY removed afterwards — the safer outcome either way); there is no name-length cutoff past which a header would escape removal; and it is exactly what iOS does.

The five-name Inertia blanking block is gone — it was this fix, restricted to five names and blanking instead of removing.

One deliberate behaviour change: a failed setenv used to go unnoticed and the request was dispatched with that header missing. It now aborts the request, and finally still removes whatever had been set up to that point.

Notes:

  • HTTP_HOST is re-set by the C side per dispatch (php_bridge.c:286, :487), which is untracked and unchanged — it is constant for the embedded server, so persisting is harmless.
  • native_webview_php_request() is unaffected: it receives its cookies and content type as parameters rather than through the environment, so there is nothing to unset.
  • bionic's setenv/unsetenv are not safe against a concurrent getenv() from another thread. That is a pre-existing property of passing headers through the environment; this change does not add to it — all header setenv/unsetenv calls stay on the single phpExecutor thread, as before.

Test plan

Physical device (Pixel 4a 5G, Android 14), release build via native:package android --build-type=release.

  • Route::get('/leak', fn () => response($_SERVER['HTTP_X_FOO'] ?? '(absent)')); one fetch with X-Foo: bar, then two without — before: bar, bar, bar; after: bar, (absent), (absent)
  • Inertia visit followed by a top-level navigation returns HTML, not a raw page object
  • Regular Inertia XHR navigation still receives JSON
  • POST with Content-Type: application/x-www-form-urlencoded still parses into $_POST after the change
  • Login → tab navigation → logout completes on a release build
  • Classic (non-persistent) runtime mode — nativeHandleRequest() sits inside the same try/finally so the path is covered identically, but I have only exercised the persistent default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android: request headers persist in the environment and leak into the next request

1 participant