Fix form data escaping and add filter 'wplf_form_data'

libreform / wp-libre-form

Easy native HTML5 forms for WordPress. Version 1.5 is unmaintained, but works without issue. 2.0 has been rewritten from the ground, and can be found at https://github.com/libreform/libreform

https://wordpress.org/plugins/wp-libre-form

GNU General Public License v3.0

67 stars 27 forks source link

Fix form data escaping and add filter 'wplf_form_data' #40

Closed jpeltoniemi closed 7 years ago

jpeltoniemi commented 7 years ago

Fixed form data escaping when passing to add_post_meta. Discussed in #38.

Values don't need to be html-escaped for database.
WP applies magic quotes. Removes the added slashes so _wplf_formdata filter gets and can output clean data.
Adds proper slashes for add_post_meta (https://codex.wordpress.org/Function_Reference/update_post_meta#Character_Escaping).

Added _wplf_formdata filter. Allows manipulation of form data before it is used.

jpeltoniemi commented 7 years ago

Is there something wrong with this one?

anttiviljami commented 7 years ago

In stockholm for WordCamp right now. :) I'll take a look at this as soon as I can. Sorry for the delay!

anttiviljami commented 7 years ago

Thanks for the PR ! :+1: ❤️

jpeltoniemi commented 7 years ago

Ah, ok. Have fun! ;)

anttiviljami commented 7 years ago

I'm not too sure about this:

 // copy $_POST to $form_data in order to undo WordPress' magic quotes
 $form_data = stripslashes_deep($_POST);

A lot of wplf_validate_submission and wplf_post_validate_submission stuff tends to check values from $_POST, and if that's not the data that actually will get saved to the database, I feel it would be a bit confusing.

I don't see WordPress's magic quotes as a bad thing at all. Why do we need to get rid of that?

jpeltoniemi commented 7 years ago

That's for the sake of the wplf_form_data filter. It won't affect $_POST at all, so anything else relying on $_POST being escaped won't know the difference.

It helps in a sense that if you were to write more special characters into form data in wplf_form_data, you'd have to take care of proper escaping yourself, which I at least find pointless. Instead we get to work on the raw data(without ever having to worry about how the data is escaped) and let WPLF handle escaping at the end ($value = wp_slash($value); on line 58)

I've been expanding on this code a bit and if you don't mind a bigger PR, I'd like to include this in the next one instead.

Of course, if you still have doubts about my approach, ask away. I'm pretty convinced I'm not at least completely wrong :D And if you want, we can have a chat in finnish. Would probably be way quicker, too.