Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow to import template files relative to root #2

Merged
merged 8 commits into from
Mar 29, 2018

Conversation

johanjanssens
Copy link
Member

@johanjanssens johanjanssens commented Mar 28, 2018

Note: This PR required joomlatools/joomlatools-framework#151 for testing

Moved the /pages folder one level lower and make it a required folder. The minimum structure now is

  • joomlatools-pages
    • pages

The root folder can be configured in the page:// locator.

The changes allow to import templates relative to the root folder, to test use:

  • /file.html
  • /folder/file.html

Following imports are resolved against the root folder, eg joomlatools-files.

  • file.html
  • folder/file.html

Following imports are resolved relative from the folder the template is imported into.

  • ../file.html
  • ../folder/file.html

Following imports are resolved relative from the folder the template is imported into. The last allow to jump out of the root folder, haven't tested this yet, probably need to harden for this.

Examples:

<?= import('/templates/file'); ?>

Will load a partial from root, eg /joomlatools-pages/

<?= import('../file'); ?>

Will load a partial relative from the file the import is used into

Notes:

As part of the changes made in: #9 a page template no longer requires a '.html' suffix.

@johanjanssens johanjanssens added this to the 0.1 milestone Mar 28, 2018
@johanjanssens johanjanssens self-assigned this Mar 28, 2018
@johanjanssens johanjanssens requested a review from raeldc March 28, 2018 02:17
@johanjanssens
Copy link
Member Author

@raeldc Added additional check to prevent directory traversal attempts outside of the pages root.

Copy link
Contributor

@raeldc raeldc left a comment

Choose a reason for hiding this comment

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

Tested. Everything works as the ticket described. Only broke it by recursively importing files - which can be fixed on the framework level.

@johanjanssens
Copy link
Member Author

@raeldc I have added basic support for detecting recursion. Can you check if that solves it for your use-case?

@johanjanssens johanjanssens merged commit 81df488 into master Mar 29, 2018
@johanjanssens johanjanssens deleted the feature/1-import branch March 29, 2018 00:20
@raeldc raeldc mentioned this pull request Apr 24, 2018
62 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants