hamster
hamster copied to clipboard
Bug Fix: Escape html characters for XML/HTML reports
Having any html/xml characters in the description breaks the reports output in html (surely in xml as well, have only tested html).
Thanks for your contribution, this seems like a good thing to fix.
However, a few thoughts on the implementation:
- This is now fixed for description, but I would say the same treatment should be applied to the activity, category, tags, and really all other data that is inserted into the HTML (unless it is very clear that it can never contain special characters, and even then it would not hurt).
- IMHO it would be more elegant if the escaping is triggered from inside the template, as that is the place where you know exactly how the value will be used (i.e. escape as late as possible). However, it seems that
string.Template
does not support anything more advanced than just string replacements, so I guess your current approach is acceptable (no point in switching to a more complex template language just for this). - I'm pretty sure that this is not needed for XML, since there an XML library is used that is given the value to set for attributes, so that can handle escaping as needed (unlike HTML, where it's just string replacement, which is not aware of the need for escaping, or even the difference between attributes and HTML tags at all). Would be good if you could confirm this, and if I am correct, remove the XML changes.
Thanks for your contribution, this seems like a good thing to fix.
However, a few thoughts on the implementation:
1. This is now fixed for description, but I would say the same treatment should be applied to the activity, category, tags, and really all other data that is inserted into the HTML (unless it is very clear that it can never contain special characters, and even then it would not hurt).
I had thought about this, and you are right -- any string input should be html escaped, they are now all escaped.
2. IMHO it would be more elegant if the escaping is triggered from inside the template, as that is the place where you know exactly how the value will be used (i.e. escape as late as possible). However, it seems that `string.Template` does not support anything more advanced than just string replacements, so I guess your current approach is acceptable (no point in switching to a more complex template language just for this).
Yeah, actually the template is where I looked first... but as you say, best kept in the .py file.
3. I'm pretty sure that this is _not_ needed for XML, since there an XML library is used that is given the value to set for attributes, so that can handle escaping as needed (unlike HTML, where it's just string replacement, which is not aware of the need for escaping, or even the difference between attributes and HTML tags at all). Would be good if you could confirm this, and if I am correct, remove the XML changes.
You are correct, I never use XML exports, these are exported as attribute values, so it's not necessary (quoted strings).
Lastly, I have been using bullet points on my description -- I have added code to convert new lines to break line in html so it's respected. Hope that w/ this change we can merge, as it's relatively simple :)
On another note: Later... time permitting, I would like to improve the html in the template to make it responsive -- also what do you think of not printing days w/ 0 hours?