mirror of
https://github.com/EthanShoeDev/fressh.git
synced 2026-01-11 06:12:51 +00:00
some fixes
This commit is contained in:
@@ -28,8 +28,7 @@ export default function Index() {
|
||||
secretsManager.connections.query.list,
|
||||
);
|
||||
|
||||
const preferredStoredConnection =
|
||||
storedConnectionsQuery.data?.firstConnection;
|
||||
const preferredStoredConnection = storedConnectionsQuery.data?.[0];
|
||||
const connectionForm = useAppForm({
|
||||
// https://tanstack.com/form/latest/docs/framework/react/guides/async-initial-values
|
||||
defaultValues: preferredStoredConnection
|
||||
@@ -56,7 +55,7 @@ export default function Index() {
|
||||
value.host,
|
||||
value.port,
|
||||
value.username,
|
||||
privateKey.privateKey,
|
||||
privateKey,
|
||||
);
|
||||
})();
|
||||
|
||||
@@ -231,7 +230,7 @@ const KeyPairSection = withFieldGroup({
|
||||
) : (
|
||||
<>
|
||||
<field.PickerField label="Key">
|
||||
{listPrivateKeysQuery.data?.keys.map((key) => (
|
||||
{listPrivateKeysQuery.data?.map((key) => (
|
||||
<Picker.Item key={key.id} label={key.id} value={key.id} />
|
||||
))}
|
||||
</field.PickerField>
|
||||
@@ -276,9 +275,9 @@ function PreviousConnectionsSection(props: {
|
||||
<Text style={styles.mutedText}>Loading connections...</Text>
|
||||
) : listConnectionsQuery.isError ? (
|
||||
<Text style={styles.errorText}>Error loading connections</Text>
|
||||
) : listConnectionsQuery.data?.manifest.connections.length ? (
|
||||
) : listConnectionsQuery.data?.length ? (
|
||||
<View style={styles.listContainer}>
|
||||
{listConnectionsQuery.data?.manifest.connections.map((conn) => (
|
||||
{listConnectionsQuery.data?.map((conn) => (
|
||||
<ConnectionRow
|
||||
key={conn.id}
|
||||
id={conn.id}
|
||||
@@ -298,7 +297,7 @@ function ConnectionRow(props: {
|
||||
onSelect: (connection: ConnectionDetails) => void;
|
||||
}) {
|
||||
const detailsQuery = useQuery(secretsManager.connections.query.get(props.id));
|
||||
const details = detailsQuery.data?.details;
|
||||
const details = detailsQuery.data;
|
||||
|
||||
return (
|
||||
<Pressable
|
||||
|
||||
@@ -26,21 +26,27 @@ export default function Shell() {
|
||||
console.log('Received data (on Shell):', data);
|
||||
setShellData((prev) => prev + data);
|
||||
});
|
||||
// return () => {
|
||||
// sshConn.client.off('Shell')
|
||||
// }
|
||||
return () => {
|
||||
// // Remove the handler by clearing the internal handler map.
|
||||
// // The library lacks a public `.off()`, but it routes through a single
|
||||
// // handler map per event name. Deleting the handler prevents updates.
|
||||
// // eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
// delete (sshConn.client as any)._handlers?.Shell;
|
||||
|
||||
// Set to no-op
|
||||
sshConn.client.on('Shell', () => {});
|
||||
};
|
||||
}, [setShellData, sshConn.client]);
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
setTimeout(() => {
|
||||
try {
|
||||
sshConnectionManager.removeAndDisconnectSession({ sessionId });
|
||||
console.log('Disconnected from SSH server');
|
||||
} catch (error) {
|
||||
console.error('Error disconnecting from SSH server', error);
|
||||
}
|
||||
}, 3_000);
|
||||
console.log('Clean up shell screen (immediate disconnect)');
|
||||
try {
|
||||
sshConnectionManager.removeAndDisconnectSession({ sessionId });
|
||||
console.log('Disconnected from SSH server');
|
||||
} catch (error) {
|
||||
console.error('Error disconnecting from SSH server', error);
|
||||
}
|
||||
};
|
||||
}, [sessionId]);
|
||||
|
||||
|
||||
@@ -4,35 +4,6 @@ import * as SecureStore from 'expo-secure-store';
|
||||
import * as z from 'zod';
|
||||
import { queryClient } from './utils';
|
||||
|
||||
const keys = {
|
||||
storagePrefix: 'privateKey_',
|
||||
manifestKey: 'privateKeysManifest',
|
||||
chunkSize: 1800, // Safely under 2048 byte limit
|
||||
} as const;
|
||||
|
||||
const keyManifestSchema = z.object({
|
||||
manifestVersion: z.number().default(1),
|
||||
keys: z.array(
|
||||
z.object({
|
||||
id: z.string(),
|
||||
priority: z.number(),
|
||||
createdAtMs: z.int(),
|
||||
chunkCount: z.number().default(1),
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
async function getKeyManifest() {
|
||||
const rawManifest = await SecureStore.getItemAsync(keys.manifestKey);
|
||||
const manifest = rawManifest
|
||||
? JSON.parse(rawManifest)
|
||||
: {
|
||||
manifestVersion: 1,
|
||||
keys: [],
|
||||
};
|
||||
return keyManifestSchema.parse(manifest);
|
||||
}
|
||||
|
||||
// Utility functions for chunking large data
|
||||
function splitIntoChunks(data: string, chunkSize: number): string[] {
|
||||
const chunks: string[] = [];
|
||||
@@ -42,96 +13,264 @@ function splitIntoChunks(data: string, chunkSize: number): string[] {
|
||||
return chunks;
|
||||
}
|
||||
|
||||
function getChunkKey(keyId: string, chunkIndex: number): string {
|
||||
return `${keys.storagePrefix}${keyId}_chunk_${chunkIndex}`;
|
||||
/**
|
||||
* Secure store does not support:
|
||||
* - Listing keys
|
||||
* - Storing more than 2048 bytes
|
||||
*
|
||||
* We can bypass both of those by using manifest entries and chunking.
|
||||
*/
|
||||
function makeBetterSecureStore<T extends object = object>(params: {
|
||||
storagePrefix: string;
|
||||
extraManifestFieldsSchema?: z.ZodType<T>;
|
||||
}) {
|
||||
// const sizeLimit = 2048;
|
||||
const sizeLimit = 2000;
|
||||
const rootManifestVersion = 1;
|
||||
const manifestChunkVersion = 1;
|
||||
|
||||
const rootManifestKey = `${params.storagePrefix}rootManifest`;
|
||||
const manifestChunkKey = (manifestChunkId: string) =>
|
||||
`${params.storagePrefix}manifestChunk_${manifestChunkId}`;
|
||||
const entryKey = (entryId: string, chunkIdx: number) =>
|
||||
`${params.storagePrefix}entry_${entryId}_chunk_${chunkIdx}`;
|
||||
|
||||
const rootManifestSchema = z.looseObject({
|
||||
manifestVersion: z.number().default(rootManifestVersion),
|
||||
// We need to chunk the manifest itself
|
||||
manifestChunksIds: z.array(z.string()),
|
||||
});
|
||||
|
||||
const entrySchema = z.object({
|
||||
id: z.string(),
|
||||
chunkCount: z.number().default(1),
|
||||
metadata: params.extraManifestFieldsSchema ?? z.object({}),
|
||||
});
|
||||
// type Entry = {
|
||||
// id: string;
|
||||
// chunkCount: number;
|
||||
// metadata: T;
|
||||
// };
|
||||
|
||||
type Entry = z.infer<typeof entrySchema>;
|
||||
|
||||
const manifestChunkSchema = z.object({
|
||||
manifestChunkVersion: z.number().default(manifestChunkVersion),
|
||||
entries: z.array(entrySchema),
|
||||
});
|
||||
|
||||
async function getManifest() {
|
||||
const rawRootManifestString =
|
||||
await SecureStore.getItemAsync(rootManifestKey);
|
||||
|
||||
console.log(
|
||||
`Root manifest for ${params.storagePrefix} is ${rawRootManifestString?.length} bytes`,
|
||||
);
|
||||
const unsafedRootManifest = rawRootManifestString
|
||||
? JSON.parse(rawRootManifestString)
|
||||
: {
|
||||
manifestVersion: rootManifestVersion,
|
||||
manifestChunksIds: [],
|
||||
};
|
||||
const rootManifest = rootManifestSchema.parse(unsafedRootManifest);
|
||||
const manifestChunks = await Promise.all(
|
||||
rootManifest.manifestChunksIds.map(async (manifestChunkId) => {
|
||||
const rawManifestChunkString = await SecureStore.getItemAsync(
|
||||
manifestChunkKey(manifestChunkId),
|
||||
);
|
||||
if (!rawManifestChunkString)
|
||||
throw new Error('Manifest chunk not found');
|
||||
console.log(
|
||||
`Manifest chunk for ${params.storagePrefix} ${manifestChunkId} is ${rawManifestChunkString?.length} bytes`,
|
||||
);
|
||||
const unsafedManifestChunk = JSON.parse(rawManifestChunkString);
|
||||
return {
|
||||
manifestChunk: manifestChunkSchema.parse(unsafedManifestChunk),
|
||||
manifestChunkId,
|
||||
manifestChunkSize: rawManifestChunkString.length,
|
||||
};
|
||||
}),
|
||||
);
|
||||
return {
|
||||
rootManifest,
|
||||
manifestChunks,
|
||||
};
|
||||
}
|
||||
|
||||
async function getEntry(id: string) {
|
||||
const manifest = await getManifest();
|
||||
const manifestEntry = manifest.manifestChunks.reduce<Entry | undefined>(
|
||||
(_, mChunk) =>
|
||||
mChunk.manifestChunk.entries.find((entry) => entry.id === id),
|
||||
undefined,
|
||||
);
|
||||
if (!manifestEntry) throw new Error('Entry not found');
|
||||
|
||||
const rawEntryChunks = await Promise.all(
|
||||
Array.from({ length: manifestEntry.chunkCount }, async (_, chunkIdx) => {
|
||||
const rawEntryChunk = await SecureStore.getItemAsync(
|
||||
entryKey(id, chunkIdx),
|
||||
);
|
||||
console.log(
|
||||
`Entry chunk for ${params.storagePrefix} ${id} ${chunkIdx} is ${rawEntryChunk?.length} bytes`,
|
||||
);
|
||||
return rawEntryChunk;
|
||||
}),
|
||||
);
|
||||
const entry = rawEntryChunks.join('');
|
||||
return entry;
|
||||
}
|
||||
|
||||
async function listEntries() {
|
||||
const manifest = await getManifest();
|
||||
return manifest.manifestChunks.flatMap(
|
||||
(mChunk) => mChunk.manifestChunk.entries,
|
||||
);
|
||||
}
|
||||
|
||||
async function deleteEntry(id: string) {
|
||||
const manifest = await getManifest();
|
||||
const manifestChunkContainingEntry = manifest.manifestChunks.find(
|
||||
(mChunk) => mChunk.manifestChunk.entries.some((entry) => entry.id === id),
|
||||
);
|
||||
if (!manifestChunkContainingEntry) throw new Error('Entry not found');
|
||||
|
||||
const manifestEntry =
|
||||
manifestChunkContainingEntry.manifestChunk.entries.find(
|
||||
(entry) => entry.id === id,
|
||||
);
|
||||
if (!manifestEntry) throw new Error('Entry not found');
|
||||
|
||||
const deleteEntryChunksPromise = Array.from(
|
||||
{ length: manifestEntry.chunkCount },
|
||||
async (_, chunkIdx) => {
|
||||
await SecureStore.deleteItemAsync(entryKey(id, chunkIdx));
|
||||
},
|
||||
);
|
||||
|
||||
const updateManifestChunkPromise = SecureStore.setItemAsync(
|
||||
manifestChunkKey(manifestChunkContainingEntry.manifestChunkId),
|
||||
JSON.stringify({
|
||||
...manifestChunkContainingEntry,
|
||||
entries: manifestChunkContainingEntry.manifestChunk.entries.filter(
|
||||
(entry) => entry.id !== id,
|
||||
),
|
||||
}),
|
||||
);
|
||||
|
||||
await Promise.all([
|
||||
...deleteEntryChunksPromise,
|
||||
updateManifestChunkPromise,
|
||||
]);
|
||||
}
|
||||
|
||||
async function upsertEntry(params: {
|
||||
id: string;
|
||||
metadata: T;
|
||||
value: string;
|
||||
}) {
|
||||
await deleteEntry(params.id).catch(() => {
|
||||
console.log(`Entry ${params.id} not found, creating new one`);
|
||||
});
|
||||
|
||||
const valueChunks = splitIntoChunks(params.value, sizeLimit);
|
||||
const newManifestEntry = entrySchema.parse({
|
||||
id: params.id,
|
||||
chunkCount: valueChunks.length,
|
||||
...params.metadata,
|
||||
});
|
||||
const newManifestEntrySize = JSON.stringify(newManifestEntry).length;
|
||||
if (newManifestEntrySize > sizeLimit / 2)
|
||||
throw new Error('Manifest entry size is too large');
|
||||
const manifest = await getManifest();
|
||||
|
||||
const existingManifestChunkWithRoom = manifest.manifestChunks.find(
|
||||
(mChunk) => sizeLimit > mChunk.manifestChunkSize + newManifestEntrySize,
|
||||
);
|
||||
const manifestChunkWithRoom =
|
||||
existingManifestChunkWithRoom ??
|
||||
(await (async () => {
|
||||
const newManifestChunk = {
|
||||
manifestChunk: {
|
||||
entries: [],
|
||||
manifestChunkVersion: manifestChunkVersion,
|
||||
},
|
||||
manifestChunkId: crypto.randomUUID(),
|
||||
manifestChunkSize: 0,
|
||||
} satisfies NonNullable<(typeof manifest.manifestChunks)[number]>;
|
||||
await SecureStore.setItemAsync(
|
||||
rootManifestKey,
|
||||
JSON.stringify(manifest.rootManifest),
|
||||
);
|
||||
return newManifestChunk;
|
||||
})());
|
||||
|
||||
manifestChunkWithRoom.manifestChunk.entries.push(newManifestEntry);
|
||||
await Promise.all([
|
||||
SecureStore.setItemAsync(
|
||||
manifestChunkKey(manifestChunkWithRoom.manifestChunkId),
|
||||
JSON.stringify(manifestChunkWithRoom.manifestChunk),
|
||||
),
|
||||
...valueChunks.map((vChunk, chunkIdx) =>
|
||||
SecureStore.setItemAsync(
|
||||
entryKey(newManifestEntry.id, chunkIdx),
|
||||
vChunk,
|
||||
),
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
return {
|
||||
getManifest,
|
||||
getEntry,
|
||||
listEntries,
|
||||
upsertEntry,
|
||||
deleteEntry,
|
||||
};
|
||||
}
|
||||
|
||||
const betterKeyStorage = makeBetterSecureStore({
|
||||
storagePrefix: 'privateKey_',
|
||||
extraManifestFieldsSchema: z.object({
|
||||
priority: z.number(),
|
||||
createdAtMs: z.int(),
|
||||
}),
|
||||
});
|
||||
|
||||
const betterConnectionStorage = makeBetterSecureStore({
|
||||
storagePrefix: 'connection_',
|
||||
extraManifestFieldsSchema: z.object({
|
||||
priority: z.number(),
|
||||
createdAtMs: z.int(),
|
||||
modifiedAtMs: z.int(),
|
||||
}),
|
||||
});
|
||||
|
||||
async function savePrivateKey(params: {
|
||||
keyId: string;
|
||||
privateKey: string;
|
||||
priority: number;
|
||||
}) {
|
||||
const manifest = await getKeyManifest();
|
||||
|
||||
const existingKey = manifest.keys.find((key) => key.id === params.keyId);
|
||||
|
||||
if (existingKey) throw new Error('Key already exists');
|
||||
|
||||
// Split the private key into chunks if it's too large
|
||||
const chunks = splitIntoChunks(params.privateKey, keys.chunkSize);
|
||||
const chunkCount = chunks.length;
|
||||
|
||||
const newKey = {
|
||||
await betterKeyStorage.upsertEntry({
|
||||
id: params.keyId,
|
||||
priority: params.priority,
|
||||
createdAtMs: Date.now(),
|
||||
chunkCount,
|
||||
};
|
||||
|
||||
manifest.keys.push(newKey);
|
||||
|
||||
// Save each chunk separately
|
||||
for (let i = 0; i < chunks.length; i++) {
|
||||
await SecureStore.setItemAsync(getChunkKey(params.keyId, i), chunks[i]!);
|
||||
}
|
||||
|
||||
await SecureStore.setItemAsync(keys.manifestKey, JSON.stringify(manifest));
|
||||
metadata: {
|
||||
priority: params.priority,
|
||||
createdAtMs: Date.now(),
|
||||
},
|
||||
value: params.privateKey,
|
||||
});
|
||||
await queryClient.invalidateQueries({ queryKey: [keyQueryKey] });
|
||||
}
|
||||
|
||||
async function getPrivateKey(keyId: string) {
|
||||
const manifest = await getKeyManifest();
|
||||
const key = manifest.keys.find((key) => key.id === keyId);
|
||||
if (!key) throw new Error('Key not found');
|
||||
|
||||
// Reassemble the private key from chunks
|
||||
const chunks: string[] = [];
|
||||
for (let i = 0; i < key.chunkCount; i++) {
|
||||
const chunk = await SecureStore.getItemAsync(getChunkKey(keyId, i));
|
||||
if (!chunk) throw new Error(`Key chunk ${i} not found`);
|
||||
chunks.push(chunk);
|
||||
}
|
||||
|
||||
const privateKey = chunks.join('');
|
||||
return {
|
||||
...key,
|
||||
privateKey,
|
||||
};
|
||||
return await betterKeyStorage.getEntry(keyId);
|
||||
}
|
||||
|
||||
async function deletePrivateKey(keyId: string) {
|
||||
const manifest = await getKeyManifest();
|
||||
const key = manifest.keys.find((key) => key.id === keyId);
|
||||
if (!key) throw new Error('Key not found');
|
||||
|
||||
manifest.keys = manifest.keys.filter((key) => key.id !== keyId);
|
||||
|
||||
// Delete all chunks for this key
|
||||
for (let i = 0; i < key.chunkCount; i++) {
|
||||
await SecureStore.deleteItemAsync(getChunkKey(keyId, i));
|
||||
}
|
||||
|
||||
await SecureStore.setItemAsync(keys.manifestKey, JSON.stringify(manifest));
|
||||
await betterKeyStorage.deleteEntry(keyId);
|
||||
await queryClient.invalidateQueries({ queryKey: [keyQueryKey] });
|
||||
}
|
||||
|
||||
const connections = {
|
||||
storagePrefix: 'connection_',
|
||||
manifestKey: 'connectionsManifest',
|
||||
} as const;
|
||||
|
||||
const connectionsManifestSchema = z.object({
|
||||
manifestVersion: z.number().default(1),
|
||||
connections: z.array(
|
||||
z.object({
|
||||
id: z.string(),
|
||||
priority: z.number(),
|
||||
createdAt: z.date(),
|
||||
modifiedAt: z.date(),
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
export const connectionDetailsSchema = z.object({
|
||||
host: z.string().min(1),
|
||||
port: z.number().min(1),
|
||||
@@ -150,82 +289,32 @@ export const connectionDetailsSchema = z.object({
|
||||
|
||||
export type ConnectionDetails = z.infer<typeof connectionDetailsSchema>;
|
||||
|
||||
async function getConnectionManifest() {
|
||||
const rawManifest = await SecureStore.getItemAsync(connections.manifestKey);
|
||||
const manifest = rawManifest
|
||||
? JSON.parse(rawManifest)
|
||||
: {
|
||||
manifestVersion: 1,
|
||||
connections: [],
|
||||
};
|
||||
return connectionsManifestSchema.parse(manifest);
|
||||
}
|
||||
|
||||
async function upsertConnection(params: {
|
||||
id: string;
|
||||
details: ConnectionDetails;
|
||||
priority: number;
|
||||
}) {
|
||||
const manifest = await getConnectionManifest();
|
||||
const existingConnection = manifest.connections.find(
|
||||
(connection) => connection.id === params.id,
|
||||
);
|
||||
|
||||
const newConnection = existingConnection
|
||||
? {
|
||||
...existingConnection,
|
||||
priority: params.priority,
|
||||
modifiedAt: new Date(),
|
||||
}
|
||||
: {
|
||||
id: params.id,
|
||||
priority: params.priority,
|
||||
createdAt: new Date(),
|
||||
modifiedAt: new Date(),
|
||||
};
|
||||
|
||||
await SecureStore.setItemAsync(
|
||||
connections.manifestKey,
|
||||
JSON.stringify(manifest),
|
||||
);
|
||||
await SecureStore.setItemAsync(
|
||||
`${connections.storagePrefix}${params.id}`,
|
||||
JSON.stringify(params.details),
|
||||
);
|
||||
await betterConnectionStorage.upsertEntry({
|
||||
id: params.id,
|
||||
metadata: {
|
||||
priority: params.priority,
|
||||
createdAtMs: Date.now(),
|
||||
modifiedAtMs: Date.now(),
|
||||
},
|
||||
value: JSON.stringify(params.details),
|
||||
});
|
||||
await queryClient.invalidateQueries({ queryKey: [connectionQueryKey] });
|
||||
return existingConnection ?? newConnection;
|
||||
return params.details;
|
||||
}
|
||||
|
||||
async function deleteConnection(id: string) {
|
||||
const manifest = await getConnectionManifest();
|
||||
const connection = manifest.connections.find(
|
||||
(connection) => connection.id === id,
|
||||
);
|
||||
if (!connection) throw new Error('Connection not found');
|
||||
manifest.connections = manifest.connections.filter(
|
||||
(connection) => connection.id !== id,
|
||||
);
|
||||
await SecureStore.setItemAsync(
|
||||
connections.manifestKey,
|
||||
JSON.stringify(manifest),
|
||||
);
|
||||
await SecureStore.deleteItemAsync(`${connections.storagePrefix}${id}`);
|
||||
await betterConnectionStorage.deleteEntry(id);
|
||||
await queryClient.invalidateQueries({ queryKey: [connectionQueryKey] });
|
||||
}
|
||||
|
||||
async function getConnection(id: string) {
|
||||
const manifest = await getConnectionManifest();
|
||||
const connection = manifest.connections.find(
|
||||
(connection) => connection.id === id,
|
||||
);
|
||||
if (!connection) throw new Error('Connection not found');
|
||||
const detailsString = await SecureStore.getItemAsync(
|
||||
`${connections.storagePrefix}${id}`,
|
||||
);
|
||||
if (!detailsString) throw new Error('Connection details not found');
|
||||
const detailsJson = JSON.parse(detailsString);
|
||||
const details = connectionDetailsSchema.parse(detailsJson);
|
||||
return { ...connection, details };
|
||||
const connDetailsString = await betterConnectionStorage.getEntry(id);
|
||||
return connectionDetailsSchema.parse(JSON.parse(connDetailsString));
|
||||
}
|
||||
|
||||
const connectionQueryKey = 'connections';
|
||||
@@ -233,16 +322,17 @@ const connectionQueryKey = 'connections';
|
||||
const listConnectionsQueryOptions = queryOptions({
|
||||
queryKey: [connectionQueryKey],
|
||||
queryFn: async () => {
|
||||
const manifest = await getConnectionManifest();
|
||||
const firstConnectionMeta = manifest.connections[0];
|
||||
const firstConnection = firstConnectionMeta
|
||||
? await getConnection(firstConnectionMeta.id)
|
||||
: null;
|
||||
|
||||
return {
|
||||
manifest,
|
||||
firstConnection,
|
||||
};
|
||||
const connManifests = await betterConnectionStorage.listEntries();
|
||||
const results = await Promise.all(
|
||||
connManifests.map(async (connManifest) => {
|
||||
const details = await getConnection(connManifest.id);
|
||||
return {
|
||||
details,
|
||||
id: connManifest.id,
|
||||
};
|
||||
}),
|
||||
);
|
||||
return results;
|
||||
},
|
||||
});
|
||||
|
||||
@@ -256,7 +346,7 @@ const keyQueryKey = 'keys';
|
||||
|
||||
const listKeysQueryOptions = queryOptions({
|
||||
queryKey: [keyQueryKey],
|
||||
queryFn: getKeyManifest,
|
||||
queryFn: async () => await betterKeyStorage.listEntries(),
|
||||
});
|
||||
|
||||
// https://github.com/dylankenneally/react-native-ssh-sftp/blob/ea55436d8d40378a8f9dabb95b463739ffb219fa/android/src/main/java/me/keeex/rnssh/RNSshClientModule.java#L101-L119
|
||||
@@ -279,7 +369,7 @@ async function generateKeyPair(params: {
|
||||
export const secretsManager = {
|
||||
keys: {
|
||||
utils: {
|
||||
getKeyManifest,
|
||||
listPrivateKeys: () => betterKeyStorage.listEntries(),
|
||||
savePrivateKey,
|
||||
getPrivateKey,
|
||||
deletePrivateKey,
|
||||
|
||||
245
doc/ssh-native-module-fix.md
Normal file
245
doc/ssh-native-module-fix.md
Normal file
@@ -0,0 +1,245 @@
|
||||
# Android SSH Native Module: Crash Root Cause and Fix Plan
|
||||
|
||||
This document explains the dev-only crash observed when navigating back
|
||||
(unmounting the Shell screen) and disconnecting an SSH session, and details the
|
||||
native changes required in `@dylankenneally/react-native-ssh-sftp` to resolve it
|
||||
properly.
|
||||
|
||||
## Summary
|
||||
|
||||
- Symptom: App crashes in development when leaving the Shell screen while an SSH
|
||||
shell is active and `disconnect()` runs. Release builds do not crash.
|
||||
- Root cause: The Android native module sometimes invokes the `startShell`
|
||||
callback more than once and lets the shell read loop terminate via an
|
||||
exception. In React Native dev mode, invoking a native callback multiple times
|
||||
can crash the bridge.
|
||||
- JS-side mitigations we applied in the app:
|
||||
- Removed the artificial 3s timeout before disconnect; disconnect immediately
|
||||
on unmount.
|
||||
- Replaced the Shell event handler with a no-op on unmount to avoid setState
|
||||
after unmount.
|
||||
- Proper native fix (recommended upstream):
|
||||
1. Ensure `startShell` callback is invoked exactly once (on initial start
|
||||
only).
|
||||
2. Make `closeShell` shut down the loop cleanly and null out resources.
|
||||
3. Guard `sendEvent` against emitting when the bridge is inactive.
|
||||
4. Optionally remove clients from the `clientPool` after disconnect.
|
||||
|
||||
## Where the issue happens
|
||||
|
||||
File:
|
||||
`node_modules/@dylankenneally/react-native-ssh-sftp/android/src/main/java/me/keeex/rnssh/RNSshClientModule.java`
|
||||
|
||||
Relevant methods:
|
||||
|
||||
- `startShell(String key, String ptyType, Callback callback)`
|
||||
- Starts the shell on a background thread, calls `callback.invoke()` when
|
||||
connected, then loops reading lines and emitting `Shell` events.
|
||||
- On stream termination, the code currently exits via exceptions
|
||||
(`IOException`, etc.) and in the catch blocks also calls
|
||||
`callback.invoke(error.getMessage())` a second time.
|
||||
|
||||
- `closeShell(String key)`
|
||||
- Closes output stream, input reader, and the channel, but it does not set the
|
||||
fields to `null`. The `startShell` loop uses
|
||||
`while (client._bufferedReader != null && ...)`, which relies on `null` to
|
||||
terminate cleanly.
|
||||
|
||||
- `sendEvent(ReactContext reactContext, String eventName, @Nullable WritableMap params)`
|
||||
- Emits DeviceEventManager events without checking whether the React bridge is
|
||||
alive.
|
||||
|
||||
- `disconnect(String key)`
|
||||
- Calls `closeShell(key)` and `disconnectSFTP(key)` and then disconnects the
|
||||
session but does not remove the entry from `clientPool`.
|
||||
|
||||
## Why dev-only
|
||||
|
||||
React Native’s dev bridge is stricter: invoking a native callback multiple times
|
||||
can trigger an error that surfaces as an immediate crash during development. In
|
||||
release builds, this usually doesn’t bring the app down the same way, which
|
||||
matches the behavior observed.
|
||||
|
||||
Specifically:
|
||||
|
||||
1. `startShell` calls the callback on success (good) but may call the same
|
||||
callback again inside a catch block when the shell loop ends with an
|
||||
exception (bad). That is a “callback invoked twice” scenario.
|
||||
|
||||
2. Because `closeShell` does not null
|
||||
`_bufferedReader`/`_dataOutputStream`/`_channel`, the read loop
|
||||
(`while (client._bufferedReader != null && (line = ...))`) doesn’t exit by
|
||||
the `null` check; it exits by throwing `IOException` when the stream is
|
||||
closed, sending control flow to the catch block that re-invokes the callback.
|
||||
|
||||
3. Emitting events after the bridge is torn down (e.g., during fast
|
||||
unmount-navigate) can also cause noise in dev logs and compound timing
|
||||
problems.
|
||||
|
||||
## Native changes (proposed)
|
||||
|
||||
1. Send the `startShell` callback once only
|
||||
|
||||
- Current pattern:
|
||||
- On success: `callback.invoke();`
|
||||
- On exceptions: `callback.invoke(error.getMessage());` (second invocation)
|
||||
|
||||
- Proposed change:
|
||||
- Keep the success callback invocation.
|
||||
- Remove all subsequent `callback.invoke(...)` inside the catch blocks of
|
||||
`startShell`.
|
||||
- Log the exception with `Log.e(...)` but do not call the callback again. The
|
||||
callback is for “start shell” completion, not for “shell ended later”.
|
||||
|
||||
2. Make `closeShell` cleanly terminate the read loop
|
||||
|
||||
- After closing resources, set fields to `null`:
|
||||
- `client._channel = null;`
|
||||
- `client._dataOutputStream = null;`
|
||||
- `client._bufferedReader = null;`
|
||||
|
||||
- With this, the read loop conditional `client._bufferedReader != null` becomes
|
||||
false and exits without throwing.
|
||||
|
||||
3. Harden `sendEvent`
|
||||
|
||||
- Before emitting:
|
||||
- `if (reactContext == null || !reactContext.hasActiveCatalystInstance()) { return; }`
|
||||
- Wrap `emit` in a try/catch to prevent rare bridge-state races:
|
||||
```java
|
||||
try {
|
||||
reactContext
|
||||
.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class)
|
||||
.emit(eventName, params);
|
||||
} catch (Throwable t) {
|
||||
Log.w(LOGTAG, "Failed to emit event " + eventName, t);
|
||||
}
|
||||
```
|
||||
|
||||
4. Optionally remove clients from the pool after disconnect
|
||||
|
||||
- In `disconnect(String key)`, after `client._session.disconnect();`, call
|
||||
`clientPool.remove(key);` so future lookups cannot access stale references.
|
||||
|
||||
## Example diffs (illustrative)
|
||||
|
||||
Note: Line numbers may vary; these are conceptual patches to apply in the
|
||||
indicated methods.
|
||||
|
||||
### In `startShell`
|
||||
|
||||
```java
|
||||
// On success (keep):
|
||||
callback.invoke();
|
||||
|
||||
// In catch blocks (change):
|
||||
} catch (JSchException error) {
|
||||
Log.e(LOGTAG, "Error starting shell: " + error.getMessage());
|
||||
// DO NOT invoke callback again here
|
||||
} catch (IOException error) {
|
||||
Log.e(LOGTAG, "Error starting shell: " + error.getMessage());
|
||||
// DO NOT invoke callback again here
|
||||
} catch (Exception error) {
|
||||
Log.e(LOGTAG, "Error starting shell: " + error.getMessage());
|
||||
// DO NOT invoke callback again here
|
||||
}
|
||||
```
|
||||
|
||||
### In `closeShell`
|
||||
|
||||
```java
|
||||
if (client._channel != null) {
|
||||
client._channel.disconnect();
|
||||
}
|
||||
if (client._dataOutputStream != null) {
|
||||
client._dataOutputStream.flush();
|
||||
client._dataOutputStream.close();
|
||||
}
|
||||
if (client._bufferedReader != null) {
|
||||
client._bufferedReader.close();
|
||||
}
|
||||
|
||||
// Ensure the shell loop terminates cleanly:
|
||||
client._channel = null;
|
||||
client._dataOutputStream = null;
|
||||
client._bufferedReader = null;
|
||||
```
|
||||
|
||||
### In `sendEvent`
|
||||
|
||||
```java
|
||||
private void sendEvent(ReactContext reactContext, String eventName, @Nullable WritableMap params) {
|
||||
if (reactContext == null || !reactContext.hasActiveCatalystInstance()) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
reactContext
|
||||
.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class)
|
||||
.emit(eventName, params);
|
||||
} catch (Throwable t) {
|
||||
Log.w(LOGTAG, "Failed to emit event " + eventName, t);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### In `disconnect`
|
||||
|
||||
```java
|
||||
SSHClient client = clientPool.get(key);
|
||||
if (client != null) {
|
||||
client._session.disconnect();
|
||||
clientPool.remove(key); // optional but recommended
|
||||
}
|
||||
```
|
||||
|
||||
## Why these changes fix the issue
|
||||
|
||||
- Single-callback guarantee: React Native mandates native callbacks are invoked
|
||||
once per request. Making `startShell` callback only for the initial start
|
||||
satisfies this and prevents dev crashes.
|
||||
- Clean loop termination: Nulling the stream references ensures the shell loop
|
||||
exits by condition rather than by exception, avoiding the catch path that
|
||||
previously re-invoked the callback.
|
||||
- Safe event emission: Avoids emitting into a destroyed bridge during fast
|
||||
navigation/unmount cycles, reducing flakiness in dev.
|
||||
- Resource hygiene: Removing client entries and nulling references prevents
|
||||
accidental reuse of stale state and helps GC.
|
||||
|
||||
## What we changed in app code (JS)
|
||||
|
||||
- In `apps/mobile/src/app/shell.tsx`:
|
||||
- Removed `setTimeout` and disconnect immediately in the unmount cleanup.
|
||||
- Replaced the `Shell` event handler with a no-op in the effect cleanup to
|
||||
avoid setState on an unmounted component while native drains.
|
||||
- We deliberately did not call `closeShell()` ourselves, since the library’s
|
||||
`disconnect()` already handles it.
|
||||
|
||||
These app-level changes reduce the timing window and stop React state updates
|
||||
after unmount. They help, but the true fix is in the native module as outlined
|
||||
above.
|
||||
|
||||
## Testing
|
||||
|
||||
1. Build a Dev Client (or run a development build) so the native module is
|
||||
loaded.
|
||||
2. Connect, navigate to the shell, then press back to unmount while the shell is
|
||||
active.
|
||||
3. Capture logs:
|
||||
- `adb logcat --pid=$(adb shell pidof -s dev.fressh.app) RNSSHClient:V ReactNative:V ReactNativeJS:V AndroidRuntime:E *:S`
|
||||
4. With the native changes applied, verify:
|
||||
- No “callback invoked twice” or RN bridge callback violations.
|
||||
- No crash in dev.
|
||||
- Events stop cleanly and disconnect completes.
|
||||
|
||||
## Upstreaming
|
||||
|
||||
These fixes are small, safe, and self-contained. Consider opening a PR to
|
||||
`@dylankenneally/react-native-ssh-sftp` with:
|
||||
|
||||
- Callback discipline in `startShell`.
|
||||
- Clean resource nulling in `closeShell`.
|
||||
- Safe `sendEvent`.
|
||||
- Optional `clientPool.remove(key)` on disconnect.
|
||||
|
||||
Until it’s merged, you can maintain a `patch-package` to keep the project stable
|
||||
in development.
|
||||
Reference in New Issue
Block a user