Don't store settings as JSON about postmark-wordpress HOT 3 OPEN

@guillaumemolter can you confirm that WP-CLI's search-replace doesn't support JSON? I don't see why it wouldn't...

This amounts to a difference in opinion. We either manually run json_decode, or WP automatically runs unserialize. The same amount of processing is done regardless.

Plus the Postmark plugin's UI is javascript based, and uses the unmodified JSON settings (makes thing a lot simpler).

from postmark-wordpress.

@mgibbs189

can you confirm that WP-CLI's search-replace doesn't support JSON? I don't see why it wouldn't...

Yeah no you right...thinking about it twice...I don't see why it would be an issue.

This amounts to a difference in opinion. We either manually run json_decode, or WP automatically runs unserialize. The same amount of processing is done regardless.

Well until you need to do some data validation, escaping etc... see https://github.com/wildbit/postmark-wordpress/blob/master/postmark.php#L95 where the json is decoded only to check that it's valid... and then if you want to sanitize the data properly, on need to decode and re-encode.

Once again there is nothing wrong with JSON ( I work with JSON on all my JS projects) and PHP serialized strings are a pain to work with, but my opinion or yours don't really matter here, it's the WordPress way of doing things. And in my experience, when you work with complex CMS/Frameworks, if you don't have a really good reason not to,it's always a good idea to embrace the philosophy of the project.

If anything else it would make the code easier to read and understand for WordPress devs so they can more easily contribute to a plugin that wildbit clearly doesn't have time to update/improve.

Plus the Postmark plugin's UI is javascript based, and uses the unmodified JSON settings (makes thing a lot simpler).

Yes, as you will see I have refactored this, to be able to properly escape the data before displaying it. Once again security good practice never trust the data (especially when this data hasn't been validated and sanitized properly on the back end. Also IMO easier to understand.

Some extra resources to justify all this:

• https://codex.wordpress.org/Data_Validation
• WCB2015 XSS, CSRF, SQLI, WTH(?!?) - The Truth on Theme Security

from postmark-wordpress.

I am biased - being a WordPress dev - but I am support the opinion to go the WordPress way.
Main points by @guillaumemolter I support are

• reusing data validation and escaping already proven in Core and
• lowering the threshold for contributors from the WP space by doing it the WP way !!!!

Especially not using the helpers provided by WP core is bad. WP is already bad when it comes to dependencies and reusability so we shouldn't make it worse by ignoring working concepts for code reuse.

ALSO: a change here will not break anything anyone could have extended upon the plugin. No API/hooks will be affected.

from postmark-wordpress.