Eleventy: Paths are not stitched together correctly

[kleinfreund]

TL;DR: When joining paths in Eleventy, the pattern parentDir + '/' + childDir is used. This can cause problems when the components of the operation have leading and trailing slashes. Paths in Eleventy need to be properly joined together and normalized to avoid problems.

---

Background

Working around Eleventy’s nested directories, I found that setting dir.includes to the empty string (i.e. "") allows one to include from across the whole input directory instead of just from the includes directory. Setting the includes directory like this effectively makes the input directory the includes directory. Almost.

When setting Eleventy’s directories via the configuration, the resulting paths are just concatenated. This is no problem most of the time. In my case, Eleventy considers dir.input the input directory and dir.input/ the includes directory which most of the time will resolve to the same directory.

I just found an exception: Pagination. Instead of creating a page ULR like /1/, I currently end up with //1/ which browsers treat as an absolute URL with an unspecified scheme. It’s actually pretty interesting: //1/ is resolved to the URL 0.0.0.1 because the part after // is a number and used as the last part for the address 0.0.0.1.

Test Cases

| dir.input | dir.includes | Current | Expected |
|-----------|-----------------|-----------------------|------------------|
| 'src' | '_includes' | 'src/includes' | 'src/includes' |
| 'src' | '_includes/' | 'src/includes/' | 'src/includes' |
| 'src' | '/_includes' | 'src//includes' | 'src/includes' |
| 'src' | './_includes' | 'src/./includes' | 'src/includes' |
| 'src' | '//_includes' | 'src///includes' | 'src/includes' |
| 'src' | '' | 'src/' | 'src' |
| 'src' | '/' | 'src//' | 'src' |
| 'src' | '.' | 'src/.' | 'src' |
| 'src' | './' | 'src/./' | 'src' |
| 'src' | '../' | 'src/../' | Should error |
| 'src' | '../../' | 'src/../../' | Should error |

Proposed rules for joining paths

• Double-dot (..) segments are problematic and should either produce an error or a warning. Personally, I would flat-out forbid any usage of double-dot segments. Users could still use symbolic links to work around this limitation. Otherwise, escaping Eleventy’s realm (i.e. the project directory) just asks for trouble. I don’t want Eleventy to attempt converting my home directory into a website.
• Store paths without any trailing slashes. The slash can always be appended if necessary. This gives users the control.
• Redundant segments should be removed. When using Node.js’ path.join method, this is done automatically.

Possible change to EleventyFiles.js line 21:

- this.includesDir = this.inputDir + "/" + this.config.dir.includes;
+ this.includesDir = path.join(this.inputDir, this.config.dir.includes).replace(/\/+$/, "");

Possible change to TemplateData.js:

- this.dataDir =
-   inputDir +
-   "/" +
-   (this.config.dir.data !== "." ? this.config.dir.data : "");
+ this.dataDir = path.join(inputDir, this.config.dir.data).replace(/\/+$/, "");

Doing a quick search across the project reveals that there are a lot of instances where paths are concatenated with the pattern parentDir + "/" + childDir. It could be worthwhile to review these as well.

I’d be happy to work on this and submit a pull request if this is something you want.

[zachleat]

I need a bit more time to review this (it looks great on first glance) but I did want to leave a quick comment to make sure you’ve seen TemplatePath.normalize function https://github.com/11ty/eleventy/blob/master/src/TemplatePath.js#L62 I think there is some overlap there in what you’re suggesting.

If not, I’d love a method that works similarly, takes a bunch of args and removes duplicate slashes, etc. Makes for easy testing too.

One other thing that comes to mind is the Template functions used to output the url, which should definitely keep trailing slashes per discussion in #213.

[kleinfreund]

I haven’t seen that normalizing function. I thought about having a utility normalizing function for Eleventy paths that would most likely call path.join like TemplatePath.normalize does.

Note that the comment there is not correct. path.join doesn’t remove trailing slashes.

Also, very valid point with regards to permalinks: If removing a trailing slash there should change Eleventy’s general behavior, we should document it and take care of it.

We can also make this two separate changes: First, normalizing includes and data directories. Second, evaluating all other instances of path concatenation and fixing potential problem cases.

[kleinfreund]

186 is somewhat related. Both the input and the output directory should be located inside Eleventy’s project/command directory. Also, they should not be descendants of each other.

[rendall]

The nodejs path library is pretty nifty. In particular, the 'normalize' function would smooth all of those paths out without extra external dependencies.

https://nodejs.org/api/path.html#path_path_normalize_path

[kleinfreund]

@rendall The proposed change above uses path.join which normalizes the paths already.

[kleinfreund]

@zachleat What do you think now? I think we should implement a utility function joinedPath which takes an arbitrary amount of path segments, joins them together and normalizes them. This might be just calling path.join, but allows to transition to a different mechanism later on. Every Eleventy directory/path should only ever be stored after calling this function.

Also, should we completely disallow navigating outside Eleventy’s realm via double-dot segments (i.e. ../) and symbolic links? I know Jekyll does this for security reasons, probably related to being used by GitHub Pages.

[zachleat]

Yes I definitely think we need some cleanup here @kleinfreund. Note Eleventy’s TemplatePath class has a lot of this already built in, it just needs to be applied consistently. Check out the tests to see how it works https://github.com/11ty/eleventy/blob/master/test/TemplatePathTest.js#L1