Fix libxposed API 101 conformance - #794
Merged
Merged
Conversation
ExceptionMode.PROTECTIVE is documented as: "If the exception is thrown before proceed(), the framework will continue the chain without the hook; if the exception is thrown after proceed, the framework will return the value / exception proceeded as the result." executeDownstream recorded a node's own hooker exception in downstreamThrowable but never cleared it when handleInterceptorException went on to recover, so the node advertised both a stale exception and a null result. A parent node reads exactly those two fields, so it would rethrow an exception the chain had already suppressed. Two DEFAULT-mode hooks are enough to hit it, with no concurrency: the lower-priority hook throws before proceed and is skipped, the chain produces the original value, then the higher-priority hook throws after proceed and the parent resurrects the first hook's exception. The caller sees IllegalStateException instead of the method's return value. Route the recovery through executeDownstream as well, and have it write both fields so exactly one is ever meaningful. Also return an immutable list from Chain.getArgs(), which the API documents as "The returned list is immutable" but toList() implements with a mutable ArrayList.
Two unrelated symptoms, one shared cause plus one missing guard. callbackSnapshot returns nullptr when the executable is absent from hooked_methods, but the Kotlin declaration was non-null, so Kotlin's own null check threw. Invokers default to Type.Chain.FULL, so the ordinary act of obtaining an invoker for a method the module has not hooked and calling it raised NullPointerException — a type invoke() does not document, next to InvocationTargetException, IllegalArgumentException and IllegalAccessException. Declare the return nullable and treat null as "no hooks" on both call sites. Separately, intercept() documents "@throws IllegalArgumentException if origin is framework internal or Constructor#newInstance". Method.invoke was rejected but Constructor.newInstance was accepted, even though the framework reflects through both and hooking either recurses into hook dispatch.
package-info.java pins the format: "META-INF/xposed/module.prop (Java java.util.Properties format)". The daemon instead split each line on the first '=', which rejects ':' as a separator, treats '!' comment lines as data, and handles neither escapes nor line continuations. A spec-legal module.prop could therefore yield no targetApiVersion, which sends the module down the "NONE" branch and silently makes it unloadable. The manager app already reads the same file with Properties.load, so the two sides could disagree about the same APK. Use Properties on the daemon side too, and match the manager's leading-digit integer parsing so a value like "101.0" cannot be read differently by each. Properties.load rejects malformed \uXXXX escapes where the old parser tolerated them, which would have turned one silent-drop failure mode into another, so the parse is isolated and falls back to empty properties. A legacy module in particular ships no module.prop at all and is selected by assets/xposed_init.
minApiVersion and targetApiVersion are required of a module, but the framework has to survive a third-party module that omits one. extractIntPart dereferenced its argument immediately, so a missing key produced a NullPointerException; the enclosing catch covered only IOException and OutOfMemoryError, so it escaped the InstalledModule constructor, escaped computeIfAbsent in the reload loop, and aborted reloadInstalledModules before it assigned installedModules. The user was left with no modules at all, from any one bad APK. Guard the null, and widen the catch so a single unreadable module is logged and skipped rather than taking the list down with it. Also normalise scope.list the way the daemon normalises its init lists. Entries are matched by exact string, so a comment line or a trailing space silently drops a package from the module's recommended scope. This one is a robustness fix rather than a conformance fix: API 101 specifies no lexical format for scope.list.
RemotePreferences.Editor always writes a "clear" boolean into the diff bundle it sends, and sets it for edit().clear(). Neither end read it. On the daemon side, edit().clear() therefore left every existing key in the database, so a module app clearing a group and writing a fresh set ended up with the union of both. Delete the group before applying the rest of the diff. In hooked processes, VectorRemotePreferences.onUpdate inspected only "delete" and "put", so a cleared group kept serving its old values from the in-process cache and no OnSharedPreferenceChangeListener fired. Clear the cache and report every dropped key as a change. deleteRemotePreferences had no notification path at all, so hooked processes kept a deleted group alive until their process restarted. Now that the receiver understands "clear", send it.
IXposedScopeCallback is the only way a module learns the outcome of requestScope, and two paths left it silent or violated its contract. An empty package list fell through the forEach without dispatching anything, so the module waited for a callback that could never arrive. Nothing was requested, so answer onScopeRequestApproved with an empty list. The generic failure path forwarded Throwable.message, which is routinely null, into onScopeRequestFailed(@nonnull String). The library stub passes it straight to the module's listener, so a module that trusts the annotation crashes inside the binder callback. Fall back to toString().
Chain#proceed() documents "@throws Throwable if any interceptor or the original executable throws an exception". The terminal that BaseInvoker builds for Invoker.Type.Chain called invokeOriginalMethod raw, i.e. through Method.invoke, so the original's exception reached the hooker wrapped in InvocationTargetException. The ordinary hooked-call path already unwraps it, and so does the Origin invoker path, so a hooker's catch clause worked or did not depending on how the call happened to be entered. Route the chain terminal through the same helper.
Owner
Author
|
Updated — Still open:
Rejected, so nobody redoes them: gating on |
DEFAULT is documented as "Follows the global exception mode configured in module.prop. Defaults to PROTECTIVE if not specified", and package-info names the key and its values: exceptionMode (string) [protective| passthrough]. Nothing in the tree read it, so DEFAULT silently meant PROTECTIVE and a module could not opt into passthrough globally. The daemon is the only process that can read module.prop, so it normalises the key to a boolean on PreLoadedApk, which already carries the per-module legacy flag to the injected process. VectorModuleManager turns that into the enum for the module's VectorContext, and the builder resolves DEFAULT at hook registration. Registration, not throw time: the record is stored natively and reaches VectorChain with no way back to the module, and module.prop cannot change for the life of the process. VectorChain is untouched — once records hold a concrete mode its existing PASSTHROUGH test is right. Framework-internal hooks build VectorHookBuilder directly with no module and keep PROTECTIVE through the constructor default. Letting one of those propagate would take the boot path down with it.
package-info says system server is the virtual package "system", and that "android package is still a valid scope target because some of its components declare android:process=":ui"". The remap fired whenever the bound package was android, which is the android:ui process — in system server currentPackageName() is null, so isFirstPackage is false and the name was already android. So the one process the remap could reach was the one that should have kept its real name. The two APIs disagree here, so keep both names. de.robv modules identify system server by lpparam.packageName == "android", and android:ui has always been handed "system" to keep those apart; changing that would make every legacy module run its system-server path a second time. The modern callbacks now get the real package name and the legacy bridge keeps the name it has always seen.
XposedInterface#openRemoteFile documents FileNotFoundException for a file that does not exist or a path that is forbidden. The daemon threw RemoteException for the first and let ensureModuleFilePath's own RemoteException escape for the second. Modules did see FileNotFoundException, but only by accident: RemoteException is not marshallable, so the binder runtime discarded it with an "Uncaught remote exception!" stack trace and the client read a null reply, which VectorContext's elvis turned into the right exception. Every miss cost a full stack trace in the log, and the contract held only as long as nothing threw a type Parcel.writeException does support — an IllegalArgumentException would have reached modules unchanged. Return null deliberately instead, with the AIDL marked @nullable and a one-line warning, so VectorContext produces the documented exception on purpose. Also de-duplicate getScope(): the scope table has a row per (app, user), so a module enabled for several users saw the same package repeatedly. Cross-user filtering is deliberately not done here — the spec does not require it and it would change which packages a module sees.
HSSkyBoy
added a commit
to HSSkyBoy/Vector
that referenced
this pull request
Jul 26, 2026
XposedInterface.API_102 states the behaviour change plainly: "Libxposed modules can not call legacy de.robv.android.xposed APIs." targetApiVersion only ever selected MODERN over LEGACY loading, so nothing stopped a module declaring 102 from using the legacy bridge. The module classloader is where this has to be enforced. Its parent is the framework's own loader, which carries de.robv, so refusing to resolve that package covers reflective lookups against the loader as well as direct references. targetApiVersion is carried on PreLoadedApk to reach it. module.prop parsing follows JingMatrix#794: Java Properties instead of a hand-rolled split("="), tolerating a malformed file rather than making the APK unloadable, and leading-digit integers matching the manager's extractIntPart. That PR is still open and owns this parsing plus an exceptionPassthrough field on the same parcelable, so expect both to conflict here and resolve in its favour. Note that :daemon does not compile yet: bumping libxposed to 102 added getRunningTargets() and hotReloadModule() to IXposedService, which the hot reload work still has to implement.
VectorModuleManager.loadModule is reachable only from XposedInit.loadModules, which only runs from the ActivityThread.attach hook. The late-inject branch is guarded on the activity service already being published, so attach completed long before we arrived and that hook can never fire — yet the branch still called dispatchSystemServerLoaded, which walks activeModules. That set was empty, so onSystemServerStarting went out to nobody and no module ran in a late-injected system server at all. Load the modules there before dispatching, and guard loadModules with a compareAndSet so the ordinary path cannot load a second generation on top. The callback is dispatched even though "system server is ready to start critical services" is no longer true this late; the services are already running. Loading the modules and staying silent would be worse — they would simply never run. Both the late load and the late dispatch are logged as such so the situation is visible in a bug report rather than inferred.
invokeOriginalMethod dispatches through a jmethodID cached once from java.lang.reflect.Method.invoke. When the executable is hooked that id is applied to lsplant's backup Method and is correct. When there is no hook item at all it is applied to the reflected object the caller passed in, and for a constructor that object is a java.lang.reflect.Constructor — a receiver the id does not belong to. Under CheckJNI that aborts; otherwise it indexes the wrong vtable slot. The native comment calls that fallback rare, which holds for the hook callback path but not for invokers: getInvoker(ctor).invoke(obj) on a constructor nobody hooked reaches it every time. Fix it in Kotlin rather than JNI. BaseInvoker already distinguishes the unhooked case, and already has the shorty builder and the non-virtual entry point that invokeSpecial uses, so an unhooked constructor is routed through invokeSpecialMethod and everything else is untouched.
Any call to hookClassInitializer killed the target process outright: JNI DETECTED ERROR IN APPLICATION: attempt to return an instance of java.lang.reflect.Constructor from java.lang.reflect.Method org.matrix.vector.nativebridge.HookBridge.getStaticInitializer(java.lang.Class) ART reflects <clinit> as a java.lang.reflect.Constructor, not a Method, so ToReflectedMethod returns a Constructor while the Kotlin declaration promised a Method, and the JNI return-type check aborts with SIGABRT. Measured on a Pixel 6 running Android 17: signal 6, tombstone, process gone. Widen the declaration to the common supertype and match the descriptor used to register the native method. hook() already takes an Executable, so nothing downstream changes. This only removes the crash. The hook still cannot fire, because resolving <clinit> through GetStaticMethodID initialises the class first - measured, the class goes from uninitialised to initialised across the hookClassInitializer call itself, and the hooker is never entered afterwards. Fixing that needs a non-initialising lookup, which is tracked separately.
The API exists to run code before a class sets up its static state, but resolving <clinit> through GetStaticMethodID runs the static initializer during the lookup, so the hook was always installed too late to fire. Locate the method from ART's layout instead. A class's ArtMethods live in one contiguous array, direct methods first, ordered by dex method id; dex method ids are ordered by name and "<clinit>" sorts before "<init>", so a class that has a static initializer keeps it in the first slot, one stride below the lowest method reflection can see. Reflection over declared members does not initialize the class, so the whole path is free of the side effect. ArtMethod addresses are read from java.lang.reflect.Executable.artMethod rather than through jmethodIDs, because a Java-debuggable process hands out index based ids instead of pointers and the arithmetic would be meaningless there. A class with no static initializer has something unrelated in that slot, and reflecting it walks a bogus pointer and takes the process down. The candidate is therefore checked by two plain word reads first: its declaring class must match its neighbour's, and its access flags must say static constructor. Nothing dereferences it until both hold. Measured on a Pixel 6 running Android 17, on debuggable and non-debuggable targets alike: the class stays uninitialized across the lookup, the hooker runs ahead of the class's own initializer, and a class with no static initializer is refused instead of crashing.
findStaticInitializer replaced its only caller. Leaving it behind would keep a function that initializes any class it is asked about, which is the behaviour the previous commit exists to avoid.
Owner
Author
|
Correction: the bit below about
It now finds the method from ART's layout. Two things cost me time and are worth knowing:
Measured on a Pixel 6 / Android 17, debuggable and non-debuggable: class stays uninitialized across the lookup, the hooker runs before the class's own initializer, classes with no Late-injected system server loads the modules and dispatches, both logged. Skipping the callback would be more literal but modules would never run there. Unhooked constructor fixed in |
JingMatrix
changed the title
The lookup assumed <clinit> was the first ArtMethod of a class, one stride below the lowest member reflection can see. Dex method ids are ordered by name and "<clinit>" does sort before "<init>", but not before everything: '$' and '-' come earlier still. dexdump of a release build shows what that means in practice. Clinit$Colour direct: $values <clinit> <init> tag valueOf values Clinit$Nestmate direct: -$$Nest$smsecret <clinit> <init> secret So every enum, and every class whose private member is reached from a nested class, kept its static initializer in the second slot. The old arithmetic read one slot below the array on those, and missed the method it was looking for. Reflection reports each declared direct and virtual method except <clinit>, and ART lays those out contiguously with copied methods after them, so the addresses are a run of evenly spaced slots with one hole in it. The hole is the method, and it is bracketed by two members that are inside the array, so nothing needs to be assumed about where the array begins. Only a class whose initializer really is first has no hole, and that one candidate is read solely once msync says its page is mapped. The element size no longer comes from the target class either, so a class whose only members are <clinit> and an implicit constructor is no longer refused for showing a single member. That is the shape most worth hooking.
hookClassInitializer says the Chain passed to the hooker reports "a synthetic Method representing the static initializer". ART reflects <clinit> as a Constructor, because it carries ACC_CONSTRUCTOR, so the hooker was given a type it could not cast. A Method is now allocated without running a constructor and given the same Executable state, naming the same ArtMethod under the documented type. Measured on a Pixel 6 running Android 17: executable=java.lang.reflect.Method name=<clinit> declaring=Colour thisObject=null args=[] proceed=null which is the four bullets that section lists.
Invoker#invoke and #invokeSpecial are declared to throw InvocationTargetException and documented against Method#invoke, so the executable's own exception belongs inside one. It arrived that way from an unhooked constructor and raw from everything else, which is the kind of difference a module only finds in production. Chain#proceed is documented the other way round - it throws whatever the executable threw - so the wrapping goes at the public boundary and the chain terminal keeps unwrapping for hookers. The paths that skip the chain get it from the native dispatch, which already separates a target exception from its own argument failures. PASS invoker-wraps-unhooked ITE wrapping IllegalStateException PASS invoker-wraps-hooked ITE wrapping IllegalStateException PASS invoker-wraps-unhooked-ctor ITE wrapping IllegalStateException
The list was immutable but backed by the chain's own array, which the chain rewrites in place, so a hooker holding it across proceed() could watch it change underneath. It is a copy now.
Preferences are stored per Android user, but every registered callback for the group was notified regardless of which user's store had changed. Honouring the clear flag made that visible: one user clearing a group wiped the in-process cache of every other user's hooked processes, for values the daemon had not touched. Registrations now carry the user they belong to.
targetApiVersion was parsed leading-digits-first in the daemon so it could not disagree with the manager's extractIntPart, and scope.list was trimmed in the manager so it could not disagree with the daemon's init lists. Neither is specified anywhere, and taking one side's behaviour as the definition of correct for the other is how two implementations of a spec quietly drift into implementing each other instead. Both go back to reading the file as written.
Properties.load documents IllegalArgumentException for a malformed unicode escape, and the catch around it named only IOException and OutOfMemoryError. Installing a module whose module.prop contains one left the manager's module page completely empty - no list, no tabs, no count - and removing that module brought all seven back. Same failure as the missing-key crash fixed earlier in this branch, reached through a different exception. A module whose module.prop could not be read now reports no API version at all rather than the defaults the field would otherwise keep, since nothing read out of that file before the failure can be trusted either.
The list said nothing about which Xposed API a module was built against, so whether an installed module was current, dated, or asking for more than this build implements could only be found by reading its module.prop. A line under the module's icon now says so - the interface named in small caps, the version beside it carrying the colour: accent targets exactly the API this framework implements error declares a minApiVersion beyond what this framework implements secondary anything else - an older or newer target, and every legacy module A legacy module is labelled Xposed rather than API, because the number it reports is the original Xposed API, counted on a scale of its own that has nothing to say about the one this framework implements. minApiVersion and targetApiVersion are both required of a module, so a module that declared neither, or only one of the two, reports a question mark rather than putting a number on screen it never claimed. The colour is never the only carrier: the line sets a content description and a tooltip spelling the situation out. staticScope is honoured on the scope picker instead of announced in the module list. A module that sets it says its scope list is the whole of it, so that screen now offers only the apps it claims and closes the list with a centred note. Both the daemon and the manager had parsed that key all along without either of them acting on it anywhere.
Restricting the picker to the apps a module claims hid any app that had been enabled before the module declared staticScope - the daemon still had it in scope and still loaded the module there, but the row that could switch it off was gone. Such an app is listed again, so the restriction only governs what can be added.
Owner
Author
|
This was the hard one, and I would welcome another pair of eyes on it. It was worse than "the hook never fires" — every call aborted the process, because ART reflects So it has to be found without asking the runtime to resolve it. ART keeps a class's methods in one contiguous array and reflection shows all of them except My first attempt assumed the hole was always at the start, since
Checked on a Pixel 6 / Android 17, debuggable and release: the class stays uninitialised through the lookup, the hooker runs before the class's own initialiser, and Where it is thin: one device and one Android version. And a class whose If anyone knows a better way to get at the method array's bounds, I would take it. |
module.prop's staticScope is documented as "the module scope is fixed and users
should not apply the module on apps outside the scope list". Restricting what
the manager offers is not enough to make that true: the socket CLI, a backup
restore and a module's own requestScope all write the scope table without ever
passing through the manager, and the daemon did not even parse the key.
It reads staticScope and scope.list now, and every writer funnels through
setModuleScope, so refusing there covers all of them. The CLI and requestScope
check first as well, so the answer can name the packages instead of failing
silently. Removing scope is always allowed - whatever is on has to be possible
to turn off.
Rows that predate the module claiming a fixed scope are dropped when the cache
is rebuilt, which is what makes the scope fixed rather than merely awkward to
extend.
Measured against a module declaring scope.list = org.matrix.hrtarget:
scope add ... com.termux Error: ... com.termux cannot be added.
It claims: org.matrix.hrtarget.
scope set ... com.termux same
scope add ... hrtarget accepted
com.termux, added before the module fixed its scope, gone after a reboot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes against libxposed API 101, the version master vendors. Nothing to do with #757 — no submodule bumps, no new API.
All of it was found and checked with small test modules that assert the documented behaviour and log pass/fail, on a Pixel 6 running Android 17, debuggable and release targets.
hookClassInitializernever worked at all. It crashed the process, and once that was fixed the hook still could not fire. It works now — there is a separate comment below, that is the part worth reading properly.The chain and the invokers. Two protective hooks could resurrect an exception the chain had already swallowed.
ExceptionMode.DEFAULTignoredexceptionMode, so passthrough was unreachable.getInvoker()threw NPE on an unhooked method,Constructor.newInstancewas hookable when the doc names it as the one you cannot hook, and an unhooked constructor dispatchedMethod.invoke's id against itself.Invoker.invokewrapped the target's exception only for unhooked constructors — it isMethod#invokesemantics on every path now, whileChain.proceedstill gives hookers the raw one.getArgs()was mutable.Elsewhere. A late-injected system server loaded no modern modules but dispatched
onSystemServerStartinganyway.android:uiwas reported to modules as thesystempackage.edit().clear()did nothing, and fixing it turned up that preference updates reached every Android user's hooked processes rather than the one that made the change. Empty scope requests never called back,openRemoteFilethrewRemoteExceptionwhere the doc promisesFileNotFoundException,getScope()repeated a package once per user, andmodule.propwas not parsed asPropertiesdespite the spec naming it.Manager. One bad
module.propcould blank the entire module list — twice over, through a missing key and through a malformed escape. The list now shows which API each module targets.staticScopeis enforced instead of ignored: the picker offers only what a module claims, and the daemon refuses the rest whether it comes from the socket CLI, a backup restore or the module's ownrequestScope, and drops stale rows when it starts.Two things were tried and then reverted: teaching the daemon to parse
targetApiVersionlike the manager, and the manager to trimscope.listlike the daemon. Neither is in the spec, and that road ends with two implementations of a spec implementing each other instead.