search

getgrav / grav-theme-soraarticle

Sora Article is a minimal Theme designed for blogger, ported to Grav.
https://getgrav.org
Other
5 stars 4 forks source link

Grav 1 -> 2 Migration MegaIssue #11

Open leycec opened 4 years ago

leycec commented 4 years ago

This issue is intended to catalogue all pending issues required for Grav 2 compliance. Grav 2 is expected to break backward compatibility in various minor ways – mostly security-focused improvements to YAML and Twig parsing.

Twig: The Woody Elephant in the Room

While repairing YAML deprecations is trivial, repairing Twig deprecations is... not.

Notably, this theme will need to made compatible with Twig auto-escaping. While Grav 1 disabled Twig auto-escaping by default, Grav 2 is expected to enable Twig auto-escaping to reduce attack surface (e.g., from user-injected XSS attacks). This is a Good Thing™.

This theme does not currently support Twig auto-escaping and is thus Grav 2-incompatible. To verify this, simply disable the strict_mode/twig_compat boolean in users/config/system.yaml (e.g., via Configuration -> Advanced -> Twig Compatibility from the Admin Panel) and clear the cache. Chaos rapidly ensues in the form of auto-escaped HTML popping up literally everywhere. That's bad, especially where that HTML derives from user input.

Refactoring this theme for full compliance with Twig auto-escaping will be non-trivial, but unavoidable:

The transition to use auto-escaping will not be easy. During the transition all the template files should either contain both |e and |raw filters on every variable to make sure that the template file is safe to be used in both modes, or you can surround all the template code with {% autoescape %} Twig tags.

Can I get an "Ugh!", anybody?

Unleash the Gravitational Dogs of War

Clearly, the above discussion is not exhaustive. Imma just tryin' to get this cumbersome party started. Please feel free to chime in with additional issues as you painfully stumble over 'em, folks.

It'd be noice if the intrepid open-source volunteers amongst us could begin to hack at some of this low-hanging fruit – ideally before the impending release of Grav 2. Vitamins for all! :banana: :cherries: :strawberry: :green_apple:

mahagr commented 4 years ago

In Grav 1.x I consider every variable without escaping (|e or |raw) to be a bug as there's no way to know if the variable was safe to be used (either HTML or known). Most cases where the escaping is missing is because nobody even thought about it. Not only that -- but Grav does have autoescape configuration setting, making the template to work either it turned on or off, not both!

In Grav 2.x nobody needs to worry about the escaping anymore, which will simplify both the templates and make all the templates automatically safe from XSS attacks. It doesn't prevent someone from adding |raw everywhere, though, but at least it prevents accidents.

pushbuttonreceivecode commented 4 years ago

Here's a security-focused improvement issue up for consideration: https://github.com/getgrav/grav-theme-soraarticle/issues/12