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 regression in --allow-local-files option with Snapd Chromium #283

Merged
merged 4 commits into from
Sep 12, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## [Unreleased]

### Fixed

- Fix regression in `--allow-local-files` option with Snapd Chromium ([#201](https://github.com/marp-team/marp-cli/issues/201), [#283](https://github.com/marp-team/marp-cli/pull/283))

## v0.21.0 - 2020-08-20

### Added
Expand Down
34 changes: 15 additions & 19 deletions src/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,6 @@ export class Converter {
baseFile: File,
processer: (page: puppeteer.Page, uri: string) => Promise<T>
) {
const { executablePath } = generatePuppeteerLaunchArgs()

const tmpFile: File.TmpFileInterface | undefined = await (() => {
if (!this.options.allowLocalFiles) return undefined

Expand All @@ -358,12 +356,11 @@ export class Converter {
)

// Snapd Chromium cannot access from sandbox container to user-land `/tmp`
// directory so create tmp file to home directory if executed Chromium was
// placed in `/snap`.
const home =
process.platform === 'linux' && executablePath?.startsWith('/snap/')

return baseFile.saveTmpFile({ home, extension: '.html' })
// directory so create tmp file to home directory if in Linux.
return baseFile.saveTmpFile({
home: process.platform === 'linux',
extension: '.html',
})
})()

const uri = await (async () => {
Expand All @@ -377,29 +374,28 @@ export class Converter {
try {
const browser = await Converter.runBrowser()
const page = await browser.newPage()
const {
missingFileSet: missingTracker,
failedFileSet: failedTracker,
} = this.trackFailedLocalFileAccess(page)
const { missingFileSet, failedFileSet } = this.trackFailedLocalFileAccess(
page
)

try {
return await processer(page, uri)
} finally {
if (missingTracker.size > 0) {
if (missingFileSet.size > 0) {
warn(
`${missingTracker.size > 1 ? 'Some of t' : 'T'}he local file${
missingTracker.size > 1 ? 's are' : ' is'
`${missingFileSet.size > 1 ? 'Some of t' : 'T'}he local file${
missingFileSet.size > 1 ? 's are' : ' is'
} missing and will be ignored. Make sure the file path${
missingTracker.size > 1 ? 's are' : ' is'
missingFileSet.size > 1 ? 's are' : ' is'
} correct.`
)
}
if (failedTracker.size > 0) {
if (failedFileSet.size > 0) {
warn(
`Marp CLI has detected accessing to local file${
failedTracker.size > 1 ? 's' : ''
failedFileSet.size > 1 ? 's' : ''
}. ${
failedTracker.size > 1 ? 'They are' : 'That is'
failedFileSet.size > 1 ? 'They are' : 'That is'
} blocked by security reason. Instead we recommend using assets uploaded to online. (Or you can use ${chalk.yellow(
'--allow-local-files'
)} option if you are understood of security risk)`
Expand Down
7 changes: 3 additions & 4 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,9 @@ export class File {
async saveTmpFile(
opts: { extension?: string; home?: boolean } = {}
): Promise<File.TmpFileInterface> {
const tmp: string = await tmpNamePromise({
dir: opts.home ? os.homedir() : undefined,
postfix: opts.extension,
})
let tmp: string = await tmpNamePromise({ postfix: opts.extension })

if (opts.home) tmp = path.join(os.homedir(), path.basename(tmp))

await this.saveToFile(tmp)

Expand Down