Navigation Menu

Skip to content
This repository has been archived by the owner on Sep 25, 2021. It is now read-only.

Slash fix #180

Closed
wants to merge 1 commit into from
Closed

Slash fix #180

wants to merge 1 commit into from

Conversation

david62311
Copy link
Contributor

prevents the \ character from being generated

prevents the \ character from being generated
@timw255
Copy link
Contributor

timw255 commented May 30, 2016

Since the switch to parameterized queries, I think we're able to drop the string manipulation (of this sort) before saving to the database. I think this brings up a larger decision though...

What do you guys think about switching to strip_tags before saving and htmlspecialchars before displaying anything users have entered? (Rather than trying to manually handle this everywhere.)

@timw255
Copy link
Contributor

timw255 commented May 30, 2016

Actually, htmlawed is already included. Maybe we should use that instead of strip_tags.

@renlok
Copy link
Owner

renlok commented May 30, 2016

Probably a good idea, but I dont know efficient htmlawed is would that run a risk of effecting performance?

@timw255
Copy link
Contributor

timw255 commented May 30, 2016

Seems fast enough

Using the defaults, it processed some random HTML in:
[htmLawed processing time 0.0012 s, peak memory usage 0.92 MB]

@david62311
Copy link
Contributor Author

They've updated the htmlawed several times and we are still using the old version. We need to update that too.

@timw255
Copy link
Contributor

timw255 commented May 30, 2016

@david62311 Didn't notice that. I'll update.
It's being used in the admin already...and it seems easy enough to switch over.

I'll give it a go and report back

@renlok
Copy link
Owner

renlok commented May 30, 2016

It does, well I guess going with htmlawed seems like the sensible choice

@timw255
Copy link
Contributor

timw255 commented May 31, 2016

Well, the results are promising. It's now storing stuff in the db in raw form (which is good!). Just have to make sure htmlspecialchars is used when it's output for viewing and not used when output as a value in a textbox or something.

I left cleanvars but modified it to clean HTML using htmLawed (and removed the encoding bit)...then removed uncleanvars since we won't need to do that anymore. Will submit PR when I'm done testing.

@renlok
Copy link
Owner

renlok commented May 31, 2016

sounds good.

@timw255
Copy link
Contributor

timw255 commented Jun 1, 2016

This might be nailed down now. #183 addresses this and a few other things.

@renlok renlok closed this Jun 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants