Why this exists alongside {@link FixtureDocumentsProvider}. Every passing convert and + * join test hands the worker a {@code Uri.fromFile(...)}, which takes the {@code uri.path} arm and + * never touches {@code FFmpegKitConfig.getSafParameterForRead}. That bridge is on 100% of real user + * conversions and was on 0% of tested ones; only its failure side was covered, by + * {@code UnopenableUriTest} pointing at an authority that does not exist. + * + *
Why not the documents provider. It cannot be reached. Measured three ways on an API 34 + * emulator: a {@code DOCUMENTS_PROVIDER} declared without {@code MANAGE_DOCUMENTS} is refused at + * install ("Provider must be protected by MANAGE_DOCUMENTS"); instrumentation runs in the target + * app's process, so {@code Instrumentation.getContext()} still carries the app's uid and is denied; + * and {@code adoptShellPermissionIdentity(MANAGE_DOCUMENTS)} is denied identically. The denial says + * what is required — "you obtain access using ACTION_OPEN_DOCUMENT or related APIs" — so a + * documents provider is reachable only through a picker-issued grant. See issue #226. + * + *
The bridge does not need one. {@code getSafParameterForRead} opens a file descriptor through + * the resolver and hands FFmpeg a {@code saf:} path; any readable {@code content://} URI exercises + * it. An ordinary provider may be exported without a permission, so this one is, and the whole test + * stays headless — no DocumentsUI, and none of the flake #190 records. + * + *
Unlike {@link FixtureDocumentsProvider} this may use {@code androidx} and Kotlin freely — it is + * loaded into the app process like any other provider, not into the bare test process. It is kept + * in Java anyway, next to its sibling, so the two read alike. + */ +public final class FixtureContentProvider extends ContentProvider { + + /** Authority. Distinct from the documents provider's, and from anything the app declares. */ + public static final String AUTHORITY = "org.libremediaconverter.test.content"; + + /** Builds a URI for one of this source set's committed assets, e.g. {@code sample_h264.mp4}. */ + public static Uri uriFor(String assetName) { + return new Uri.Builder().scheme("content").authority(AUTHORITY).appendPath(assetName).build(); + } + + @Override + public boolean onCreate() { + return true; + } + + @Override + public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException { + if (!"r".equals(mode)) { + throw new FileNotFoundException("this provider is read-only: " + mode); + } + return ParcelFileDescriptor.open(unpack(assetOf(uri)), ParcelFileDescriptor.MODE_READ_ONLY); + } + + /** + * Enough of {@link OpenableColumns} for {@code InputQuery.describe} to name and size the input. + * + *
Without these the app reaches the "Size unknown" screen, which is a different test. + */ + @Override + public Cursor query(Uri uri, String[] projection, String selection, String[] args, String sort) { + String asset = assetOf(uri); + File file; + try { + file = unpack(asset); + } catch (FileNotFoundException e) { + return null; + } + MatrixCursor cursor = new MatrixCursor( + new String[] {OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE}); + cursor.newRow().add(OpenableColumns.DISPLAY_NAME, asset).add(OpenableColumns.SIZE, file.length()); + return cursor; + } + + @Override + public String getType(Uri uri) { + return assetOf(uri).endsWith(".m4a") ? "audio/mp4" : "video/mp4"; + } + + @Override + public Uri insert(Uri uri, ContentValues values) { + throw new UnsupportedOperationException("read-only fixture provider"); + } + + @Override + public int delete(Uri uri, String selection, String[] args) { + throw new UnsupportedOperationException("read-only fixture provider"); + } + + @Override + public int update(Uri uri, ContentValues values, String selection, String[] args) { + throw new UnsupportedOperationException("read-only fixture provider"); + } + + private static String assetOf(Uri uri) { + String asset = uri.getLastPathSegment(); + return asset == null ? "" : asset; + } + + /** + * The asset on disk, unpacked the first time anything asks. + * + *
Reported as {@link FileNotFoundException} rather than swallowed: a provider answering with + * a zero-byte file would fail the conversion for a reason nothing states. + */ + private File unpack(String asset) throws FileNotFoundException { + File file = new File(getContext().getCacheDir(), "provided_" + asset); + if (file.length() > 0L) { + return file; + } + try (InputStream source = getContext().getAssets().open(asset); + OutputStream sink = new FileOutputStream(file)) { + byte[] buffer = new byte[8192]; + int read; + while ((read = source.read(buffer)) != -1) { + sink.write(buffer, 0, read); + } + } catch (IOException e) { + throw new FileNotFoundException("could not unpack " + asset + ": " + e); + } + return file; + } +} diff --git a/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommand.kt b/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommand.kt index ec939b6..68a92c6 100644 --- a/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommand.kt +++ b/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommand.kt @@ -35,6 +35,21 @@ object FFmpegConcatCommand { add("concat") add("-safe") add("0") + // And -protocol_whitelist permits the *scheme* those paths carry, which is a + // separate gate (#238). Every input the user actually picks is a content:// URI -- + // JoinScreen uses OpenMultipleDocuments -- so ConcatEngine maps it through + // FFmpegKitConfig.getSafParameterForRead and writes an `ffkitsaf:` path into the + // list file. The concat demuxer applies its own whitelist, defaulting to + // "file,crypto,data", and refused every one of them: + // + // [ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'! + // + // This only widens that default. It is on the stream-copy branch alone because it + // is the only one that feeds the demuxer a list file -- REENCODE passes each input + // with its own -i, where the whitelist does not apply, which is why joining over SAF + // worked for mismatched clips and failed for matching ones. + add("-protocol_whitelist") + add(PROTOCOL_WHITELIST) add("-i") add(listFile.absolutePath) add("-c") @@ -84,4 +99,12 @@ object FFmpegConcatCommand { add(output.absolutePath) } } + + /** + * The concat demuxer's protocol whitelist: FFmpeg's own default, plus ffmpeg-kit's SAF scheme. + * + * Spelled out rather than appended to an unknown default, because the default is FFmpeg's and + * could change under us; naming all four keeps the command self-describing. See #238. + */ + private const val PROTOCOL_WHITELIST = "file,crypto,data,ffkitsaf" } diff --git a/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommandTest.kt b/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommandTest.kt index c5b853d..764e1bc 100644 --- a/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommandTest.kt +++ b/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegConcatCommandTest.kt @@ -56,6 +56,33 @@ class FFmpegConcatCommandTest { assertEquals("0", args[args.indexOf("-safe") + 1]) } + /** + * The gate that `-safe 0` does not open, and the one every real join needs (#238). + * + * `-safe 0` permits absolute *paths*; the concat demuxer separately whitelists the *protocol*, + * defaulting to `file,crypto,data`. `JoinScreen` picks with `OpenMultipleDocuments`, so real + * inputs are `content://` and `ConcatEngine` writes `ffkitsaf:` paths into the list file — which + * the demuxer refused outright, failing every stream-copy join a user could actually start. + * + * The re-encode strategy has no equivalent assertion because it needs none: it passes each + * input with its own `-i` and never feeds the demuxer a list file. That asymmetry is exactly + * why the defect survived — joining mismatched clips over SAF worked. + */ + @Test + fun `stream copy whitelists the protocol its list file entries actually use`() { + val args = FFmpegConcatCommand.build( + ConcatStrategy.STREAM_COPY, + inputs, + listFile, + output, + OutputFormat.MP4_H264, + ) + val whitelist = args[args.indexOf("-protocol_whitelist") + 1].split(",") + assertTrue("ffmpeg-kit's SAF scheme must be permitted, got $whitelist", "ffkitsaf" in whitelist) + // The defaults have to survive too: the list file itself is opened over `file`. + assertTrue("the demuxer still reads the list file itself, got $whitelist", "file" in whitelist) + } + @Test fun `re-encode passes every input separately and builds a filter graph`() { val args = FFmpegConcatCommand.build(