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

fix: handle paths with whitespaces #2123

Merged
merged 24 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8bdc6cb
fix: handle paths with whitespaces
EmiM Nov 28, 2023
e52adea
fix: handle paths with whitespaces
EmiM Nov 28, 2023
a0a0ffb
Merge branch 'develop' into fix/1966
EmiM Nov 28, 2023
335677a
fix: e2e test for invitation link;
EmiM Nov 28, 2023
4c7387a
fix: mock torHashedPassword in tests
vinkabuki Nov 29, 2023
6df0c5a
Merge branch 'develop' into fix/1966
EmiM Dec 1, 2023
2d293da
Merge branch 'develop' into fix/1966
EmiM Dec 4, 2023
f440117
fix: wrap tor path in double quotes to handle spaces in path; spawn t…
EmiM Dec 5, 2023
6a1ed7b
fix: change exec to execFile for killing process on windows
EmiM Dec 5, 2023
5633470
chore: update changelog
EmiM Dec 5, 2023
e7bb7e6
chore: debug e2e tests
EmiM Dec 5, 2023
8340e50
fix: wrap tor arguments in quotation marks if they're path
EmiM Dec 6, 2023
f111fa9
fix: killing hanging tor processes
EmiM Dec 6, 2023
6bed943
test: add logs for debugging purposes
EmiM Dec 6, 2023
468b8e0
Merge branch 'develop' into fix/1966
EmiM Jan 4, 2024
da7a113
Merge branch 'develop' into fix/1966
EmiM Jan 22, 2024
7071be6
Merge branch 'develop' into fix/1966
EmiM Jan 23, 2024
903760e
test: check if spawning tor needs detached mode
EmiM Jan 23, 2024
adf5d3e
Merge branch 'develop' into fix/1966
EmiM Jan 24, 2024
d534932
chore: debug
EmiM Jan 24, 2024
8857f7b
fix: kill tor processes properly; cleanup changelog
EmiM Jan 25, 2024
a4358f7
chore: cleanup
EmiM Jan 25, 2024
11b674b
chore: cleanup
EmiM Jan 25, 2024
f358ae7
fix: killing tor process tests
EmiM Jan 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
[2.1.0] - unreleased
[Unreleased]

* Handle spaces in tor process path. Run tor process in shell

[2.0.1]

* refactor: Remove SAVE_OWNER_CERTIFICATE event

Expand All @@ -21,6 +25,7 @@
* Ask push notifications runtime permission on Android app start

* Fix for multiplicating "welcome" messages when joining a community

* Fix: base jdenticon on pubkey instead of username - this way unregistered user with duplicated username will have different profile image

* Updated old logo of Linux and Windows with rounded ones
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/nest/common/test.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const torPath = torBinForPlatform()
const libPath = torDirForPlatform()
export const defaultConfigForTest = {
socketIOPort: TEST_DATA_PORT,
torBinaryPath: torBinForPlatform(),
torResourcesPath: torDirForPlatform(),
torBinaryPath: torPath,
torResourcesPath: torPath,
torControlPort: await getPort(),
options: {
env: {
Expand Down
6 changes: 4 additions & 2 deletions packages/backend/src/nest/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import { UserData } from '@quiet/types'
import createHttpsProxyAgent from 'https-proxy-agent'
import PeerId from 'peer-id'
import tmp from 'tmp'
import crypto from 'crypto'
import crypto, { sign } from 'crypto'
import { type PermsData } from '@quiet/types'
import { TestConfig } from '../const'
import logger from './logger'
import { Libp2pNodeParams } from '../libp2p/libp2p.types'
import { createLibp2pAddress, createLibp2pListenAddress, isDefined } from '@quiet/common'
import { Libp2pService } from '../libp2p/libp2p.service'
import { CertFieldsTypes, getReqFieldValue, loadCSR } from '@quiet/identity'
import { execFile } from 'child_process'

const log = logger('test')

Expand Down Expand Up @@ -127,7 +128,8 @@ export const torBinForPlatform = (basePath = '', binName = 'tor'): string => {
return basePath
}
const ext = process.platform === 'win32' ? '.exe' : ''
return path.join(torDirForPlatform(basePath), `${binName}`.concat(ext))
// Wrap path in quotes to handle spaces in path
return `"${path.join(torDirForPlatform(basePath), `${binName}`.concat(ext))}"`
}

export const torDirForPlatform = (basePath?: string): string => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ beforeEach(async () => {
],
})
.overrideProvider(TOR_PASSWORD_PROVIDER)
.useValue({ torPassword: '', torHashedPassword: '' })
.useValue({
torPassword: 'b5e447c10b0d99e7871636ee5e0839b5',
torHashedPassword: '16:FCFFE21F3D9138906021FAADD9E49703CC41848A95F829E0F6E1BDBE63',
})
.compile()

connectionsManagerService = await module.resolve(ConnectionsManagerService)
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/nest/tor/tor.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const torParamsProvider = {
LD_LIBRARY_PATH: configOptions.torResourcesPath,
HOME: os.homedir(),
},
detached: true,
// detached: true, // TODO: check if this is needed
}

return { torPath, options }
Expand Down
16 changes: 9 additions & 7 deletions packages/backend/src/nest/tor/tor.service.tor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@ import { type DirResult } from 'tmp'
import { jest } from '@jest/globals'
import { TorControlAuthType } from './tor.types'
import { TorControl } from './tor-control.service'
import crypto from 'crypto'
import { sleep } from '../common/sleep'

jest.setTimeout(200_000)

describe('TorControl', () => {
let module: TestingModule
let torService: Tor
let torControl: TorControl
let tmpDir: DirResult
let tmpAppDataPath: string

const torPassword = crypto.randomBytes(16).toString('hex')
const torPassword = 'b5e447c10b0d99e7871636ee5e0839b5'
const torHashedPassword = '16:FCFFE21F3D9138906021FAADD9E49703CC41848A95F829E0F6E1BDBE63'

beforeEach(async () => {
jest.clearAllMocks()
Expand All @@ -29,7 +31,7 @@ describe('TorControl', () => {
imports: [TestModule, TorModule],
})
.overrideProvider(TOR_PASSWORD_PROVIDER)
.useValue({ torPassword: torPassword, torHashedPassword: '' })
.useValue({ torPassword, torHashedPassword })
.overrideProvider(TOR_PARAMS_PROVIDER)
.useValue({
torPath: torBinForPlatform(),
Expand Down Expand Up @@ -148,19 +150,19 @@ describe('TorControl', () => {
expect(status).toBe(false)
})

it('should find hanging tor process and kill it', async () => {
it('should find hanging tor processes and kill them', async () => {
const processKill = jest.spyOn(process, 'kill')
await torService.init()
torService.clearHangingTorProcess()
expect(processKill).toHaveBeenCalledTimes(1)
expect(processKill).toHaveBeenCalledTimes(2) // Spawning with {shell:true} starts 2 processes so we need to kill 2 processes
})

it('should find hanging tor process and kill it if Quiet path includes space', async () => {
it('should find hanging tor processes and kill them if Quiet path includes space', async () => {
tmpDir = createTmpDir('quietTest Tmp_') // On MacOS quiet data lands in '(...)/Application Support/(...)' which caused problems with grep
tmpAppDataPath = tmpQuietDirPath(tmpDir.name)
const processKill = jest.spyOn(process, 'kill')
await torService.init()
torService.clearHangingTorProcess()
expect(processKill).toHaveBeenCalledTimes(1)
expect(processKill).toHaveBeenCalledTimes(2) // Spawning with {shell:true} starts 2 processes so we need to kill 2 processes
})
})
48 changes: 33 additions & 15 deletions packages/backend/src/nest/tor/tor.service.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import * as child_process from 'child_process'
import crypto from 'crypto'
import * as fs from 'fs'
import path from 'path'
import getPort from 'get-port'
import { removeFilesFromDir } from '../common/utils'
import { EventEmitter } from 'events'
import { ConnectionProcessInfo, SocketActionTypes, SupportedPlatform } from '@quiet/types'
import { SocketActionTypes, SupportedPlatform } from '@quiet/types'
import { Inject, OnModuleInit } from '@nestjs/common'
import { ConfigOptions, ServerIoProviderTypes } from '../types'
import { CONFIG_OPTIONS, QUIET_DIR, SERVER_IO_PROVIDER, TOR_PARAMS_PROVIDER, TOR_PASSWORD_PROVIDER } from '../const'
Expand All @@ -16,7 +15,7 @@ import Logger from '../common/logger'

export class Tor extends EventEmitter implements OnModuleInit {
socksPort: number
process: child_process.ChildProcessWithoutNullStreams | any = null
process: child_process.ChildProcessWithoutNullStreams | null = null
torDataDirectory: string
torPidPath: string
extraTorProcessParams: TorParams
Expand Down Expand Up @@ -93,7 +92,7 @@ export class Tor extends EventEmitter implements OnModuleInit {
try {
this.clearHangingTorProcess()
} catch (e) {
this.logger('Error occured while trying to clear hanging tor processes')
this.logger('Error occured while trying to clear hanging tor processes', e)
}

try {
Expand All @@ -113,7 +112,7 @@ export class Tor extends EventEmitter implements OnModuleInit {
resolve()
} catch {
this.logger('Killing tor')
await this.process.kill()
this.clearHangingTorProcess()
removeFilesFromDir(this.torDataDirectory)

// eslint-disable-next-line
Expand Down Expand Up @@ -159,11 +158,15 @@ export class Tor extends EventEmitter implements OnModuleInit {
public clearHangingTorProcess() {
const torProcessId = child_process.execSync(this.hangingTorProcessCommand()).toString('utf8').trim()
if (!torProcessId) return
this.logger(`Found tor process with pid ${torProcessId}. Killing...`)
try {
process.kill(Number(torProcessId), 'SIGTERM')
} catch (e) {
this.logger.error(`Tried killing hanging tor process. Failed. Reason: ${e.message}`)
const ids = torProcessId.split('\n') // Spawning with {shell:true} starts 2 processes
this.logger(`Found tor process(es) with pid(s) ${ids}. Killing...`)

for (const id of ids) {
try {
process.kill(Number(id.trim()))
} catch (e) {
this.logger.error(`Tried killing hanging tor process with id ${id}. Failed. Reason: ${e.message}`)
}
}
}

Expand Down Expand Up @@ -204,6 +207,10 @@ export class Tor extends EventEmitter implements OnModuleInit {
reject(new Error("Can't spawn tor - no controlPort"))
return
}
const options: child_process.SpawnOptionsWithoutStdio = {
...this.torParamsProvider.options,
shell: true,
}

this.process = child_process.spawn(
this.torParamsProvider.torPath,
Expand All @@ -215,15 +222,26 @@ export class Tor extends EventEmitter implements OnModuleInit {
'--ControlPort',
this.controlPort.toString(),
'--PidFile',
this.torPidPath,
`"${this.torPidPath}"`,
'--DataDirectory',
this.torDataDirectory,
`"${this.torDataDirectory}"`,
'--HashedControlPassword',
this.torPasswordProvider.torHashedPassword,
// ...this.torProcessParams
],
this.torParamsProvider.options
options
)
this.process.stderr.on('data', e => {
this.logger.error('Tor process. Stderr:', e)
})

this.process.on('exit', (code, signal) => {
this.logger(`Tor exited with code ${code} and signal ${signal}`)
})

this.process.on('error', err => {
this.logger.error(`Tor process. Error occurred: ${err.message}`)
})

this.process.stdout.on('data', (data: any) => {
this.logger(data.toString())
Expand Down Expand Up @@ -326,7 +344,7 @@ export class Tor extends EventEmitter implements OnModuleInit {

public async kill(): Promise<void> {
return await new Promise((resolve, reject) => {
this.logger('Killing tor...')
this.logger('Killing tor... with pid', this.process?.pid)
if (this.process === null) {
this.logger('TOR: Process is not initalized.')
resolve()
Expand All @@ -342,7 +360,7 @@ export class Tor extends EventEmitter implements OnModuleInit {
this.process?.on('error', () => {
reject(new Error('TOR: Something went wrong with killing tor process'))
})
this.process?.kill()
this.clearHangingTorProcess()
})
}
}
5 changes: 4 additions & 1 deletion packages/e2e-tests/src/tests/oneClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,14 @@ describe('One Client', () => {
const isRegisterModal = await registerModal.element.isDisplayed()

expect(isRegisterModal).toBeTruthy()
console.log('Registration - vefore typeUsername')
await registerModal.typeUsername('testuser')
console.log('Registration - before submit')
await registerModal.submit()
console.log('Registration - after submit')
})

it('User waits for the modal JoiningLoadingPanel to disappear', async () => {
it.skip('User waits for the modal JoiningLoadingPanel to disappear', async () => {
const loadingPanelCommunity = new JoiningLoadingPanel(app.driver)
const isLoadingPanelCommunity = await loadingPanelCommunity.element.isDisplayed()
expect(isLoadingPanelCommunity).toBeTruthy()
Expand Down
Loading