fix jinja2 templates #63
Conversation
fastapi_mail/config.py
Outdated
| fp = str(v) | ||
| if not os.path.exists(fp) or not os.path.isdir(fp) or not os.access(fp, os.R_OK): | ||
| raise TemplateFolderDoesNotExist( | ||
| f"{v!r} is not a valid path to an email template folder") | ||
| return v |
There was a problem hiding this comment.
hi, thank you for your PR. I think first we need to check TEMPLATE_FOLDER variable is None or not. If a TEMPLATE_FOLDER variable is none we don't check any validations. Otherwise, we validate the template path.
There was a problem hiding this comment.
Because the TEMPLATE_FOLDER variable is optional.
There was a problem hiding this comment.
Right you are, it has been changed
|
|
||
| if os.path.isdir(v) and os.access(v, os.R_OK) and validate_path(v): |
There was a problem hiding this comment.
Why you remove the validate path function? This function check for path traversal security vulnerability.
There was a problem hiding this comment.
Hi,
i had issues with it when I instantiated ConnectionConfig outside of the parent folder, in the end I had to monkey patch that function to always return True otherwise I would not be able proceed.
I'm not very familiar with the issues of path traversal, I'm going to look in to handling it with pathlib and make a new commit.
There was a problem hiding this comment.
What are the issues when you instantiate ConnectionConfig outside of the parent folder? Why the previous verification wasn't enough?
|
Take a look at this improvement, is it sufficient? I have also removed some of the |
|
Yes, you are right at first it gets a bit complicated, to have field html and body. But if you properly set it up, this won't be a big problem. Your workaround also seems to be meaningful. thank you for your additional changes. Thanks for your work.Highly appreciated your work! |
|
@kucera-lukas Thank you a lot again for your work! it is released available in 0.4.0. |
Hi,
I've been using this library for a few weeks and ran into some issues with the jinja2 integration.
This has been improved so there is proper directory validation and template engine creation is done separately via a method.
Created a separate jinja2 argument (template_body) so user knows which argument to use whether they're using jinja2 or not.