From 8524ed3dd2446b88d7f10911f247c5e7c2c34b81 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 21 Aug 2026 10:50:35 +0200 Subject: [PATCH] Open the document by what it is called when its bytes only say text odrcore builds its open candidates from the content alone - `list_file_types` is `magic::file_type`, and the name is never consulted. A document whose signature does not sit at the front of the file therefore never opens as itself. The case that turned this up is a pdf saved straight out of a browser, with the http response that delivered it still attached, so `%PDF-1.4` starts at byte 178. What the user got depended on the file. `odr-public`-shaped ones read as text with a charset the core could name, and the reader was shown pages of pdf source with a bar offering to open it elsewhere. The file in #552 has binary enough content that no charset could be named, so the guard in `host` refused it and the app reported an unsupported format - for a pdf the core parses perfectly well once told what it is. Neither knob in `DecodePreference` fixes this on its own. `fileTypePriority` only stable-sorts the candidates detection already found, so it is inert when detection found the wrong one or none. `asFileType` replaces detection outright and has no fallback, which would hand a file misnamed by its extension to a parser that cannot read it. So it is a second attempt rather than the first: `CoreLoader.openFile` opens as detection reads the bytes, and only where that reading is the residual answer does it open again as the type the name states. Two things keep it narrow. The name has to be one the core files as a `DOCUMENT` - csv and plain text are both `text` by category, so whether comma separated values are a table or prose stays the core's question, which #576 and 6.8.0 tuned deliberately. And the name is taken from `IdentifiedFile.filename`, not from its `mimeType`: `FileIdentifier` takes that from `Odr.mimetype` wherever it answered, so feeding it back would only repeat the reading this is here to back up. `CoreTest` pins both halves against a fixture it builds at run time from `dummy.pdf`, so no binary asset joins the repo: without a declared type the served html carries the http preamble as text, with one it does not, and an odt declared a pdf still opens as an odt. Checked on API 31 (full suite, 84 tests) and API 36, plus a sweep of the corpus' txt and csv files, which are what this could have disturbed and did not - the 5000 row csv still comes out a table. The private pdf #552 names renders as the document it is. Closes #552 --- CHANGELOG.md | 3 + CLAUDE.md | 7 +++ .../app/opendocument/droid/test/CoreTest.kt | 57 +++++++++++++++++ .../droid/background/CoreLoader.kt | 63 ++++++++++++++++++- 4 files changed, 128 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 544fb0d4187d..931049706084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ takes 500 characters, so not everything here reaches the store. ## Unreleased +- A PDF saved straight out of a browser opens. The web server's response was + still sitting in front of the file, so it was read as plain text and shown as + pages of its own source, or refused as an unsupported format. - A password-protected Word, Excel or PowerPoint file says so, instead of failing to open for no stated reason. No password opens one yet, so the app no longer asks for one either. diff --git a/CLAUDE.md b/CLAUDE.md index e1dee248d41c..fb6e13ad033e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -231,6 +231,13 @@ Both are needed: the first keeps `isRenderedByCore` off a `.bin`, the second sto bar appearing over a page that cannot draw. `LandingTests.aDocumentThatFailsToOpenComesBackToTheList` holds this. +The same guess is why **what the file is called can outrank it**. odrcore never looks at the +name, so a document whose signature does not sit at the front - a pdf carrying the http response +that delivered it - reads as text. `CoreLoader.openFile` opens it again as the *filename*'s type, +but only where the core files that as a `DOCUMENT`: csv and plain text are both `text`, and which +of the two a file is stays the core's question. Not `IdentifiedFile.mimeType` - `FileIdentifier` +takes that from `Odr.mimetype`, so it is the same reading again. + ### How the document is displayed is answered over the document, not in the settings Three of the buttons in `DocumentActions` are about what the page looks like rather than what can diff --git a/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt b/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt index 08fee1a502fc..e5cd77a467bf 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/CoreTest.kt @@ -3,11 +3,13 @@ package app.opendocument.droid.test import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.LargeTest import androidx.test.platform.app.InstrumentationRegistry +import app.opendocument.core.FileType import app.opendocument.core.OdrException import app.opendocument.droid.background.CoreLoader import app.opendocument.droid.nonfree.CrashManager import java.io.File import java.io.FileOutputStream +import java.net.URL import org.junit.AfterClass import org.junit.Assert import org.junit.BeforeClass @@ -224,6 +226,52 @@ class CoreTest { } } + /** A pdf behind the http response that delivered it reads as text until the name is asked. */ + @Test + fun testWhatTheFileIsCalledOpensWhatDetectionReadsAsText() { + val prefixed = File(cacheDir(), "http-prefixed.pdf") + prefixed.writeBytes(HTTP_PREAMBLE.toByteArray() + pdfTestFile.readBytes()) + + val asDetected = firstView("preamble-detected", prefixed, declaredType = null) + Assert.assertTrue( + "detection alone should read the preamble and call the whole file text", + asDetected.contains("HTTP/1.0 200 OK"), + ) + + val asNamed = firstView("preamble-named", prefixed, FileType.PORTABLE_DOCUMENT_FORMAT) + Assert.assertFalse( + "a file called .pdf should open as one rather than as its own source", + asNamed.contains("HTTP/1.0 200 OK"), + ) + } + + /** Only a text reading is outranked, so a real format still opens as itself. */ + @Test + fun testWhatTheFileIsCalledDoesNotOverrideARealFormat() { + val views = + coreLoader.host( + prefix = "odt-called-pdf", + inputPath = testFile.absolutePath, + cachePath = File(cacheDir(), "odt_called_pdf").path, + declaredType = FileType.PORTABLE_DOCUMENT_FORMAT, + ) + + Assert.assertFalse("the odt should still open as an odt", views.isEmpty()) + } + + /** The html odrcore serves for [file]'s first view. */ + private fun firstView(prefix: String, file: File, declaredType: FileType?): String { + val views = + coreLoader.host( + prefix = prefix, + inputPath = file.absolutePath, + cachePath = File(cacheDir(), prefix).path, + declaredType = declaredType, + ) + + return URL(views.first().url).readText() + } + @Test fun testSpreadsheetSheetNames() { val views = @@ -256,6 +304,14 @@ class CoreTest { private lateinit var pptTestFile: File private lateinit var xlsTestFile: File private lateinit var encryptedDocTestFile: File + private lateinit var pdfTestFile: File + + /** What a document saved straight out of a browser carries in front of itself. */ + private const val HTTP_PREAMBLE = + "HTTP/1.0 200 OK\r\n" + + "Cache-Control: no-cache, private\r\n" + + "Content-Disposition: inline\r\n" + + "Content-Type: application/pdf\r\n\r\n" // @JvmStatic because junit requires @BeforeClass / @AfterClass to be static @JvmStatic @@ -270,6 +326,7 @@ class CoreTest { pptTestFile = extract("style-various-1.ppt") xlsTestFile = extract("file_example_XLS_10.xls") encryptedDocTestFile = extract("encrypted.doc") + pdfTestFile = extract("dummy.pdf") } @JvmStatic diff --git a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt index df6d8449a3b5..8c88bdb53f5c 100644 --- a/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt +++ b/app/src/main/java/app/opendocument/droid/background/CoreLoader.kt @@ -4,9 +4,12 @@ import android.content.Context import android.net.Uri import android.system.Os import android.util.Log +import app.opendocument.core.DecodePreference import app.opendocument.core.DecodedFile import app.opendocument.core.Document import app.opendocument.core.DocumentType +import app.opendocument.core.FileCategory +import app.opendocument.core.FileType import app.opendocument.core.Html import app.opendocument.core.HtmlColorScheme import app.opendocument.core.HtmlConfig @@ -73,6 +76,7 @@ class CoreLoader(private val context: Context) { editable = request.editable, paging = PaginationSetting.isEnabled(context), keepDocument = true, + declaredType = declaredType(file), ) return LoadedDocument( @@ -88,7 +92,8 @@ class CoreLoader(private val context: Context) { * Opens [inputPath], translates it to html and publishes it on the shared http server under * [prefix], replacing whatever was published before. * - * [keepDocument] retains the decoded document for [retranslate]. + * [keepDocument] retains the decoded document for [retranslate]; [declaredType] is what the + * document is called - see [openFile]. */ fun host( prefix: String, @@ -98,6 +103,7 @@ class CoreLoader(private val context: Context) { editable: Boolean = false, paging: Boolean = false, keepDocument: Boolean = false, + declaredType: FileType? = null, ): List { val server = checkNotNull(sharedServer) { "core server is not running" } @@ -105,7 +111,7 @@ class CoreLoader(private val context: Context) { server.clear() - var file = Odr.open(inputPath) + var file = openFile(inputPath, declaredType) if (file.passwordEncrypted()) { // the format's own answer rather than a list of ours, and already narrowed to a file @@ -178,6 +184,59 @@ class CoreLoader(private val context: Context) { } } + /** + * What the document is *called*. Not [IdentifiedFile.mimeType]: `FileIdentifier` takes that + * from `Odr.mimetype` wherever it answered, so it would be the same reading again. + */ + private fun declaredType(file: IdentifiedFile): FileType? { + val extension = MimeTypeResolver.parseExtension(file.filename)?.lowercase() ?: return null + val type = Odr.fileTypeByFileExtension(extension) ?: return null + + return type.takeIf { it != FileType.UNKNOWN } + } + + /** + * [inputPath] as odrcore reads its bytes, or as [declaredType] where it answers *text* - its + * bucket for bytes nothing else claims, and where a pdf carrying its http response lands. + * + * Detection stays first, and only a name the core files as a `DOCUMENT` outranks text: whether + * comma separated values are a table or prose stays the core's question. + */ + private fun openFile(inputPath: String, declaredType: FileType?): DecodedFile { + val detected = + try { + Odr.open(inputPath) + } catch (e: Throwable) { + // nothing was recognised at all, which the name may still answer for + if (declaredType == null) { + throw e + } + + return openAs(inputPath, declaredType) ?: throw e + } + + if ( + declaredType == null || + declaredType == detected.fileType() || + !detected.isTextFile || + Odr.fileCategoryByFileType(declaredType) != FileCategory.DOCUMENT + ) { + return detected + } + + return openAs(inputPath, declaredType) ?: detected + } + + /** [inputPath] opened as [type], or null where it is not one after all. */ + private fun openAs(inputPath: String, type: FileType): DecodedFile? = + try { + Odr.open(inputPath, DecodePreference().apply { asFileType = type }) + } catch (e: Throwable) { + Log.i(TAG, "not a " + Odr.fileTypeToString(type)) + + null + } + /** The document with [htmlDiff] applied, written to a file of ours. Null if that failed. */ fun retranslate(request: DocumentRequest, file: IdentifiedFile, htmlDiff: String): File? { try {