Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: handle bad access when accessing paths #2375

Merged
merged 1 commit into from Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 0 additions & 9 deletions packages/desktop/app/AppState.ts
@@ -1,10 +1,7 @@
import { MessageType } from '../test/TestIpcMessage'
import { Store } from './javascripts/Main/Store/Store'
import { StoreKeys } from './javascripts/Main/Store/StoreKeys'
import { Paths, Urls } from './javascripts/Main/Types/Paths'
import { UpdateState } from './javascripts/Main/UpdateManager'
import { handleTestMessage } from './javascripts/Main/Utils/Testing'
import { isTesting } from './javascripts/Main/Utils/Utils'
import { WindowState } from './javascripts/Main/Window'

export class AppState {
Expand All @@ -27,12 +24,6 @@ export class AppState {
this.store.set(StoreKeys.LastRunVersion, this.version)

this.updates = new UpdateState(this)

if (isTesting()) {
handleTestMessage(MessageType.AppStateCall, (method, ...args) => {
;(this as any)[method](...args)
})
}
}

public isRunningVersionForFirstTime(): boolean {
Expand Down
41 changes: 22 additions & 19 deletions packages/desktop/app/index.ts
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import { app, ipcMain, shell } from 'electron'
import log from 'electron-log'
import fs from 'fs-extra'
Expand All @@ -9,8 +10,6 @@ import { Store } from './javascripts/Main/Store/Store'
import { StoreKeys } from './javascripts/Main/Store/StoreKeys'
import { isSnap } from './javascripts/Main/Types/Constants'
import { Paths } from './javascripts/Main/Types/Paths'
import { setupTesting } from './javascripts/Main/Utils/Testing'
import { isTesting } from './javascripts/Main/Utils/Utils'
import { CommandLineArgs } from './javascripts/Shared/CommandLineArgs'

enableExperimentalFeaturesForFileAccessFix()
Expand Down Expand Up @@ -39,10 +38,6 @@ if (userDataPathIndex > 0) {
migrateSnapStorage()
}

if (isTesting()) {
setupTesting()
}

log.transports.file.level = 'info'

process.on('uncaughtException', (err) => {
Expand Down Expand Up @@ -96,8 +91,12 @@ function migrateSnapStorage() {
fs.moveSync(fullFilePath, path.join(dest, fileName), {
overwrite: false,
})
} catch (error: any) {
console.error(`Migration: error occured while moving ${fullFilePath} to ${dest}:`, error?.message ?? error)
} catch (error) {
console.error(
`Migration: error occured while moving ${fullFilePath} to ${dest}:`,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(error as any)?.message ?? error,
)
}
}

Expand All @@ -110,18 +109,22 @@ function migrateSnapStorage() {
* Backups location has not been altered by the user. Move it to the
* user documents directory
*/
console.log(`Migration: moving ${store.data.backupsLocation} to ${Paths.documentsDir}`)
const newLocation = path.join(Paths.documentsDir, path.basename(store.data.backupsLocation))
try {
fs.copySync(store.data.backupsLocation, newLocation)
} catch (error: any) {
console.error(
`Migration: error occured while moving ${store.data.backupsLocation} to ${Paths.documentsDir}:`,
error?.message ?? error,
)
const documentsDir = Paths.documentsDir
console.log(`Migration: moving ${store.data.backupsLocation} to ${documentsDir}`)
if (documentsDir) {
const newLocation = path.join(documentsDir, path.basename(store.data.backupsLocation))
try {
fs.copySync(store.data.backupsLocation, newLocation)
} catch (error) {
console.error(
`Migration: error occured while moving ${store.data.backupsLocation} to ${documentsDir}:`,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(error as any)?.message ?? error,
)
}
store.set(StoreKeys.LegacyTextBackupsLocation, newLocation)
console.log('Migration: finished moving backups directory.')
}
store.set(StoreKeys.LegacyTextBackupsLocation, newLocation)
console.log('Migration: finished moving backups directory.')
}
}
}
Expand Up @@ -88,7 +88,7 @@ export class FilesBackupManager implements FileBackupsDevice {
return value === true
}

async getUserDocumentsDirectory(): Promise<string> {
async getUserDocumentsDirectory(): Promise<string | undefined> {
return Paths.documentsDir
}

Expand All @@ -103,7 +103,12 @@ export class FilesBackupManager implements FileBackupsDevice {
}

const LegacyTextBackupsDirectory = 'Standard Notes Backups'
return path.join(Paths.homeDir, LegacyTextBackupsDirectory)
const homeDir = Paths.homeDir
if (homeDir) {
return path.join(homeDir, LegacyTextBackupsDirectory)
}

return undefined
}

private getFileBackupsMappingFilePath(backupsLocation: string): string {
Expand Down
@@ -1,3 +1,5 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* eslint-disable @typescript-eslint/no-explicit-any */
import { compareVersions } from 'compare-versions'
import log from 'electron-log'
import fs from 'fs'
Expand Down
Expand Up @@ -247,7 +247,7 @@ export class RemoteBridge implements CrossProcessBridge {
return this.fileBackups.migrateLegacyFileBackupsToNewStructure(newPath)
}

getUserDocumentsDirectory(): Promise<string> {
getUserDocumentsDirectory(): Promise<string | undefined> {
return this.fileBackups.getUserDocumentsDirectory()
}

Expand Down
16 changes: 12 additions & 4 deletions packages/desktop/app/javascripts/Main/Types/Paths.ts
Expand Up @@ -36,11 +36,19 @@ export const Paths = {
get userDataDir(): string {
return app.getPath('userData')
},
get homeDir(): string {
return app.getPath('home')
get homeDir(): string | undefined {
try {
return app.getPath('home')
} catch (error) {
return undefined
}
},
get documentsDir(): string {
return app.getPath('documents')
get documentsDir(): string | undefined {
try {
return app.getPath('documents')
} catch (error) {
return undefined
}
},
get tempDir(): string {
return app.getPath('temp')
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop/app/javascripts/Renderer/DesktopDevice.ts
Expand Up @@ -152,7 +152,7 @@ export class DesktopDevice extends WebOrDesktopDevice implements DesktopDeviceIn
return this.remoteBridge.wasLegacyTextBackupsExplicitlyDisabled()
}

getUserDocumentsDirectory(): Promise<string> {
getUserDocumentsDirectory(): Promise<string | undefined> {
return this.remoteBridge.getUserDocumentsDirectory()
}

Expand Down
2 changes: 1 addition & 1 deletion packages/files/src/Domain/Device/FileBackupsDevice.ts
Expand Up @@ -49,7 +49,7 @@ interface PlaintextBackupsMethods {
interface TextBackupsMethods {
getTextBackupsCount(location: string): Promise<number>
saveTextBackupData(location: string, data: string): Promise<void>
getUserDocumentsDirectory(): Promise<string>
getUserDocumentsDirectory(): Promise<string | undefined>
}

interface LegacyBackupsMethods {
Expand Down
21 changes: 14 additions & 7 deletions packages/services/src/Domain/Backups/FilesBackupService.ts
Expand Up @@ -160,14 +160,21 @@ export class FilesBackupService
}

private async automaticallyEnableTextBackupsIfPreferenceNotSet(): Promise<void> {
if (this.storage.getValue(StorageKey.TextBackupsEnabled) == undefined) {
this.storage.setValue(StorageKey.TextBackupsEnabled, true)
const location = await this.device.joinPaths(
await this.device.getUserDocumentsDirectory(),
await this.prependWorkspacePathForPath(TextBackupsDirectoryName),
)
this.storage.setValue(StorageKey.TextBackupsLocation, location)
if (this.storage.getValue(StorageKey.TextBackupsEnabled) != undefined) {
return
}

this.storage.setValue(StorageKey.TextBackupsEnabled, true)
const documentsDir = await this.device.getUserDocumentsDirectory()
if (!documentsDir) {
return
}

const location = await this.device.joinPaths(
documentsDir,
await this.prependWorkspacePathForPath(TextBackupsDirectoryName),
)
this.storage.setValue(StorageKey.TextBackupsLocation, location)
}

openAllDirectoriesContainingBackupFiles(): void {
Expand Down
3 changes: 3 additions & 0 deletions packages/services/src/Domain/HomeServer/HomeServerService.ts
Expand Up @@ -170,6 +170,9 @@ export class HomeServerService
let location = await this.getHomeServerDataLocation()
if (!location) {
const documentsDirectory = await this.desktopDevice.getUserDocumentsDirectory()
if (!documentsDirectory) {
return
}
location = `${documentsDirectory}/${this.HOME_SERVER_DATA_DIRECTORY_NAME}`
}

Expand Down