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 {