-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
recognize a template instance and don't bother escaping it #108
Comments
Yeah, this is a bit tricky. It's been part of the contract that A somewhat similar issue comes up with non-stringy types (like numbers) which ideally you wouldn't escape for performance reasons. One way this maybe could work is if there is a crate-internal trait |
Yeah, specialization does seem it will/would be the "right" way to do this. I'm not sure how your crate-internal Unless you were to restrict the template expansion to only accept expressions with type Actually, stepping back, couldn't you just use |
I've been wondering if you'd be open to a breaking change on this, obviously with an accompanying version bump? The more I look at this, it seems to be a kind of type safety bug. Specifically, askama fails to distinguish the "type" of a string, as in whether it is HTML or plain Unicode, or LaTeX or whatever. And it neither makes this distinction at compile time nor at runtime, which means that if you ever miss a "safe" you get incorrect escaping, and if you ever have an extra "safe" then you may have a security be hole at worst and a poor display at best. When I think of the problem in this way it seems like I'd want to put the error at compile time, which means a breaking change. I have some ideas for a new API but want to hear first whether in principle you'd be open to a breaking change. |
Definitely not opposed to API change per se, but I haven't been able to come up with a good idea so far. So, I'd like to hear/see your ideas! |
I see two issues that I'd like to change. One is using Display to insert variables, which means the representation cannot be specific to the format and escaping is required. The second is that a given type can only have a Template for one format. So we cannot have e.g. a separate implementation for HTML and TeX. My current thinking is to add a type parameter to the Template class, so it would be something like Askama would have some set of built in formats (HTML, maybe JavaScript, maybe LaTeX math mode and LaTeX text mode?) and would implement That's most of what I'm thinking at the moment. This would break any templates the rely on the current behavior using |
So do you have an actual case for templating the same type into different content types? If so, how do you think that should look in terms of attributes for such a type? It might actually be more relevant to you that you can have different escape modes in a single template buffer, which would need different things. There's also some overlap with the discussion in #136, if you had not seen that yet. |
The simple example of a type that could benefit from multiple mixed representations would be Of course, I don't expect a template to be defined for P.S. This was written on my phone so I apologise for any resulting unclarity. |
So Askama internally also has the In general I want Askama to be powerful enough to support the type of advanced stuff you're talking about here, but it's also important to me that the simple cases it covers today don't become much harder (following the "simple things easy, complex things possible" adage). But, realistically, getting there will require some exploration of the problem and solution space which I won't be able to drive myself (except maybe if you can/want to hire me to do so), though I can guide it along towards a direction that I'm willing to support in Askama itself. |
I agree that my proposal would also address #136 by enabling different escaping for different formats. |
How would you feel about something like the following as a solution: pub trait MarkupDisplayable<E: Escaper> {
type Repr: Display;
fn display_value(self) -> DisplayValue<Self::Repr>;
}
pub struct MarkupDisplay<E: Escaper, T: MarkupDisplayable<E>> {
value: T,
escaper: E,
}
|
Actually realizing that migrating old code to work with this change may be more annoying if you're displaying foreign types in Askama; you'd either need to make a newtype, or include a filter that handles types that implement |
Without specialization in the language, I'm not sure there's going to be a design here that I find satisfying in terms of cost/benefit ratio. I think we could have some trait that we implement for well-known types (like relevant std types and maybe any type which implements |
If you were willing to drop the requirement that // Example of some context that might be wanted by inner-templates
enum Escaping {
None,
Html,
}
trait Renderable {
fn render(&self, escaping: Escaping) -> String;
}
impl<T> Renderable for T
where
T: std::fmt::Display
{
fn render(&self, escaping: Escaping) -> String {
self.to_string()
}
}
struct ATemplateImpl {
}
impl Renderable for ATemplateImpl {
fn render(&self, escaping: Escaping) -> String {
"hello".to_string()
}
}
impl ToString for ATemplateImpl {
fn to_string(&self) -> String {
self.render(Escaping::Html)
}
} |
@Mossop any particular reason this has come up for you now? I feel like at this point templates implementing |
I only just started using asakama and I hit this bug almost immediately so figured I'd make a suggestion 🤷 |
One major problem I see is that this would break current use cases which assume that Templates get escaped, too. Maybe I am using IMO using an explicit |
autoref-specialization seems like it could be viable here? (Depending on what the generated code looks like) |
Yes, it could. I've played with that once but didn't get it to work but if someone wants to spend time on it, would be cool! |
The advantage would be, that we could possibly mark arbitrary types as safe, so the HTML escaper has to be called less often, e.g. when printing numbers. |
Yeah, could provide a nice performance benefit. |
A lovely feature would be to figure out how to recognize that an expression is a type that has derived Template, and then not escape its output (assuming both templates involved are html). I'm not sure how to implement this, but it took me a while to figure out that I needed {{ var|safe }} to avoid having the html escaped.
I can't imagine a scenario where you'd have a type with an html template, and then want that output escaped when the variable is shown in another template.
The text was updated successfully, but these errors were encountered: