diff --git a/Cargo.lock b/Cargo.lock index cd6f957..57645f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1226,6 +1226,7 @@ dependencies = [ "android_logger", "dr-sync", "dr-ui", + "jni 0.21.1", "log", "slint", ] diff --git a/apps/darkroom-android/Cargo.toml b/apps/darkroom-android/Cargo.toml index bfcbc7b..5699149 100644 --- a/apps/darkroom-android/Cargo.toml +++ b/apps/darkroom-android/Cargo.toml @@ -26,6 +26,21 @@ slint.workspace = true log.workspace = true android_logger = "0.15" +# The launch Intent and the share sheet are Java-only surfaces — see `intents` +# — and JNI is the only way to reach them. +# +# Target-gated because the crate still has to compile on the host: it is a +# workspace member, `cargo test --workspace` builds it, and the manifest tests +# in `lib.rs` are the one part of it that runs there. +# +# 0.21 rather than the 0.22 that android-activity 0.6 uses. Both are already in +# the lock — Slint's Android backend depends on two major versions of +# android-activity and pulls both — so this adds nothing to the build either +# way, and every object here comes from a raw pointer rather than from a type +# android-activity handed over, so the two never have to agree. +[target.'cfg(target_os = "android")'.dependencies] +jni = "0.21" + [features] # Mirrors darkroom-desktop: the CPU readback path is gone since S1 landed # zero-copy. It mattered more here than on desktop — the same wrong path with diff --git a/apps/darkroom-android/android/AndroidManifest.xml b/apps/darkroom-android/android/AndroidManifest.xml index 97a9819..2395a5a 100644 --- a/apps/darkroom-android/android/AndroidManifest.xml +++ b/apps/darkroom-android/android/AndroidManifest.xml @@ -7,6 +7,12 @@ here is a distribution manifest yet. Only network access is declared: file access needs no manifest permission because the library grid reads through SAF, which grants per-tree at runtime (ARCH §6.9). + + Minimal is not the same as empty, and the entries below that are not the + activity are the difference. A manifest is the only place a component can be + declared: an intent filter is how the system learns this app is worth + offering for a photograph, and a provider is how it learns the class exists + at all. Neither can be moved into code (FR-PLAT-AND-6). --> @@ -51,10 +57,30 @@ + is how it learns which library to load, and must match [lib].name. + + `singleTask` because a second instance of this activity is not + survivable. The intent filters below mean another app can now + launch it while it is already running, and under the default + launch mode that starts a *second* NativeActivity — in the + caller's task, in this same process, calling android_main again. + Two Slint backends and two wgpu devices in one process is not a + degraded experience, it is a failed second launch on top of a + working first one. + + What it costs, stated plainly: a share that arrives while DarkRoom + is already running brings it forward without opening the image. + The Intent goes to `onNewIntent`, and android-activity's event + stream has no variant for it (MainEvent in 0.6 stops at Destroy), + so nothing native ever sees it. Reading it would mean a Java + Activity subclass forwarding it across JNI — the same shape of + change FR-PLAT-AND-5 declined for onTrimMemory, and for the same + reason. Launched from cold, which is the ordinary case for "open + this photograph", the Intent is on getIntent() and is read. --> @@ -66,6 +92,80 @@ + + + + + + + + + + + + + + + + + + + diff --git a/apps/darkroom-android/android/java/paris/tourolle/darkroom/ExportProvider.java b/apps/darkroom-android/android/java/paris/tourolle/darkroom/ExportProvider.java new file mode 100644 index 0000000..221666f --- /dev/null +++ b/apps/darkroom-android/android/java/paris/tourolle/darkroom/ExportProvider.java @@ -0,0 +1,260 @@ +package paris.tourolle.darkroom; + +import android.content.ContentProvider; +import android.content.ContentValues; +import android.content.Context; +import android.database.Cursor; +import android.database.MatrixCursor; +import android.net.Uri; +import android.os.ParcelFileDescriptor; +import android.provider.OpenableColumns; +import android.util.Log; +import android.webkit.MimeTypeMap; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.util.List; +import java.util.Locale; + +/** + * Hands an exported file to another app, and hands out nothing else. + * + *

FR-PLAT-AND-6's outbound half. Android has refused {@code file://} URIs + * between apps since API 24 — passing one raises {@code FileUriExposedException} + * in the *sending* process — so the only way to give a photo to the share sheet + * is a {@code content://} URI backed by a provider, plus a per-Intent read + * grant that expires with the task that received it. + * + *

Why this is not AndroidX's FileProvider

+ * + *

Because AndroidX is a Maven artefact and this build has no Gradle and no + * dependency resolver (see docker/android/README.md). Pulling in the one class + * would mean adopting the whole mechanism that fetches it. What + * {@code FileProvider} does is a hundred lines — map a request path onto a + * directory, refuse anything outside it, answer the two columns the share sheet + * reads — and those lines are below. The configuration it takes as an XML + * {@code } resource is a constant here instead, because there is + * exactly one directory worth serving and a second place to state it is a + * second place for it to be wrong. + * + *

The one directory

+ * + *

{@code getFilesDir()}, which is the same directory the Rust side calls + * {@code internal_data_path} and passes to {@code dr_sync::account::set_data_dir} + * — {@code ANativeActivity.internalDataPath} and {@code Context.getFilesDir()} + * are the same path. Everything the app writes for itself, the export outbox + * included, is under it. Nothing else is reachable: a request is resolved + * against the real filesystem with {@link File#getCanonicalFile()} and then + * checked to be *inside* that root, so {@code ../} and a symlink planted in the + * outbox are refused by the same test. Serving a path the caller composed, + * unchecked, would turn a share button into a reader for every file this app + * can see, which on Android includes credentials and the whole catalog. + * + *

{@code android:exported="false"} in the manifest is the outer half of the + * same rule: no app can address this provider at all except through a URI this + * app handed it with a read grant attached. + */ +public final class ExportProvider extends ContentProvider { + private static final String TAG = "DarkRoom"; + + /** + * Must equal {@code android:authorities} in AndroidManifest.xml. + * + *

A mismatch is not a build error and not a runtime error here: it is a + * {@code SecurityException} in whichever app opened the share sheet, naming + * an authority that does not exist. A test in {@code lib.rs} asserts the + * two strings are the same for that reason. + */ + public static final String AUTHORITY = "paris.tourolle.darkroom.exports"; + + /** Nothing to set up; the root is resolved per request against the context. */ + @Override + public boolean onCreate() { + return true; + } + + /** + * The {@code content://} URI for a file, or null if it is not one this + * provider may serve. + * + *

Returning null rather than an unusable URI keeps the refusal at the + * point where the path is known. A URI for a file outside the root would be + * rejected later by {@link #openFile}, in the *receiving* app's stack trace, + * where nothing says which of our files was asked for. + */ + public static Uri uriFor(Context context, File file) { + try { + File root = root(context); + File target = file.getCanonicalFile(); + String relative = within(root, target); + if (relative == null) { + Log.w(TAG, "not shareable, outside " + root + ": " + target); + return null; + } + // Built segment by segment rather than with a composed path + // string: appendPath percent-encodes, and getPathSegments below + // decodes symmetrically. A file called "Rue d'Alésia.jpg" survives + // the round trip only because both halves agree. + Uri.Builder builder = new Uri.Builder().scheme("content").authority(AUTHORITY); + for (String segment : relative.split("/")) { + if (!segment.isEmpty()) { + builder.appendPath(segment); + } + } + return builder.build(); + } catch (IOException e) { + Log.w(TAG, "cannot resolve " + file + " for sharing: " + e); + return null; + } + } + + /** + * The two columns a share target actually reads. + * + *

Without {@code _display_name} the receiving app shows the URI's last + * segment, and without {@code _size} a mail client cannot tell whether the + * attachment fits before it starts reading. Both are optional in the sense + * that the transfer still works; both are the difference between "DSC_4471 + * final.jpg, 8.2 MB" and an unnamed blob. + */ + @Override + public Cursor query(Uri uri, String[] projection, String selection, + String[] selectionArgs, String sortOrder) { + File file = resolve(uri); + if (file == null) { + return null; + } + String[] columns = projection != null + ? projection + : new String[] {OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE}; + MatrixCursor cursor = new MatrixCursor(columns, 1); + MatrixCursor.RowBuilder row = cursor.newRow(); + for (String column : columns) { + if (OpenableColumns.DISPLAY_NAME.equals(column)) { + row.add(file.getName()); + } else if (OpenableColumns.SIZE.equals(column)) { + row.add(file.length()); + } else { + // A column we do not have. Null rather than omitted: a cursor + // whose row is shorter than its projection throws in the + // caller, which is a crash in someone else's app. + row.add(null); + } + } + return cursor; + } + + /** + * From the extension, because that is all there is. + * + *

The type decides which apps the chooser offers, so guessing wrong + * narrows the sheet rather than breaking the transfer. Exports are JPEG, + * PNG or TIFF and {@code MimeTypeMap} knows all three. + */ + @Override + public String getType(Uri uri) { + File file = resolve(uri); + if (file == null) { + return null; + } + String name = file.getName(); + int dot = name.lastIndexOf('.'); + if (dot >= 0 && dot < name.length() - 1) { + String extension = name.substring(dot + 1).toLowerCase(Locale.ROOT); + String type = MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension); + if (type != null) { + return type; + } + } + return "application/octet-stream"; + } + + /** + * Read-only, always. + * + *

A write mode is refused rather than quietly downgraded: a caller that + * asked for "rw" intends to save something back, and letting it open the + * file read-only would fail at its first write with an error about a + * descriptor rather than about permission. Nothing this app shares is meant + * to be edited in place by the app it was shared with. + */ + @Override + public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException { + if (!"r".equals(mode)) { + throw new SecurityException("this provider is read-only, asked for '" + mode + "'"); + } + File file = resolve(uri); + if (file == null) { + throw new FileNotFoundException("no such export: " + uri); + } + return ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_READ_ONLY); + } + + @Override + public Uri insert(Uri uri, ContentValues values) { + throw new UnsupportedOperationException("exports are written by the app, not through it"); + } + + @Override + public int update(Uri uri, ContentValues values, String selection, String[] selectionArgs) { + throw new UnsupportedOperationException("exports are written by the app, not through it"); + } + + @Override + public int delete(Uri uri, String selection, String[] selectionArgs) { + throw new UnsupportedOperationException("exports are deleted by the app, not through it"); + } + + /** The served root, resolved through the filesystem so the check below is real. */ + private static File root(Context context) throws IOException { + return context.getFilesDir().getCanonicalFile(); + } + + /** The file a request names, or null if it names anything else. */ + private File resolve(Uri uri) { + Context context = getContext(); + if (context == null) { + return null; + } + List segments = uri.getPathSegments(); + if (segments.isEmpty()) { + return null; + } + try { + File root = root(context); + File candidate = root; + for (String segment : segments) { + candidate = new File(candidate, segment); + } + candidate = candidate.getCanonicalFile(); + if (within(root, candidate) == null || !candidate.isFile()) { + Log.w(TAG, "refused " + uri); + return null; + } + return candidate; + } catch (IOException e) { + Log.w(TAG, "refused " + uri + ": " + e); + return null; + } + } + + /** + * {@code target}'s path relative to {@code root}, or null if it is not + * under it. + * + *

Both sides are canonical by the time they get here, which is what + * makes one string comparison enough for {@code ../} and for a symlink + * alike. The trailing separator matters: without it a sibling directory + * whose name merely starts with the root's — {@code /data/.../files.old} — + * passes. + */ + private static String within(File root, File target) { + String rootPath = root.getPath() + File.separator; + String targetPath = target.getPath(); + if (!targetPath.startsWith(rootPath)) { + return null; + } + return targetPath.substring(rootPath.length()); + } +} diff --git a/apps/darkroom-android/android/java/paris/tourolle/darkroom/Intents.java b/apps/darkroom-android/android/java/paris/tourolle/darkroom/Intents.java new file mode 100644 index 0000000..0602a7c --- /dev/null +++ b/apps/darkroom-android/android/java/paris/tourolle/darkroom/Intents.java @@ -0,0 +1,320 @@ +package paris.tourolle.darkroom; + +import android.app.Activity; +import android.content.ActivityNotFoundException; +import android.content.ContentResolver; +import android.content.Context; +import android.content.Intent; +import android.database.Cursor; +import android.net.Uri; +import android.provider.OpenableColumns; +import android.util.Log; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.ArrayList; +import java.util.List; + +/** + * The two directions of FR-PLAT-AND-6: what the app was opened *with*, and + * handing a finished export to somebody else. + * + *

Why this is Java and not JNI in lib.rs

+ * + *

Every call below is reachable over JNI, and doing it that way would be + * roughly forty {@code call_method} invocations with their signatures written + * out as strings — each one a name Java checks at run time and nothing checks + * at build time. The Rust side would then hold the exact logic that is here, + * expressed less clearly, and a typo in {@code "()Landroid/content/Intent;"} + * would surface on a device as a {@code NoSuchMethodError} rather than at the + * compiler. So the platform work stays on the platform's side and the JNI + * surface is two calls, both taking and returning strings. + * + *

The class is only reachable because the APK now compiles Java at all; see + * docker/android/assemble-apk.sh. + */ +public final class Intents { + private static final String TAG = "DarkRoom"; + + /** + * Where incoming images are copied, under {@code getCacheDir()}. + * + *

The cache and not the data directory, deliberately: these are copies + * of somebody else's file, the app has no claim on them once the session + * ends, and the cache is the one place Android may reclaim under storage + * pressure without the user being asked. Putting them in the data + * directory would grow the app's footprint by a RAW file per share, for + * ever, with nothing that ever deletes them. + */ + private static final String INBOX = "incoming"; + + private Intents() { + } + + /** + * The images this launch was asked to open, as paths the decoder can read. + * + *

Empty for an ordinary launch from the launcher, which is the common + * case and not a failure. + * + *

Why the bytes are copied

+ * + *

A share arrives as a {@code content://} URI, which is a handle into + * another app's provider and not a path — there is no filename behind it to + * open, and the grant that makes it readable belongs to this task and dies + * with it. DarkRoom's decoders take paths (ARCH §6.9 is the note that + * Android has no paths to give), so the choice is to copy or to teach the + * whole read path about URIs, and the second is FR-PLAT-AND-1's SAF + * connector, which is not built. + * + *

So it is a copy, and the cost is honest: a 60 MB raw file is written + * once, to the cache, before the viewer opens. It is bounded by the share + * being a deliberate act — a person picked these files — rather than by + * anything this code does. + * + *

The inbox is emptied first. Without that, every share ever received + * accumulates until the platform decides the cache is too large, and the + * files are indistinguishable from each other by then. + */ + public static String[] receive(Activity activity) { + List uris = incoming(activity.getIntent()); + if (uris.isEmpty()) { + return new String[0]; + } + + File inbox = new File(activity.getCacheDir(), INBOX); + empty(inbox); + if (!inbox.mkdirs() && !inbox.isDirectory()) { + Log.e(TAG, "cannot create " + inbox + "; the launch intent is dropped"); + return new String[0]; + } + + List paths = new ArrayList(); + for (Uri uri : uris) { + String path = localise(activity, uri, inbox, paths.size()); + if (path != null) { + paths.add(path); + } + } + Log.i(TAG, "launch intent carried " + paths.size() + " of " + uris.size() + " image(s)"); + return paths.toArray(new String[0]); + } + + /** + * Offer a file this app produced to whatever else is installed. + * + *

Returns false when there is nothing to offer it to, or when the file + * is not one {@link ExportProvider} may serve — both of which the caller + * has to be able to say out loud, because from the user's side a share + * button that does nothing is indistinguishable from one that failed. + * + *

{@code FLAG_GRANT_READ_URI_PERMISSION} is the whole security model: + * the provider is not exported, so the receiving app can reach this one + * file, for as long as its task lives, and nothing else ever. + */ + public static boolean share(Activity activity, String path, String mimeType) { + Uri uri = ExportProvider.uriFor(activity, new File(path)); + if (uri == null) { + return false; + } + + Intent send = new Intent(Intent.ACTION_SEND); + send.setType(mimeType != null && !mimeType.isEmpty() ? mimeType : "image/*"); + send.putExtra(Intent.EXTRA_STREAM, uri); + send.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); + + // Always a chooser, never a direct start. Android's "remembered + // default" for ACTION_SEND is a per-user setting this app has no + // business consuming: the app a photograph should go to differs every + // time, and the one time it does not, the sheet is one extra tap. + Intent chooser = Intent.createChooser(send, null); + try { + activity.startActivity(chooser); + return true; + } catch (ActivityNotFoundException e) { + Log.w(TAG, "nothing installed accepts " + mimeType + ": " + e); + return false; + } + } + + /** + * The URIs an Intent carries, by the action that carried them. + * + *

Only the actions the manifest registers for. An action we did not + * declare cannot arrive, so handling one here would be code that reads as + * support for something the launcher will never offer. + */ + @SuppressWarnings("deprecation") + private static List incoming(Intent intent) { + List uris = new ArrayList(); + if (intent == null) { + return uris; + } + String action = intent.getAction(); + if (Intent.ACTION_VIEW.equals(action)) { + add(uris, intent.getData()); + } else if (Intent.ACTION_SEND.equals(action)) { + // The typed getParcelableExtra(String, Class) overload is API 33, + // and minSdk is 28. The deprecated form is the only one that exists + // on every device this APK installs on. + add(uris, (Uri) intent.getParcelableExtra(Intent.EXTRA_STREAM)); + } else if (Intent.ACTION_SEND_MULTIPLE.equals(action)) { + ArrayList many = intent.getParcelableArrayListExtra(Intent.EXTRA_STREAM); + if (many != null) { + for (Uri uri : many) { + add(uris, uri); + } + } + } + return uris; + } + + private static void add(List uris, Uri uri) { + if (uri != null) { + uris.add(uri); + } + } + + /** A URI as a readable path, copying it into the inbox if it is not one already. */ + private static String localise(Context context, Uri uri, File inbox, int index) { + // A file:// URI is already a path, and copying it would double a raw + // file on disk to no end. Rare — the platform has refused file:// URIs + // between apps since API 24 — but it is what a shell `am start -d + // file:///sdcard/…` produces, which is how this path gets tested + // without a second app installed. + if (ContentResolver.SCHEME_FILE.equals(uri.getScheme())) { + String path = uri.getPath(); + if (path != null && new File(path).canRead()) { + return path; + } + Log.w(TAG, "cannot read " + uri); + return null; + } + + File dest = new File(inbox, unique(inbox, displayName(context, uri), index)); + InputStream in = null; + OutputStream out = null; + try { + in = context.getContentResolver().openInputStream(uri); + if (in == null) { + Log.w(TAG, "no stream behind " + uri); + return null; + } + out = new FileOutputStream(dest); + byte[] buffer = new byte[64 * 1024]; + int read; + while ((read = in.read(buffer)) > 0) { + out.write(buffer, 0, read); + } + out.flush(); + return dest.getAbsolutePath(); + } catch (IOException e) { + Log.w(TAG, "cannot copy " + uri + ": " + e); + // The partial copy is removed rather than left: it has the name and + // the extension of a photograph and none of the bytes, and the + // decoder would report it as a corrupt file rather than a failed + // transfer. + dest.delete(); + return null; + } catch (SecurityException e) { + // The grant on a shared URI dies with the task that received it. + // A process resumed from a saved state can find itself holding a + // URI it may no longer read (FR-PLAT-AND-3), and that is a lost + // permission rather than a broken file. + Log.w(TAG, "no longer permitted to read " + uri + ": " + e); + dest.delete(); + return null; + } finally { + close(in); + close(out); + } + } + + /** + * What the sending app calls the file, reduced to something safe to write. + * + *

The name is chosen by another application and lands in a path this one + * composes, so it is filtered rather than trusted: a name containing a + * separator would place the copy outside the inbox, and one beginning with + * a dot would hide it from everything that lists the directory. What + * survives is the part a photographer recognises — {@code DSC_4471.NEF} — + * which is the only reason to use the sender's name at all. + */ + private static String displayName(Context context, Uri uri) { + String name = null; + Cursor cursor = null; + try { + cursor = context.getContentResolver().query( + uri, new String[] {OpenableColumns.DISPLAY_NAME}, null, null, null); + if (cursor != null && cursor.moveToFirst() && !cursor.isNull(0)) { + name = cursor.getString(0); + } + } catch (Exception e) { + // Providers are other people's code and any of them may throw. + // A name is a convenience; failing the whole open over it is not. + Log.d(TAG, "no display name for " + uri + ": " + e); + } finally { + if (cursor != null) { + cursor.close(); + } + } + if (name == null) { + name = uri.getLastPathSegment(); + } + if (name == null) { + return "shared"; + } + StringBuilder safe = new StringBuilder(name.length()); + for (int i = 0; i < name.length(); i++) { + char c = name.charAt(i); + boolean ok = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') + || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_'; + safe.append(ok ? c : '_'); + } + while (safe.length() > 0 && safe.charAt(0) == '.') { + safe.deleteCharAt(0); + } + return safe.length() > 0 ? safe.toString() : "shared"; + } + + /** + * A name nothing in the inbox has yet. + * + *

A multi-image share of a burst arrives as several files a camera named + * the same thing in different folders, and the second one silently + * overwriting the first would show the user one photograph where they + * picked four. + */ + private static String unique(File inbox, String name, int index) { + if (!new File(inbox, name).exists()) { + return name; + } + return index + "-" + name; + } + + private static void close(java.io.Closeable stream) { + if (stream != null) { + try { + stream.close(); + } catch (IOException e) { + Log.d(TAG, "close failed: " + e); + } + } + } + + /** Delete the inbox's contents, one level deep, which is all it ever has. */ + private static void empty(File inbox) { + File[] stale = inbox.listFiles(); + if (stale == null) { + return; + } + for (File file : stale) { + if (!file.delete()) { + Log.d(TAG, "could not remove stale " + file); + } + } + } +} diff --git a/apps/darkroom-android/src/intents.rs b/apps/darkroom-android/src/intents.rs new file mode 100644 index 0000000..24c55a9 --- /dev/null +++ b/apps/darkroom-android/src/intents.rs @@ -0,0 +1,192 @@ +//! What the app was launched with, and handing a finished export back out. +//! +//! FR-PLAT-AND-6's Rust side, which is deliberately the thin side. Both +//! directions are implemented in `android/java/paris/tourolle/darkroom/` and +//! everything here is the two calls that reach them; `Intents.java` carries the +//! reasoning for the split. The short version is that a JNI method signature is +//! a string Java resolves at run time and nothing checks at build time, so +//! forty of them is forty ways for a rename to become a `NoSuchMethodError` on +//! somebody's tablet. Two is two. +//! +//! # Nothing here fails loudly +//! +//! A class the loader cannot see, a pending Java exception, a shared URI whose +//! grant died with the task that received it: each ends as a log line and an +//! empty result. This runs on the way to [`dr_ui::run`], before a window +//! exists, and the alternative to opening with an empty browsing list is not +//! opening at all. + +use std::path::{Path, PathBuf}; + +use jni::errors::Result as JniResult; +use jni::objects::{JClass, JObject, JObjectArray, JString, JValue}; +use jni::{JNIEnv, JavaVM}; + +/// The class both directions live in, named the way `loadClass` wants it — +/// dots, not slashes. `find_class` takes the other form, and this code calls +/// neither by accident; see [`load_class`]. +const INTENTS: &str = "paris.tourolle.darkroom.Intents"; + +/// The images this launch was asked to open, already local and readable. +/// +/// Empty for an ordinary launch from the launcher, which is the common case +/// and not a failure. What comes back is passed to `dr_ui::run` exactly as +/// command-line paths are on the desktop, so a shared photograph becomes the +/// browsing list and `startup_action` shows it rather than the launch screen. +pub fn launch_images(app: &slint::android::AndroidApp) -> Vec { + with_activity(app, "reading the launch intent", |env, activity| { + let class = load_class(env, activity, INTENTS)?; + let returned = env + .call_static_method( + &class, + "receive", + "(Landroid/app/Activity;)[Ljava/lang/String;", + &[JValue::Object(activity)], + )? + .l()?; + + let array = JObjectArray::from(returned); + let count = env.get_array_length(&array)?; + let mut paths = Vec::with_capacity(count as usize); + for i in 0..count { + let element = env.get_object_array_element(&array, i)?; + let text: String = env.get_string(&JString::from(element))?.into(); + paths.push(PathBuf::from(text)); + } + Ok(paths) + }) + .unwrap_or_default() +} + +/// Offer a file this app produced to whatever else is installed. +/// +/// `false` means the sheet did not open — the file is not under the directory +/// [`ExportProvider`] serves, or nothing installed accepts the type. Both are +/// answers a caller has to be able to give the user, because a share control +/// that silently does nothing is indistinguishable from one that failed. +/// +/// **This half has no caller yet, and that is the honest state of it.** The +/// provider, the URI grant and the chooser are all here and are what +/// FR-PLAT-AND-6 asks for; what is missing is a share control in the interface, +/// which lives in `ui/dr-ui` and needs one thing this signature shows: an +/// `AndroidApp` to call through. Wiring it means keeping a clone of the app — +/// it is `Clone` and cheap — somewhere `ui/` can reach, which is a change to +/// how the platform entry point talks to the interface rather than a change +/// here. Until that exists this function is reachable and untested, and it is +/// deliberately not tagged as covering the requirement. +/// +/// `mime` decides which applications the chooser offers; the empty string +/// falls back to `image/*` on the Java side. +pub fn share(app: &slint::android::AndroidApp, file: &Path, mime: &str) -> bool { + with_activity(app, "opening the share sheet", |env, activity| { + let class = load_class(env, activity, INTENTS)?; + let path = env.new_string(file.to_string_lossy().as_ref())?; + let mime = env.new_string(mime)?; + env.call_static_method( + &class, + "share", + "(Landroid/app/Activity;Ljava/lang/String;Ljava/lang/String;)Z", + &[ + JValue::Object(activity), + JValue::Object(&path), + JValue::Object(&mime), + ], + )? + .z() + }) + .unwrap_or(false) +} + +/// Attach to the JVM, borrow the activity, and run `body` against both. +/// +/// Shared by the two entry points because the three steps before the +/// interesting one are identical and each has its own way of failing. `body` +/// returning `Err` is reported here, once, in the one place that can also clear +/// a pending Java exception — see [`report`]. +fn with_activity( + app: &slint::android::AndroidApp, + doing: &str, + body: impl FnOnce(&mut JNIEnv, &JObject) -> JniResult, +) -> Option { + let vm = match unsafe { JavaVM::from_raw(app.vm_as_ptr().cast()) } { + Ok(vm) => vm, + Err(e) => { + log::error!("no JVM handle, so {doing} is skipped: {e}"); + return None; + } + }; + // Cheap when the thread is already attached, which it is: the glue + // attached it before it called `android_main`. The guard exists for the + // case where it is not, and costs a lookup where it is. + let mut env = match vm.attach_current_thread() { + Ok(env) => env, + Err(e) => { + log::error!("cannot attach to the JVM, so {doing} is skipped: {e}"); + return None; + } + }; + + // SAFETY: `activity_as_ptr` documents this as an unowned JNI *global* + // reference to the Activity, valid for as long as the `AndroidApp` it came + // from. `JObject` in jni 0.21 is a plain wrapper with no `Drop`, so + // borrowing it here cannot delete a reference this code does not own — the + // one way to get this wrong is `AutoLocal` or a `GlobalRef`, both of which + // would free it out from under android-activity. + let activity = unsafe { JObject::from_raw(app.activity_as_ptr().cast()) }; + + match body(&mut env, &activity) { + Ok(value) => Some(value), + Err(e) => { + report(&mut env, doing, &e); + None + } + } +} + +/// Look an app class up through the *activity's* class loader. +/// +/// `find_class` is the obvious call and the wrong one. JNI resolves a class +/// against the loader belonging to the Java frame beneath the call, and on this +/// thread there is no such frame: `android_main` runs on a thread the native +/// glue created and attached itself, so the loader in scope is the system one. +/// It knows every class in the platform and nothing at all from this APK, and +/// says so as a `ClassNotFoundException` naming a class that is plainly in the +/// dex — which reads as a broken build rather than as the wrong loader. +/// +/// The activity is a Java object, so its loader is the app's. +fn load_class<'local>( + env: &mut JNIEnv<'local>, + activity: &JObject, + name: &str, +) -> JniResult> { + let loader = env + .call_method(activity, "getClassLoader", "()Ljava/lang/ClassLoader;", &[])? + .l()?; + let name = env.new_string(name)?; + let class = env + .call_method( + &loader, + "loadClass", + "(Ljava/lang/String;)Ljava/lang/Class;", + &[JValue::Object(&name)], + )? + .l()?; + Ok(JClass::from(class)) +} + +/// Log a JNI failure, and clear the exception behind it if there is one. +/// +/// The clearing is not tidiness. A Java exception raised through JNI stays +/// *pending* on the thread, and the next JNI call made while one is pending +/// aborts the process — so a swallowed exception here would come back as a +/// crash somewhere unrelated, most likely inside Slint. `exception_describe` +/// first, because the trace it prints to logcat is the only place the Java +/// class and line survive; `jni::errors::Error::JavaException` on its own says +/// neither. +fn report(env: &mut JNIEnv, doing: &str, e: &jni::errors::Error) { + log::error!("{doing} failed: {e}"); + if let Ok(true) = env.exception_check() { + let _ = env.exception_describe(); + let _ = env.exception_clear(); + } +} diff --git a/apps/darkroom-android/src/lib.rs b/apps/darkroom-android/src/lib.rs index 6eda761..f8c417f 100644 --- a/apps/darkroom-android/src/lib.rs +++ b/apps/darkroom-android/src/lib.rs @@ -8,6 +8,17 @@ //! browsing list and the library grid is the only way in. //! * Logging goes to logcat. `env_logger` writes to stderr, which Android //! discards. +//! +//! The first of those has one exception, and it is the launch `Intent`: a +//! gallery, a file manager or the share sheet can name images to open, and +//! those arrive as URIs on an `Intent` rather than as words on a command line. +//! [`intents`] turns them into paths, and from there they are the same list +//! the desktop builds from `argv` (FR-PLAT-AND-6). + +// The whole module is JNI against classes that exist only in the APK, so it +// is gated with everything else that cannot compile off-device. +#[cfg(target_os = "android")] +mod intents; // `slint::android` exists only when compiling for Android, so the whole entry // point is gated on the target rather than on a feature. Without this the @@ -51,6 +62,12 @@ fn android_main(app: slint::android::AndroidApp) { // After the data dir and before anything asks whether a model is present. install_bundled_face_models(&app); + // Before `init_with_event_listener`, which takes `app` by value and is the + // last moment anything can ask the activity a question. Not an ordering + // preference — after that line there is no `app` left to read the Intent + // through. + let opened_with = intents::launch_images(&app); + // TRACES: FR-PLAT-AND-5 // The listener is the whole reason this is not the one-line // `slint::android::init(app)`. Slint owns the event loop on Android, so @@ -96,11 +113,15 @@ fn android_main(app: slint::android::AndroidApp) { return; } - // Empty rather than the desktop's argv: see the module note above. + // The launch Intent's images, where there were any, standing in for the + // desktop's argv — `launch::startup_action` treats a non-empty list as + // "the user asked for these specifically", which is exactly what a share + // or a tap in a gallery is. Empty for an ordinary launch, and the library + // opens as before. // // Returning from `android_main` ends the process, so a failure here is // logged rather than propagated — there is no shell to show `Err` to. - if let Err(e) = dr_ui::run(Vec::new()) { + if let Err(e) = dr_ui::run(opened_with) { log::error!("DarkRoom exited with error: {e:#}"); } } @@ -179,3 +200,166 @@ fn install_bundled_face_models(app: &slint::android::AndroidApp) { } } } + +/// TRACES: FR-PLAT-AND-6 +/// The declarations that make this app a receiver, held to on the host. +/// +/// Everything FR-PLAT-AND-6 does on a device is unreachable from `cargo test`: +/// there is no `Intent` off-device and no `ContentProvider` to instantiate. But +/// the requirement is not only behaviour — half of it is *declaration*, and a +/// declaration can be wrong in ways that compile perfectly and fail silently. +/// An intent filter that is deleted takes the app out of every gallery's "open +/// with" menu with nothing to notice; an authority that stops matching the +/// class it names raises a `SecurityException` in whichever other app opened +/// the share sheet, which is the last place anybody would look for it. +/// +/// The manifest is read by aapt2 and the Java by javac, so a Rust build sees +/// neither. `include_str!` is what puts them where a test can reach them, and +/// this is the only place in the workspace that does. +#[cfg(test)] +mod tests { + /// The manifest with its comments removed and its whitespace flattened, so + /// a match is about the declaration and not about how it is indented. + fn manifest() -> String { + let xml = include_str!("../android/AndroidManifest.xml"); + let mut out = String::with_capacity(xml.len()); + let mut rest = xml; + // Comments first, and not by regex over the whole file: several of them + // quote the very attribute names the assertions below look for, so a + // test that read them would pass on the strength of the prose + // explaining an entry that had been deleted. + while let Some(start) = rest.find("") { + Some(end) => rest = &rest[start + end + 3..], + None => { + rest = ""; + break; + } + } + } + out.push_str(rest); + out.split_whitespace().collect::>().join(" ") + } + + /// The body of each ``, so an action and a MIME type are + /// checked to be in the *same* filter. Two filters, one naming the action + /// and one naming the type, register for neither. + fn intent_filters(manifest: &str) -> Vec<&str> { + manifest + .split("") + .skip(1) + .filter_map(|filter| filter.split("").next()) + .collect() + } + + /// The single `` element, attributes and all. + fn provider(manifest: &str) -> String { + let start = manifest + .find(" in the manifest"); + let rest = &manifest[start..]; + let end = rest.find("/>").expect("unterminated element"); + rest[..end + 2].to_string() + } + + fn attribute(element: &str, name: &str) -> Option { + let key = format!("{name}=\""); + let start = element.find(&key)? + key.len(); + let value = element[start..].split('"').next()?; + Some(value.to_string()) + } + + #[test] + fn a_gallery_can_open_a_photograph_in_this_app() { + let manifest = manifest(); + let registered = intent_filters(&manifest).iter().any(|filter| { + filter.contains("android.intent.action.VIEW") + && filter.contains("android.intent.category.DEFAULT") + && filter.contains(r#"android:mimeType="image/*""#) + }); + assert!( + registered, + "no VIEW filter for image/*: nothing will offer DarkRoom for a photograph" + ); + } + + #[test] + fn the_share_sheet_can_send_one_image_or_several() { + let manifest = manifest(); + let registered = intent_filters(&manifest).iter().any(|filter| { + // The closing quote matters: SEND is a prefix of SEND_MULTIPLE, so + // a bare substring test passes on a filter that declares only the + // second and would not be offered for a single photograph. + filter.contains(r#"android.intent.action.SEND""#) + && filter.contains(r#"android.intent.action.SEND_MULTIPLE""#) + && filter.contains("android.intent.category.DEFAULT") + && filter.contains(r#"android:mimeType="image/*""#) + }); + assert!( + registered, + "no SEND/SEND_MULTIPLE filter for image/*, so the share sheet will not list DarkRoom" + ); + } + + #[test] + fn one_activity_ever_so_a_second_launch_cannot_start_a_second_one() { + // Not style. Another app can now launch this activity while it is + // already running, and the default launch mode answers that by + // creating a second NativeActivity in this process — a second + // android_main, a second Slint backend, a second wgpu device. + assert!( + manifest().contains(r#"android:launchMode="singleTask""#), + "the activity must be singleTask; see the manifest comment" + ); + } + + #[test] + fn the_provider_authority_is_the_one_the_class_answers_to() { + let manifest = manifest(); + let element = provider(&manifest); + + let declared = + attribute(&element, "android:authorities").expect("the provider declares no authority"); + let java = include_str!("../android/java/paris/tourolle/darkroom/ExportProvider.java"); + let constant = java + .split("AUTHORITY = \"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("ExportProvider declares no AUTHORITY constant"); + + assert_eq!( + declared, constant, + "the manifest and ExportProvider disagree about the authority; \ + a share would fail as a SecurityException inside the receiving app" + ); + + let class = attribute(&element, "android:name").expect("the provider declares no class"); + let (package, _) = class + .rsplit_once('.') + .expect("the provider class is unqualified"); + assert!( + java.contains(&format!("package {package};")), + "the manifest names {class}, which is not the class in ExportProvider.java" + ); + } + + #[test] + fn the_provider_hands_out_one_file_at_a_time_and_nothing_by_itself() { + let manifest = manifest(); + let element = provider(&manifest); + // The two halves are not redundant. Without the grant, every share + // target fails; exported, every app on the device could read this + // app's private directory. + assert_eq!( + attribute(&element, "android:exported").as_deref(), + Some("false"), + "an exported provider would serve the app's private directory to anything installed" + ); + assert_eq!( + attribute(&element, "android:grantUriPermissions").as_deref(), + Some("true"), + "without URI grants the share sheet opens and every target fails to read the file" + ); + } +} diff --git a/docker/android/README.md b/docker/android/README.md index accbc79..6466dd7 100644 --- a/docker/android/README.md +++ b/docker/android/README.md @@ -51,6 +51,27 @@ inside app-private storage, `run-as` needs a debuggable build, and the app has n fetch. docs/faces.md §2.2a is the decision and its limits — these files come back out before anything is published. +## Java in the APK + +There is no Gradle here and there is no AndroidX, so `assemble-apk.sh` compiles Java itself: +everything under `apps/darkroom-android/android/java/` goes through `javac` against `android.jar`, +and `d8` merges the result with the dex Slint's build script produced for its own helper. One +`classes.dex` comes out. Drop a `.java` file in that tree and the next build picks it up; an empty +tree skips the step entirely and the APK carries Slint's dex alone, which is what it did before the +step existed. + +**Java is for what Android constructs, and nothing else.** The system instantiates a +`ContentProvider` from its manifest entry, and `Activity.getIntent()` is only reachable on an +activity object — android-activity hands Rust a JNI handle to a stock `NativeActivity`, not a +subclass it could have put code in. Those cases need a class in the APK at any price. Everything +else stays in Rust, because a second language is a second place for the logic to live. + +The step compiles `-source 8 -target 8` with `-bootclasspath android.jar`. That is not conservatism +about language features — it is the last combination in which javac lets the boot class path be +replaced. From `-target 9` the flag is rejected and the platform classes come from the JDK instead, +which compiles and then dies on the device with `NoClassDefFoundError` for a class Android never +shipped. + ## Pinned versions | Component | Version | Why this one | diff --git a/docker/android/assemble-apk.sh b/docker/android/assemble-apk.sh index 8dca2ef..dfa1f0a 100755 --- a/docker/android/assemble-apk.sh +++ b/docker/android/assemble-apk.sh @@ -103,6 +103,80 @@ DEX="$(find "${TARGET_DIR}/${RUST_TARGET}/release/build" \ [[ -n "${DEX}" ]] || { echo "error: Slint classes.dex not found — did the backend build?" >&2; exit 1; } echo " dex: ${DEX}" +# --------------------------------------------------------------------------- +# Our own Java. +# +# Almost all of this app is Rust, and the classes here are the exceptions the +# platform forces: Android constructs some things itself, from a class named in +# the manifest, and hands the result back. A `ContentProvider` is one — the +# system instantiates it, nothing in the process ever calls its constructor — +# and reaching the launch `Intent` is another, because it arrives through +# `Activity.getIntent()` and android-activity gives Rust a JNI handle to a +# stock `NativeActivity` rather than a subclass it could have put code in. +# Neither can be written as Rust at any price, so the APK needs a dex of ours. +# +# Skipped when the tree has no Java, which is the state this build was in until +# FR-PLAT-AND-6 and the state a cut-down branch may return to. The step then +# costs nothing and the APK carries Slint's dex alone, exactly as before. +JAVA_SRC="${REPO}/apps/darkroom-android/android/java" +JAVA_FILES=() +if [[ -d "${JAVA_SRC}" ]]; then + mapfile -t JAVA_FILES < <(find "${JAVA_SRC}" -name '*.java' | sort) +fi + +if [[ ${#JAVA_FILES[@]} -gt 0 ]]; then + echo "==> compiling ${#JAVA_FILES[@]} Java source(s)" + mkdir -p "${OUT}/classes" "${OUT}/dex" + + # `-source 8 -target 8` with an explicit `-bootclasspath`, because that is + # the last combination in which javac still lets the boot class path be + # replaced: from `-target 9` onwards it rejects the flag outright, and the + # platform classes then come from the *JDK* rather than from android.jar. + # That compiles cleanly and fails on the device — a JDK class Android does + # not ship raises NoClassDefFoundError the moment it is touched, with + # nothing at build time having said so. Compiling against android.jar and + # only android.jar is what makes "it compiled" mean "the device has it". + # + # `-Xlint:-options` silences one note, "source value 8 is obsolete", which + # is advice about a future JDK rather than about this code. The JDK is + # pinned in the Dockerfile, so the day it matters is a deliberate bump. + javac \ + -source 8 -target 8 \ + -bootclasspath "${ANDROID_JAR}" \ + -classpath "${ANDROID_JAR}" \ + -Xlint:-options \ + -d "${OUT}/classes" \ + "${JAVA_FILES[@]}" + + mapfile -t CLASS_FILES < <(find "${OUT}/classes" -name '*.class' | sort) + + # d8 merges, it does not only translate. Handing it Slint's finished + # classes.dex alongside our fresh .class files yields one dex holding both, + # which is what the zip step below already expects. The alternative — ours + # as a second classes2.dex — works at API 28, where multidex is native, but + # leaves two files to keep in step in the staging and zip steps for no gain + # at this size. + # + # `--min-api` is MIN_API for the same reason the linkers use it: d8 decides + # what it must desugar from the oldest device this APK may reach, and a + # higher number here emits bytecode that verifies against the build + # machine's idea of Android and not against that device's. + # + # `--lib` is android.jar rather than a copy of the classpath: desugaring + # needs to see the platform types it is desugaring against, and without it + # d8 reports missing classes for anything our code touches. + "${BT}/d8" \ + --release \ + --min-api "${MIN_API}" \ + --lib "${ANDROID_JAR}" \ + --output "${OUT}/dex" \ + "${DEX}" \ + "${CLASS_FILES[@]}" + + DEX="${OUT}/dex/classes.dex" + echo " dex: ${DEX} (ours merged with Slint's)" +fi + # A debug keystore. CI points KEYSTORE at a throwaway directory so nothing is # persisted or published; package.sh keeps one in the cache on purpose, because # Android refuses to update an installed app whose signature changed and a new @@ -234,7 +308,8 @@ echo " signing: ${SIGNING}" # The intermediates are not the artefact, and leaving them beside it invites # the wrong file being picked up by a glob. -rm -rf "${OUT}/staging" "${OUT}/res.zip" "${OUT}/base.apk" "${OUT}/unaligned.apk" +rm -rf "${OUT}/staging" "${OUT}/res.zip" "${OUT}/base.apk" "${OUT}/unaligned.apk" \ + "${OUT}/classes" "${OUT}/dex" echo "==> ${OUT}/darkroom.apk" ls -la "${OUT}/darkroom.apk" diff --git a/docker/android/package.sh b/docker/android/package.sh index 61edcda..7c8fa1b 100755 --- a/docker/android/package.sh +++ b/docker/android/package.sh @@ -6,11 +6,11 @@ # # Gradle would add a second build system, a second dependency tree, and a # second place for the toolchain versions to drift out of step with the -# Dockerfile. The four tools it would have driven — aapt2, d8, zipalign, -# apksigner — are in build-tools already and are enough on their own, because -# the app has no Java of its own: android-activity's glue calls android_main -# directly, and the only classes in the APK are the ones Slint's build script -# compiles for its own helper. +# Dockerfile. The tools it would have driven — javac, aapt2, d8, zipalign, +# apksigner — are in the image already and are enough on their own, because +# the app is Rust: android-activity's glue calls android_main directly, and +# the only Java in the APK is Slint's helper plus the handful of classes +# Android insists on constructing itself (see assemble-apk.sh's Java step). set -euo pipefail HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"