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

Prevent recursive mkdir to the root path while saving #443

Merged
merged 4 commits into from
Apr 12, 2022
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 .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ module.exports = {
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/explicit-function-return-type': 'off',
'@typescript-eslint/explicit-module-boundary-types': 'off',
'@typescript-eslint/consistent-type-assertions': [
'error',
{ assertionStyle: 'as' },
],
},
},
],
Expand Down
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

- Cannot output the conversion result into the drive root ([#442](https://github.com/marp-team/marp-cli/issues/442), [#443](https://github.com/marp-team/marp-cli/pull/443))

### Changed

- Upgrade Marpit to [v2.2.4](https://github.com/marp-team/marpit/releases/tag/v2.2.4) ([#441](https://github.com/marp-team/marp-cli/pull/441))
Expand Down
9 changes: 6 additions & 3 deletions src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,12 @@ export class File {
}

private async saveToFile(savePath: string = this.path) {
await fs.promises.mkdir(path.dirname(path.resolve(savePath)), {
recursive: true,
})
const directory = path.dirname(path.resolve(savePath))

if (path.dirname(directory) !== directory) {
await fs.promises.mkdir(directory, { recursive: true })
}

await fs.promises.writeFile(savePath, this.buffer!) // eslint-disable-line @typescript-eslint/no-non-null-assertion
}

Expand Down
4 changes: 2 additions & 2 deletions src/server/server-index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export const showAllKey = 'marp-cli-show-all'

export default function serverIndex() {
const showAll = <HTMLInputElement>document.getElementById('show-all')
const index = <HTMLElement>document.getElementById('index')
const showAll = document.getElementById('show-all') as HTMLInputElement
const index = document.getElementById('index') as HTMLElement

const applyShowAll = (state: boolean) => {
showAll.checked = state
Expand Down
4 changes: 2 additions & 2 deletions src/templates/bespoke/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ const bespokeNavigation =
if (elm?.parentElement) detectScrollable(elm.parentElement, dir)
}

if (e.deltaX !== 0) detectScrollable(<HTMLElement>e.target, 'X')
if (e.deltaY !== 0) detectScrollable(<HTMLElement>e.target, 'Y')
if (e.deltaX !== 0) detectScrollable(e.target as HTMLElement, 'X')
if (e.deltaY !== 0) detectScrollable(e.target as HTMLElement, 'Y')
if (scrollable) return

e.preventDefault()
Expand Down
3 changes: 0 additions & 3 deletions test/__mocks__/mkdirp.ts

This file was deleted.

79 changes: 53 additions & 26 deletions test/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ import { WatchNotifier } from '../src/watcher'

const puppeteerTimeoutMs = 60000

let mkdirSpy: jest.SpiedFunction<typeof fs.promises.mkdir>

jest.mock('fs')

beforeEach(() => {
mkdirSpy = jest.spyOn(fs.promises, 'mkdir').mockImplementation()
})

afterAll(() => Converter.closeBrowser())
afterEach(() => jest.restoreAllMocks())

Expand Down Expand Up @@ -58,7 +64,7 @@ describe('Converter', () => {
globalDirectives: { theme: 'default' },
imageScale: 2,
lang: 'fr',
options: <Options>{ html: true },
options: { html: true } as Options,
server: false,
template: 'test-template',
templateOption: {},
Expand Down Expand Up @@ -485,12 +491,12 @@ transition:
describe('#convertFile', () => {
it('rejects Promise when specified file is not found', () =>
expect(
(instance() as any).convertFile(new File('_NOT_FOUND_MARKDOWN_'))
instance().convertFile(new File('_NOT_FOUND_MARKDOWN_'))
).rejects.toBeTruthy())

it('converts markdown file and save as html file by default', async () => {
const write = (<any>fs).__mockWriteFile()
await (<any>instance()).convertFile(new File(onePath))
const write = (fs as any).__mockWriteFile()
await instance().convertFile(new File(onePath))

expect(write).toHaveBeenCalledWith(
`${onePath.slice(0, -3)}.html`,
Expand All @@ -500,9 +506,9 @@ transition:
})

it('converts markdown file and save to specified path when output is defined', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const output = './specified.html'
await (<any>instance({ output })).convertFile(new File(twoPath))
await instance({ output }).convertFile(new File(twoPath))

expect(write).toHaveBeenCalledWith(
output,
Expand All @@ -511,19 +517,40 @@ transition:
)
})

it('tries to create the directory of output file when saving', async () => {
const write = (fs as any).__mockWriteFile()
const output = path.resolve(__dirname, '__test_dir__/out.html')

await instance({ output }).convertFile(new File(twoPath))

expect(write).toHaveBeenCalled()
expect(mkdirSpy).toHaveBeenCalledWith(
path.resolve(__dirname, '__test_dir__'),
{ recursive: true }
)
})

it('does not try to create the directory of output file when saving to the root', async () => {
const write = (fs as any).__mockWriteFile()
const output = '/out.html'

await instance({ output }).convertFile(new File(twoPath))

expect(write).toHaveBeenCalled()
expect(mkdirSpy).not.toHaveBeenCalled()
})

it('converts markdown file but not save when output is stdout', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const stdout = jest.spyOn(process.stdout, 'write').mockImplementation()

const output = '-'
const ret = await (<any>instance({ output })).convertFile(
new File(threePath)
)
const ret = await instance({ output }).convertFile(new File(threePath))

expect(write).not.toHaveBeenCalled()
expect(stdout).toHaveBeenCalledTimes(1)
expect(ret.file.path).toBe(threePath)
expect(ret.newFile.type).toBe(FileType.StandardIO)
expect(ret.newFile?.type).toBe(FileType.StandardIO)
})

describe('when convert type is PDF', () => {
Expand All @@ -533,7 +560,7 @@ transition:
it(
'converts markdown file into PDF',
async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const opts = { output: 'test.pdf' }
const ret = await pdfInstance(opts).convertFile(new File(onePath))
const pdf: Buffer = write.mock.calls[0][1]
Expand All @@ -551,7 +578,7 @@ transition:
it(
'assigns meta info thorugh pdf-lib',
async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await pdfInstance({
output: 'test.pdf',
Expand Down Expand Up @@ -580,7 +607,7 @@ transition:
async () => {
const file = new File(onePath)

const fileCleanup = jest.spyOn(<any>File.prototype, 'cleanup')
const fileCleanup = jest.spyOn(File.prototype as any, 'cleanup')
const fileSave = jest
.spyOn(File.prototype, 'save')
.mockImplementation()
Expand Down Expand Up @@ -614,7 +641,7 @@ transition:
it(
'assigns presenter notes as annotation of PDF',
async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await pdfInstance({
output: 'test.pdf',
Expand All @@ -637,7 +664,7 @@ transition:
)

it('sets a comment author to notes if set author global directive', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await pdfInstance({
output: 'test.pdf',
Expand Down Expand Up @@ -673,7 +700,7 @@ transition:
let write: jest.Mock

beforeEach(() => {
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

const converter = (opts: Partial<ConverterOption> = {}) =>
Expand Down Expand Up @@ -781,7 +808,7 @@ transition:
let write: jest.Mock

beforeEach(() => {
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

it(
Expand Down Expand Up @@ -851,7 +878,7 @@ transition:
let write: jest.Mock

beforeEach(() => {
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

it(
Expand Down Expand Up @@ -929,7 +956,7 @@ transition:
pages: true,
type: ConvertType.png,
})
write = (<any>fs).__mockWriteFile()
write = (fs as any).__mockWriteFile()
})

it(
Expand All @@ -950,9 +977,9 @@ transition:
const notesInstance = (opts: Partial<ConverterOption> = {}) =>
instance({ ...opts, type: ConvertType.notes })

const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const output = './specified.txt'
const ret = await (<any>notesInstance({ output })).convertFile(
const ret = await notesInstance({ output }).convertFile(
new File(threePath)
)
const notes: Buffer = write.mock.calls[0][1]
Expand All @@ -969,9 +996,9 @@ transition:
const notesInstance = (opts: Partial<ConverterOption> = {}) =>
instance({ ...opts, type: ConvertType.notes })

const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const output = './specified.txt'
const ret = await (<any>notesInstance({ output })).convertFile(
const ret = await notesInstance({ output }).convertFile(
new File(onePath)
)
const notes: Buffer = write.mock.calls[0][1]
Expand All @@ -991,7 +1018,7 @@ transition:
describe('#convertFiles', () => {
describe('with multiple files', () => {
it('converts passed files', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()

await instance().convertFiles([new File(onePath), new File(twoPath)])
expect(write).toHaveBeenCalledTimes(2)
Expand All @@ -1008,7 +1035,7 @@ transition:
).rejects.toBeInstanceOf(CLIError))

it('converts passed files when output is stdout', async () => {
const write = (<any>fs).__mockWriteFile()
const write = (fs as any).__mockWriteFile()
const stdout = jest.spyOn(process.stdout, 'write').mockImplementation()
const files = [new File(onePath), new File(twoPath)]

Expand Down
17 changes: 8 additions & 9 deletions test/marp-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const runForObservation = async (argv: string[]) => {
}

jest.mock('fs')
jest.mock('mkdirp')
jest.mock('../src/preview')
jest.mock('../src/watcher', () => jest.createMockFromModule('../src/watcher'))

Expand Down Expand Up @@ -225,7 +224,7 @@ describe('Marp CLI', () => {
const files = assetFn('_files')

let writeFile: jest.Mock
beforeEach(() => (writeFile = (<any>fs).__mockWriteFile()))
beforeEach(() => (writeFile = (fs as any).__mockWriteFile()))

it('converts files in specified dir', async () => {
jest.spyOn(cli, 'info').mockImplementation()
Expand Down Expand Up @@ -373,7 +372,7 @@ describe('Marp CLI', () => {
info = jest.spyOn(cli, 'info')

info.mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()
})

describe('when passed value is theme name', () => {
Expand All @@ -396,7 +395,7 @@ describe('Marp CLI', () => {
const { css } = (await convert.mock.results[0].value).rendered
expect(css).toContain('/* @theme a */')

const converter = <Converter>convert.mock.instances[0]
const converter: Converter = convert.mock.instances[0]
const { themeSet } = converter.options
const theme = themeSet.themes.get(cssFile)

Expand Down Expand Up @@ -442,7 +441,7 @@ describe('Marp CLI', () => {
observeSpy = jest.spyOn(ThemeSet.prototype, 'observe')

jest.spyOn(cli, 'info').mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()
})

describe('with specified single file', () => {
Expand Down Expand Up @@ -556,7 +555,7 @@ describe('Marp CLI', () => {
await marpCli(cmd)
expect(cvtFiles).toHaveBeenCalled()

return <any>cvtFiles.mock.instances[0]
return cvtFiles.mock.instances[0] as any
} finally {
cvtFiles.mockRestore()
cliInfo.mockRestore()
Expand All @@ -565,7 +564,7 @@ describe('Marp CLI', () => {

it('converts file', async () => {
const cliInfo = jest.spyOn(cli, 'info').mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()

expect(await marpCli([onePath])).toBe(0)

Expand Down Expand Up @@ -765,7 +764,7 @@ describe('Marp CLI', () => {
describe('with -w option', () => {
it('starts watching by Watcher.watch()', async () => {
jest.spyOn(cli, 'info').mockImplementation()
;(<any>fs).__mockWriteFile()
;(fs as any).__mockWriteFile()

await runForObservation([onePath, '-w'])
expect(Watcher.watch).toHaveBeenCalledWith([onePath], expect.anything())
Expand Down Expand Up @@ -1071,7 +1070,7 @@ describe('Marp CLI', () => {
.mockResolvedValue(Buffer.from('# markdown'))

// reset cached stdin buffer
;(<any>File).stdinBuffer = undefined
;(File as any).stdinBuffer = undefined
})

it('converts markdown came from stdin and outputs to stdout', async () => {
Expand Down
4 changes: 2 additions & 2 deletions test/server/server-index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ describe('JavaScript for server index', () => {
<ul id="index"></ul>
`.trim()

index = <HTMLUListElement>document.getElementById('index')
showAll = <HTMLInputElement>document.getElementById('show-all')
index = document.getElementById('index') as HTMLUListElement
showAll = document.getElementById('show-all') as HTMLInputElement
})

const checkShowAll = (state: boolean) => {
Expand Down
2 changes: 1 addition & 1 deletion test/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('ThemeSet', () => {
expect(themeSet.onThemeUpdated).toHaveBeenCalledWith('testA2.md')

// It does no longer trigger after #unobserve
;(<jest.Mock>themeSet.onThemeUpdated).mockClear()
;(themeSet.onThemeUpdated as jest.Mock).mockClear()
themeSet.unobserve('testA.md')

await themeSet.load(themeA)
Expand Down
Loading