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.

Sign up for GitHub

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

Minimal include support for local editing #124

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Minimal include support for local editing #124

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3006705
Rudimentary support for includes during local editing
augustuswm Mar 17, 2025
a362dfc
Consolidate base path handling. Cleanup error handling
augustuswm Mar 17, 2025
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
51 changes: 45 additions & 6 deletions app/services/rfd.local.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,29 @@ function findLineStartingWith(content: string, prefixRegex: string): string | un

export function fetchLocalRfd(num: number): LocalRfd {
try {
const numStr = num.toString().padStart(4, '0')
const buffer = fs.readFileSync(`${localRepo}/rfd/${numStr}/README.adoc`)
const content = buffer.toString()
const basePath = contentsPath(num)
const buffer = fs.readFileSync(`${basePath}/README.adoc`)
let content = buffer.toString()

// we used to parse the whole document for state and title, but this is
// dramatically faster for live reload and seems to work fine
const state = findLineStartingWith(content, ':state: ') || 'unknown'

let title = findLineStartingWith(content, '= ') || 'Title Not Found'
title = title.replace(`RFD ${parseInt(numStr)}`, '')
title = title.replace(`RFD ${num}`, '')

// Perform basic replacement for included files
const pattern = /^include::(.*)\[\]$/gm
for (let match of content.matchAll(pattern) || []) {
const replacementContents = resolveInclude(basePath, match[1])
if (replacementContents) {
content = content.replace(match[0], replacementContents.toString())
} else {
console.warn(
`Unable to find valid file to include for ${match[0]}. Skipping instead`,
)
}
}

return {
number: num,
Expand All @@ -58,9 +71,23 @@ export function fetchLocalRfd(num: number): LocalRfd {
}
}

function resolveInclude(basePath: string, path: string) {
checkForRelativePath(path)

const fullPath = `${basePath}/${path}`
try {
return fs.readFileSync(fullPath)
} catch (e) {
console.error('Failed to load content for include')
return null
}
}

export function fetchLocalImage(num: number, src: string): Buffer | null {
const numStr = num.toString().padStart(4, '0')
const imagePath = `${localRepo}/rfd/${numStr}/${src}`
checkForRelativePath(src)

const basePath = contentsPath(num)
const imagePath = `${basePath}/${src}`
try {
return fs.readFileSync(imagePath)
} catch (e) {
Expand Down Expand Up @@ -88,3 +115,15 @@ export function fetchLocalRfds(): LocalRfd[] {

return rfds
}

function contentsPath(num: number): string {
const numStr = num.toString().padStart(4, '0')
return `${localRepo}/rfd/${numStr}`
}

function checkForRelativePath(path: string) {
if (path.includes('..')) {
console.error('Refusing to load file with a relative path part', path)
throw new Error('Path must not include any relative path parts')
Copy link
Contributor

@david-crespo david-crespo Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd log the bad path here and say "include path", not just path, to make it easier to figure out the problem. Might also say "no directory traversal with .." instead of "no relative path parts", I'm not sure the latter is recognizable enough. And as far as I can tell, you actually require the path itself to be relative to the RFD dir. So something like:

throw new Error(`Directory traversal with ".." is not allowed in include:: path. Bad path is ${path}. Included path must be relative to RFD dir ${localRepo}/rfd/${numStr}.`)

}
}
Loading