Skip to content

[BR-2158]: decode percent-encoded URIs in document upload and thumbnail paths - #518

Open
terrerox wants to merge 6 commits into
release-1.12.1from
fix/android-upload-encoded-uri-thumbnail
Open

terrerox wants to merge 6 commits into
release-1.12.1from
fix/android-upload-encoded-uri-thumbnail

Conversation

@terrerox

@terrerox terrerox commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Uploads of files whose names contain spaces or non-ASCII characters failed with ENOENT because percent-encoded cache URIs were passed unmodified to RNFS.copyFile and to the native thumbnail generator. Decode the source URI before copy (copyFileFromEncodedUri) and the native-returned thumbnail URI (fromFileUri); drop the stale manual %20 encoding. Device-verified on Android.

@terrerox
terrerox requested a review from CandelR July 1, 2026 03:53
@terrerox terrerox self-assigned this Jul 1, 2026
Base automatically changed from feature/pb-5920-file-provider-signaling-v2 to release-1.9.1 July 13, 2026 17:33
@terrerox
terrerox requested a review from TamaraFinogina as a code owner July 13, 2026 19:13
@TamaraFinogina

Copy link
Copy Markdown
Contributor

@terrerox This branch is full of conflicts now; they should be resolved first

@terrerox
terrerox force-pushed the fix/android-upload-encoded-uri-thumbnail branch from b985e99 to a64d666 Compare July 15, 2026 20:58
@terrerox
terrerox removed the request for review from TamaraFinogina July 16, 2026 03:14
@sonarqubecloud

Copy link
Copy Markdown

Comment thread src/services/common/uri/uriHelpers.ts Outdated
Comment thread src/services/drive/file/utils/uploadFileUtils.ts Outdated
Comment thread src/services/common/uri/uriHelpers.ts Outdated
@terrerox
terrerox force-pushed the release-1.9.1 branch 2 times, most recently from e599696 to 01fb3f7 Compare August 27, 2026 05:25
Base automatically changed from release-1.9.1 to master September 10, 2026 05:35
…bnail paths

Uploads of files whose names contain spaces or non-ASCII characters failed
with ENOENT because percent-encoded cache URIs were passed unmodified to
RNFS.copyFile and to the native thumbnail generator. Decode the source URI
before copy (copyFileFromEncodedUri) and the native-returned thumbnail URI
(fromFileUri); drop the stale manual %20 encoding. Device-verified on Android.
@terrerox
terrerox changed the base branch from master to feature/release-1.11.1 October 1, 2026 18:27
@terrerox
terrerox force-pushed the fix/android-upload-encoded-uri-thumbnail branch from 008265e to c66ee72 Compare October 1, 2026 18:27
Comment thread android/app/build.gradle
useLegacyPackaging enableLegacyPackaging.toBoolean()
}
resources {
excludes += 'META-INF/versions/9/OSGI-INF/MANIFEST.MF'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also fixed an Android launch crash here (NoClassDefFoundError: okhttp3/internal/Util). We pin okhttp 5.3.2, but React Native's okhttp-urlconnection was still on 4.9.2 and crashed once Cloudflare set the _cfuvid cookie. I added okhttp-urlconnection:5.3.2 and excluded the duplicate META-INF/versions/9/OSGI-INF/MANIFEST.MF it brings, which was breaking the build. This affects master too.

@terrerox
terrerox requested a review from CandelR October 1, 2026 19:23
@CandelR
CandelR changed the base branch from feature/release-1.11.1 to bugfix/mail-fixes-2 October 2, 2026 12:44
import { decodeUriSafely } from '../uri/uriHelpers';

export async function copyFileFromEncodedUri(sourceUri: string, destPath: string): Promise<void> {
await fileSystemService.copyFile(decodeUriSafely(sourceUri), destPath);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also decodes content:// URIs, which breaks them: the %2F inside the document id becomes a real / and the URI no longer points to the file. It can happen when keepLocalCopy fails and AddModal falls back to the picker URI.

It would be good to add a test with a content:// URI.

@CandelR
CandelR force-pushed the bugfix/mail-fixes-2 branch from bd554fc to 7918c83 Compare October 6, 2026 06:38
Base automatically changed from bugfix/mail-fixes-2 to feature/PB-6778-mail October 6, 2026 06:39
Base automatically changed from feature/PB-6778-mail to master October 6, 2026 14:21
@terrerox
terrerox requested a review from CandelR October 7, 2026 05:24
@sonarqubecloud

sonarqubecloud Bot commented Oct 7, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@CandelR
CandelR changed the base branch from master to release-1.12.1 October 7, 2026 08:01

@CandelR CandelR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base branch replaced the BottomModal with FloatingActionMenu, so the void changes from the last commit are on code that no longer exists. Taking the base side there should resolve it, and the Sonar issue should go away with it since it is in the same removed block :)

import fileSystemService from '@internxt-mobile/services/FileSystemService';
import { decodeFileUriSafely } from '../uri/uriHelpers';

export async function copyFileFromEncodedUri(sourceUri: string, destPath: string): Promise<void> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use arrow functions :)

* Converts a file:// URI to a filesystem path: strips the leading `file://` scheme and
* percent-decodes the rest. Malformed percent sequences are kept as-is instead of throwing.
*/
export const fileUriToPath = (uri: string): string =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileUriToPath does almost the same as stripFileUri. Could we make stripFileUri use decodeUriSafely instead and keep a single helper? That way thumbnail.generation.ts does not need to change and the other callers of stripFileUri stop throwing on a stray % too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants