Wedge
Public area => The Pub => Features => Topic started by: Arantor on February 21st, 2013, 02:10 AM
-
So, we have escaped strings, e.g. we use {string:blah} and that's going to look for a variable 'blah'. Surprisingly often that will turn out to be 'blah' in the parameters, e.g. we needed the content to be as is, but merely string escaped for queries.
So I'm wondering, is it worth having the case where we define {escliteral:string} to have 'string' appear in the query properly escaped?
Primarily wondering what the coders are likely to make of it... Nao, Dragooon, live, thoughts? It's just a convenience feature really.
-
Maybe I''m being particularly dense (recovering from a bad cold) but this confuses me. Would it htmlspecialchars the vars?
-
Nope. It would mysql_real_escape_string the vars.
Consider a query I fixed today, which is what prompted it.
wesql::query('
DELETE FROM {db_prefix}bans
WHERE ban_type = {string:id_member}
AND ban_content IN ({array_string:users})',
array(
'id_member' => 'id_member',
'users' => $users,
)
);
The whole reason for id_member is so that 'id_member' gets passed into the string with proper escaping. Except on original commit I actually forgot the variable injection. This sort of thing is used in a lot of places in Wedge.
I'm proposing it would become:
wesql::query('
DELETE FROM {db_prefix}bans
WHERE ban_type = {escliteral:id_member}
AND ban_content IN ({array_string:users})',
array(
'users' => $users,
)
);
-
I like the idea. I've had on occasion been forced to do 'stuff' => 'stuff' indeed, and this would make it neater.
Just two remarks:
- escliteral isn't an exciting name ;) I could go with 'literal', though. Or 'escape', or whatever...
- we might get people complaining that there's a bug here or there because "a variable is missing its value"... Although I doubt it -- coders usually are smart enough to figure things out without checking the source :P
-
I've implemented this locally, using the name literal. Seems straightforward enough. I've also modified the example query given (which is a real query) to reflect the new code so I know it works.
-
And as you can see, I immediately adopted it... ;D
I think the best way to ensure it's used everywhere, is to do it the hard way, i.e. search for {string: and verify what it's matched to... Also, as I asked in the changelog: should we do {literal:0} things, too..?
-
Numbers don't need escaping. Some of the places it is used as such I would personally rather not do it since fairly often it does have a habit of expressing the meaning behind that number.
The great thing is that it doesn't have to be used but changed gradually since it's just a shortcut to something we already had longer.
-
Numbers don't need escaping.
Oh, right, you probably weren't there at the time SMF underwent the {change}. That was in late 2007 or early 2008... Charter members and beta testers with access to the files were disgusted at how the team had implemented the security change -- while it was a GOOD thing, it was done in such an automated way, we'd find many useless escaped strings and it really, really made the codebase look so ugly... Over the months, though, the devs ensured that the initial mess was mostly cleaned up, but they still left a lot of {int:zero} things here and there...
Of course, there's one case where it's all right to use this system for constants. Which is explaining an integer value of an option list. For instance, SELECT something WHERE value IN ({int:nobody}, {int:everyone}) -> array('nobody' => 0, 'everyone' => 1).
In this case, yes, it's better to stick with the escaped constant.
Anyway: sorry if my sarcasm wasn't too obvious. Maybe it's the lack of sleep... Only had less than 3 hours today.
But posting this thing in the changelog sort of made me consider removing the 'obvious' ones.
Well, I guess I'll do that... When I see them!
I already spent half an hour turning strings to literal this morning, so I guess I'll leave it for another time.
Do you want to do the rest by yourself, BTW..? I think I did at least half of them.
-
Oh, right, you probably weren't there at the time SMF underwent the {change}
Over the months, though, the devs ensured that the initial mess was mostly cleaned up, but they still left a lot of {int:zero} things here and there...
Anyway: sorry if my sarcasm wasn't too obvious. Maybe it's the lack of sleep... Only had less than 3 hours today.
<--- unless it's this way.Do you want to do the rest by yourself, BTW..? I think I did at least half of them.
-
I remember Orstio publicly saying "Is this meant to be some kind of joke?"
After all, some of the devs did the transition quite... reluctantly.Bed is this way --->
<--- unless it's this way.
Unless you're looking from inside my monitor, of course. (Now get off my monitor!)I'm really not that worried. I never really planned to go on a blitz through the source, I figured I'd just update them as I encountered them, much how <URL> is being applied.
Oh, makes me think... I think somewhere else you asked about making ob_sessrewrite callable from a non-buffer context so that we could apply pretty URLs and <URL> transforms to any string, without having to bother with resetting the buffer output. Is it still in the news..? I know I wanted to do that 'back in the day', but I can't remember why I stepped back, though.
-
Is it still in the news..? I know I wanted to do that 'back in the day', but I can't remember why I stepped back, though.
-
Okay, so... Get ready for the big stupid suggestion.
Why not just call ob_sessrewrite($string) directly..? :^^;:
I'm sure we can accomodate for direct calls very easily.
Also, before I leave to take a break --
1/ I've managed to implement my flexbox test. It has the advantage of not breaking anything for old browsers. I'll commit it next. I'll be glad to get your opinion on it. It's a very, very minor thing... But one that has been 'in your face' ever since the early days of SMF 2.0. And because it's something in the Msg template... Well, it's even more 'in your face', but I think most people never noticed the difference ;)
2/ I was considering something else... A bit bigger. I know there's a "policy" for function naming in SMF (and thus Wedge), and you've explained it several times at sm.org and here, and it has something to do with functions for internal use, functions for plugin/theme/mod use, etc... I know all that. But when I go through Subs.php, and all of these pretty similar functions (semantically at least), I just can't a hold of it. Why is this camel-cased, while that one isn't..?
And then it came to me. We're Wedge. We can afford to rename functions.
So... Wouldn't it make sense to settle for one style, and only one..?
Obviously, dropping camel-case would make the most sense to me. Having everything rendered through underscores and lowercase chars would at least save people the hassle of trying to remember how a function name was to be written.
Or, just forget it ;)
-
Why not just call ob_sessrewrite($string) directly..?
It's a very, very minor thing... But one that has been 'in your face' ever since the early days of SMF 2.0
But when I go through Subs.php, and all of these pretty similar functions (semantically at least), I just can't a hold of it. Why is this camel-cased, while that one isn't..?
The only thing about using underscores, aside from the extra typing (I'd rather camelCase than underscore personally), is that sometimes odd things happen in terms of where the underscore is. ob_sessrewrite never looks right when I type it. I always think of it as obsess_rewrite. Freud has much to answer for. :^^;:
The other thing is that functions are case insensitive in PHP (or should be), meaning that if it's all camelCased, getting case wrong doesn't matter as much as misplacing _ does.