[Rejected] Sanitizing User Profiles
#11
Quote:I'm not opposed to applying the PR you provided, but as I said on GitHub we need to make sure it doesn't lead to double escaping.

LMK then if you want me to do the PR for the other pages to avoid double escaping. Otherwise reject my PR and we'll leave it as-is.

Quote:It can't work because if you put data sanitized into a database, it breaks queries and searches.

1.6x did it, never had any issue with how it functioned. It's similar changes to the 1.6x standards like MyCode and the editor which have wrecked previously working bug-free code.

Quote:And then you still have to consider people tampering with their database in phpmy/adminer. If you can make MyBB produce illegal output it's bad, even if it was done by database tampering.

Which only supports my argument to sanitize the input when put into the database.

Quote:some MyBB API (like if MyBB's function.php had a format_usertitle()) or one of MyBB's public variables

I'll argue that for usertitle specifically MyBB should consider a "allow html" setting. But I personally am trying to not distract MyBB from making 1.9x responsive theme. I firmly believe 100% effort should be placed there unless dealing with a security issue, which I think this is.

Quote:handle sanitization centrally rather than that garden variety of htmlspecialchars strewed all over everywhere

Another reason why I think it should only sanitize on input. Usertitle is basically updated in one place in the datahandler. It's already centralized and only needs to be done so in one place but now you have to ensure sanitizing in multple places on output.

I see that the commit was done at Github. I'll try to update the other pages to avoid double escaping usertitles at the very least.
Reply
#12
Quote:But I personally am trying to not distract MyBB from making 1.9x responsive theme.

Instead of sending a PR reverting changes, you can send one in which you implement the option to allow HTML in user title if not already done.

I think that would enhance the feature a little and we always appreciate contributions.
Reply
#13
(2018-04-28, 06:44 AM)Omar G. Wrote:
Quote:But I personally am trying to not distract MyBB from making 1.9x responsive theme.

Instead of sending a PR reverting changes, you can send one in which you implement the option to allow HTML in user title if not already done.

I think that would enhance the feature a little and we always appreciate contributions.


I'd like to do that but I'm short on time at the moment.
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)