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

Update go-org & fix includes #6383

Merged
merged 2 commits into from
Oct 7, 2019
Merged

Update go-org & fix includes #6383

merged 2 commits into from
Oct 7, 2019

Conversation

niklasfasching
Copy link
Contributor

see #6322

The org mode renderer supports including other files [1]. To resolve relative
paths we need the path of the current file - either absolute or relative to the
directory hugo is run from. DocumentName is the path relative to the content
directory, not the base directory that hugo is run from - we could get the full
path by looking up the content directory from the config and merging both; or
we just make the absolute path of rendered files available in the
RenderingContext.

[1]: e.g. #+INCLUDE: ./foo.py src python includes foo.py as a python source
block.

@niklasfasching niklasfasching changed the title Go org fix include Update go-org & fix includes Oct 1, 2019
@bep
Copy link
Member

bep commented Oct 2, 2019

Hugo has this concept of a virtual file system:

  • With composable/union mounts etc.
  • With some built-in sandboxing (security motivation), most notably: Only the project (not themes) can mount files outside of the project dir, only the project has symlink support.

The first point may not be a deal-breaker, but I'm not thrilled about themes "poking around on my disk and including stuff".

So what I suggest instead is that you instead of sending the filename, send in an "OpenFilefunc. that returns aio.ReadCloser`.

@niklasfasching
Copy link
Contributor Author

niklasfasching commented Oct 2, 2019

The first point may not be a deal-breaker, but I'm not thrilled about themes "poking around on my disk and including stuff".

Could you elaborate why you say theme? This is part of go-org, i.e. the Org mode / .org renderer and should be unrelated to themes.

I understand the security concerns though and like the OpenFilefunc idea - could you give me some pointers on where I could get that from for access in renderBytes? I didn't see anything obvious on a glance...

@bep
Copy link
Member

bep commented Oct 2, 2019

This is part of go-org, i.e. the Org mode / .org renderer and should be unrelated to themes. I

When we, recently, introduced Hugo Modules, we added support for file mounts, including content from themes/theme components (including org files).

As to "Open File"

See https://godoc.org/github.com/gohugoio/hugo/helpers#PathSpec => https://godoc.org/github.com/gohugoio/hugo/hugolib/filesystems#BaseFs

If you use the Content filesystem to open the files you can just Path and relative filenames.

As to how to get that filesystem to where you need it, I'm not sure. I have plans on moving these "content renderer" into its own package once I add Goldmark.

@niklasfasching
Copy link
Contributor Author

Thx! I got the BaseFS from page.pageCommon.

Would you be fine something along this changeset? If so I'll add ReadFile to go-org and update the PR with v0.1.6.

The current ReadFile allows relative includes by checking in the content FS first but falls back to trying to read the path relative to the project root. I guess that's a bad idea security wise so maybe we should just allow the content fs...

- add support for latex fragments
- allow customization of ReadFile method for includes (#+INCLUDE: ...)
The org mode renderer supports including other files [1]. We don't want to
allow reading of arbitrary files (go-org defaults to ioutil.ReadFile [2]) but want
to make use of the FileSystem abstractions hugo provides. For starters we will
allow reading from the content directory only

[1]: e.g. `#+INCLUDE: ./foo.py src python` includes `foo.py` as a python source
block.
@niklasfasching
Copy link
Contributor Author

@bep ping. Ignore my last comment, I restricted it to just Content.Fs for now.

@bep bep merged commit 020a6fb into gohugoio:master Oct 7, 2019
@bep
Copy link
Member

bep commented Oct 7, 2019

So, I don't see the security problem in reading files from the project root. We have plenty of examples of us doing that (there is even a Work filesystem in there that points to the project root.

But there are obvious benefits to having the files live in the standard Hugo folders:

  • You can compose/combine the fs via theme components.
  • The folders are watched in server mode
  • And when a file is changed, we can look at it and do a partial rebuild

@niklasfasching niklasfasching deleted the go-org-fix-include branch October 7, 2019 17:19
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants