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

Output warning if detected blocking local resources while rendering by Chrome #98

Merged
merged 8 commits into from
May 25, 2019
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]

### Added

- Output warning if detected blocking local resources while rendering by Chrome ([#84](https://github.com/marp-team/marp-cli/issues/84), [#98](https://github.com/marp-team/marp-cli/pull/98))

## v0.9.2 - 2019-05-10

### Added
Expand Down
2 changes: 2 additions & 0 deletions jest.setup.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { enterTestMode } = require('carlo')

enterTestMode()

jest.mock('wrap-ansi')
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@
"rollup-plugin-url": "^2.2.1",
"sass": "^1.20.1",
"screenfull": "^4.2.0",
"strip-ansi": "^5.2.0",
"stylelint": "^10.0.1",
"stylelint-config-prettier": "^5.1.0",
"stylelint-config-standard": "^18.3.0",
Expand Down Expand Up @@ -137,8 +136,10 @@
"puppeteer-core": "^1.15.0",
"rimraf": "^2.6.3",
"serve-index": "^1.9.1",
"strip-ansi": "^5.2.0",
"terminate": "^2.1.2",
"tmp": "^0.1.0",
"wrap-ansi": "^5.1.0",
"ws": "^7.0.0",
"yargs": "^13.2.2"
},
Expand Down
34 changes: 28 additions & 6 deletions src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,42 @@
import chalk from 'chalk'
import stripAnsi from 'strip-ansi'
import wrapAnsi from 'wrap-ansi'
import { terminalWidth } from 'yargs'

interface CLIOption {
singleLine?: boolean
}

const width: number = terminalWidth() || 80

const messageBlock = (
kind: string,
message: string,
opts: CLIOption
): string => {
const indent = stripAnsi(kind).length + 1
const target = opts.singleLine ? message : wrapAnsi(message, width - indent)

return `${kind} ${target.split('\n').join(`\n${' '.repeat(indent)}`)}`
}

let silent = false

export function silence(value: boolean) {
silent = value
}

export function info(message: string): void {
export function info(message: string, opts: CLIOption = {}): void {
// Use console.warn to output into stderr
if (!silent) console.warn(`${chalk.bgCyan.black('[ INFO ]')} ${message}`)
if (!silent)
console.warn(messageBlock(chalk.bgCyan.black('[ INFO ]'), message, opts))
}

export function warn(message: string): void {
if (!silent) console.warn(`${chalk.bgYellow.black('[ WARN ]')} ${message}`)
export function warn(message: string, opts: CLIOption = {}): void {
if (!silent)
console.warn(messageBlock(chalk.bgYellow.black('[ WARN ]'), message, opts))
}

export function error(message: string): void {
console.error(`${chalk.bgRed.white('[ ERROR ]')} ${message}`)
export function error(message: string, opts: CLIOption = {}): void {
console.error(messageBlock(chalk.bgRed.white('[ ERROR ]'), message, opts))
}
39 changes: 34 additions & 5 deletions src/converter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Marp, MarpOptions } from '@marp-team/marp-core'
import { MarpOptions } from '@marp-team/marp-core'
import { Marpit, MarpitOptions } from '@marp-team/marpit'
import chalk from 'chalk'
import * as chromeFinder from 'chrome-launcher/dist/chrome-finder'
import puppeteer from 'puppeteer-core'
import { URL } from 'url'
import { silence, warn } from './cli'
import { Engine } from './engine'
import metaPlugin from './engine/meta-plugin'
Expand Down Expand Up @@ -89,7 +91,9 @@ export class Converter {
lang,
readyScript,
base:
isFile && type !== ConvertType.html ? file!.absolutePath : undefined,
isFile && type !== ConvertType.html
? file!.absoluteFileScheme
: undefined,
notifyWS:
isFile && this.options.watch && type === ConvertType.html
? await notifier.register(file!.absolutePath)
Expand Down Expand Up @@ -225,9 +229,9 @@ export class Converter {
const tmpFile: File.TmpFileInterface | undefined = await (() => {
if (!this.options.allowLocalFiles) return undefined

const warning = `Insecure local file accessing is enabled for conversion of ${file.relativePath()}.`
warn(warning)

warn(
`Insecure local file accessing is enabled for conversion of ${file.relativePath()}.`
)
return file.saveTmpFile('.html')
})()

Expand All @@ -238,17 +242,42 @@ export class Converter {
try {
const browser = await Converter.runBrowser()
const page = await browser.newPage()
const tracker = this.trackFailedLocalFileAccess(page)

try {
return await process(page, uri)
} finally {
if (tracker.size > 0) {
warn(
`Marp CLI has detected accessing to local file${
tracker.size > 1 ? 's' : ''
}. ${
tracker.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)`
)
}
await page.close()
}
} finally {
if (tmpFile) await tmpFile.cleanup()
}
}

private trackFailedLocalFileAccess(page: puppeteer.Page): Set<string> {
const failedFileSet = new Set<string>()

page.on('requestfailed', (req: puppeteer.Request) => {
try {
const url = new URL(req.url())
if (url.protocol === 'file:') failedFileSet.add(url.href)
} catch (e) {}
})

return failedFileSet
}

static async closeBrowser() {
if (Converter.browser) await Converter.browser.close()
}
Expand Down
11 changes: 10 additions & 1 deletion src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import getStdin from 'get-stdin'
import globby from 'globby'
import mkdirp from 'mkdirp'
import path from 'path'
import * as url from 'url'
import { tmpName } from 'tmp'
import { promisify } from 'util'

Expand Down Expand Up @@ -30,10 +31,18 @@ export class File {
this.path = filepath
}

get absolutePath() {
get absolutePath(): string {
return path.resolve(this.path)
}

get absoluteFileScheme(): string {
if (url.pathToFileURL)
return url.pathToFileURL(this.absolutePath).toString()

// Fallback for Node < v10.12.0
return `file://${path.posix.resolve(this.path)}`
}

convert(output: string | false | undefined, extension: string): File {
switch (output) {
case undefined:
Expand Down
3 changes: 2 additions & 1 deletion src/marp-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ export default async function(argv: string[] = []): Promise<number> {
cli.info(
`${fn(i, '<stdin>')} ${
o.type === FileType.Null ? 'processed.' : `=> ${fn(o, '<stdout>')}`
}`
}`,
{ singleLine: true }
)
}

Expand Down
1 change: 1 addition & 0 deletions test/__mocks__/wrap-ansi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = jest.fn().mockImplementation(m => m) // No ops
10 changes: 8 additions & 2 deletions test/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { MarpitOptions } from '@marp-team/marpit'
import fs from 'fs'
import os from 'os'
import path from 'path'
import { URL } from 'url'
import { Converter, ConvertType, ConverterOption } from '../src/converter'
import { CLIError } from '../src/error'
import { File, FileType } from '../src/file'
Expand Down Expand Up @@ -182,9 +183,14 @@ describe('Converter', () => {
context(`with ${type} convert type`, () => {
it('adds <base> element with specified base path from passed file', async () => {
const converter = instance({ type })

const { result } = await converter.convert(md, dummyFile)
expect(result).toContain(`<base href="${process.cwd()}">`)

const matched = result.match(/<base href="(.+?)">/)
expect(matched).toBeTruthy()

const url = new URL((matched && matched[1]) as string)
expect(url.protocol).toBe('file:')
expect(path.resolve(url.pathname)).toBe(path.resolve(process.cwd()))
})
})
}
Expand Down
26 changes: 13 additions & 13 deletions test/marp-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe('Marp CLI', () => {
serverStart.mockResolvedValue(0)

await marpCli(['--input-dir', files, '--server'])
expect(info).toBeCalledWith(
expect(info.mock.calls.map(([m]) => m)).toContainEqual(
expect.stringContaining('http://localhost:8080/')
)
expect(serverStart).toBeCalledTimes(1)
Expand Down Expand Up @@ -276,7 +276,7 @@ describe('Marp CLI', () => {

await run()
expect(Preview.prototype.open).not.toBeCalled()
expect(warn).toBeCalledWith(
expect(warn.mock.calls.map(([m]) => m)).toContainEqual(
expect.stringContaining('Preview option was ignored')
)
})
Expand Down Expand Up @@ -356,7 +356,7 @@ describe('Marp CLI', () => {
const args = [assetFn('_files/1.md'), '--theme', themesPath]

expect(await marpCli(args)).toBe(1)
expect(cliError).toHaveBeenCalledWith(
expect(cliError.mock.calls.map(([m]) => m)).toContainEqual(
expect.stringContaining('Directory cannot pass to theme option')
)
expect(info).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -470,12 +470,10 @@ describe('Marp CLI', () => {
;(<any>fs).__mockWriteFile()

expect(await marpCli([onePath])).toBe(0)
expect(cliInfo).toHaveBeenCalledWith(
expect.stringContaining('1 markdown')
)
expect(cliInfo).toHaveBeenCalledWith(
expect.stringMatching(/1\.md => .+1\.html/)
)

const logs = cliInfo.mock.calls.map(([m]) => m)
expect(logs).toContainEqual(expect.stringContaining('1 markdown'))
expect(logs).toContainEqual(expect.stringMatching(/1\.md => .+1\.html/))
})

it('prints error and return error code when CLIError is raised', async () => {
Expand All @@ -487,7 +485,9 @@ describe('Marp CLI', () => {
.mockImplementation(() => Promise.reject(new CLIError('FAIL', 123)))

expect(await marpCli([onePath])).toBe(123)
expect(cliError).toHaveBeenCalledWith(expect.stringContaining('FAIL'))
expect(cliError.mock.calls.map(([m]) => m)).toContainEqual(
expect.stringContaining('FAIL')
)
})

context('with --pdf option', () => {
Expand Down Expand Up @@ -698,7 +698,7 @@ describe('Marp CLI', () => {
.mockImplementation(() => [])

expect(await marpCli([assetFn('_files')])).toBe(0)
expect(cliInfo).toHaveBeenCalledWith(
expect(cliInfo.mock.calls.map(([m]) => m)).toContainEqual(
expect.stringContaining('4 markdowns')
)
})
Expand Down Expand Up @@ -775,7 +775,7 @@ describe('Marp CLI', () => {

it('converts markdown came from stdin and outputs to stdout', async () => {
expect(await marpCli()).toBe(0)
expect(cliInfo).toHaveBeenCalledWith(
expect(cliInfo.mock.calls.map(([m]) => m)).toContainEqual(
expect.stringContaining('<stdin> => <stdout>')
)
expect(stdout).toHaveBeenCalledWith(expect.any(Buffer))
Expand All @@ -786,7 +786,7 @@ describe('Marp CLI', () => {
jest.spyOn(console, 'error').mockImplementation()

expect(await marpCli(['--stdin=false'])).toBe(0)
expect(cliInfo).not.toHaveBeenCalledWith(
expect(cliInfo.mock.calls.map(([m]) => m)).not.toContainEqual(
expect.stringContaining('<stdin> => <stdout>')
)
expect(stdout).not.toHaveBeenCalledWith(expect.any(Buffer))
Expand Down
9 changes: 9 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7923,6 +7923,15 @@ wrap-ansi@^2.0.0:
string-width "^1.0.1"
strip-ansi "^3.0.1"

wrap-ansi@^5.1.0:
version "5.1.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-5.1.0.tgz#1fd1f67235d5b6d0fee781056001bfb694c03b09"
integrity sha512-QC1/iN/2/RPVJ5jYK8BGttj5z83LmSKmvbvrXPNCLZSEb32KKVDJDl/MOt2N01qU2H/FkzEa9PKto1BqDjtd7Q==
dependencies:
ansi-styles "^3.2.0"
string-width "^3.0.0"
strip-ansi "^5.0.0"

wrappy@1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f"
Expand Down