02 Jun, 2014, syn wrote in the 41st comment:
Votes: 0
Pymeus said:
snprintf(query, sizeof(query) +1, "SELECT entry_num, level, category, keywords, text FROM bd_help WHERE keywords=\'%s\' OR keywords LIKE \'%s %%\' OR keywords LIKE \'%% %s\' OR keywords LIKE \'%% %s %%\' OR keywords LIKE \'%% %%%s %%\'OR keywords LIKE \'%% %s%% %%\' OR keywords LIKE \'%s%%\' LIMIT 10", passed_arg, passed_arg, passed_arg, passed_arg, passed_arg, passed_arg, passed_arg);


The same results could be gotten without so many (relatively) slow wildcard comparisons if expressed as:
"SELECT entry_num, level, category, keywords, text FROM bd_help WHERE keywords LIKE \'%% %s%% %%\' OR keywords LIKE \'%s%%\' LIMIT 10"

It's possible the query optimizer will consolidate the patterns for you, but I wouldn't bet on it. I think it's also a lot easier on the eyes too, but that's just my opinion.

It's worth pointing out that mysql has a mysql_real_escape_string() function that takes care of string escaping issues for you, when used correctly. It even accounts for character encodings other than what you're used to, although we usually ignore such things in mud code, for better or worse.

Also, as a general rule, don't give sizeof()+1 to snprintf. If in your example 'query' is a string pointer, then sizeof() returns the size of the pointer, not the char array it points to, which will truncate the query (probably to "SELE" or "SELECT e"). If it's a statically sized C array instead, then any time the expanded string fills or exceeds the size of the array, 1 byte of memory after the end of the array will be overwritten. In either case, the behavior is probably not what you want.


Thanks for the tip on the query, it hasnt thus far truncated anything at all, but I will look at doing that in another way.

As for the wildcards, I want(ed) to hit on at least a portion of the actual substring, so if someone does Help al they can get some responses that contain al. If I wanted to go crazy, I would have added more. I figured what I had was sufficient, and the first where = provides a direct hit if there is one, which is preferred over any first potential partial match it may come across.
02 Jun, 2014, quixadhal wrote in the 42nd comment:
Votes: 0
Actually, you need to have snprintf() use the size -1, NOT the size + 1. C strings require a NUL termination byte, so if you don't reserve that extra space, you may run off into the weeds if your string is actually the full length of the buffer.

As others have said, you can protect against SQL injection by using positional parameters in your query, and bound variables by whatever means your API allows.

While you can certainly push the work onto the database for this, I would actually suggest caching the list of keywords and doing that pre-validation before the query.

SELECT DISTINCT keywords FROM bd_help ORDER BY keywords DESC;

Now, if you wanted to match any word where it might be a single word out of keywords, you need to split on whitespace. You can do that in code. If you're using PostgreSQL, there's a handy utility function….

SELECT DISTINCT lower(trim(regexp_split_to_table( bd_help.keywords, '\s+' ))) AS word FROM bd_help ORDER BY word DESC;

With an ordered list of all valid help topics, you can do a binary search on the list to quickly validate what the user typed. If no match is found, return an error without ever touching the database. If at least one match is found, do the query to get the entries and present the user with a choice if there was more than one.
02 Jun, 2014, syn wrote in the 43rd comment:
Votes: 0
Yeah, I mean this was written before.. most of this thread, and our discussions.

Once I fix up a few other items I am going to re-write it given other advice in the thread, and this last. It would be (now that I get the picture) easier to, as you said, just pre-load keywords and sort/search that loaded set within the MUD, and deal with the DB only as I know I need to, versus asking if I need to, then if so, again going to the DB, effectively making 2+ round trips per help request (clearly stupid).

I am using MySQL but I will see if it has something as you mentioned, I am not terribly familiar with it yet, and not at all with Postgre so, still learning :D

My lack of DB experience is clearly showing, but that's why I wanted the help, lots of great lessons!

For instance, I had read/kind of seen things about injection protection in general, but I didnt exactly know how it would be accomplished, so I tried to simply project against any use of ' along with key phrases, like drop table
02 Jun, 2014, syn wrote in the 44th comment:
Votes: 0
I think given that I have no formal code or DB experience between volunteered projects at work with no training, or hobbyist projects that I am learning pretty well, and a lot of that is thanks to all of you folks. So to re-iterate, thank you all again!
02 Jun, 2014, Pymeus wrote in the 45th comment:
Votes: 0
It's hardly worth mentioning, but according to the POSIX spec for snprintf() the exact size of the buffer will do.
POSIX said:
The snprintf() function shall be equivalent to sprintf(), with the addition of the n argument which states the size of the buffer referred to by s. If n is zero, nothing shall be written and s may be a null pointer. Otherwise, output bytes beyond the n-1st shall be discarded instead of being written to the array, and a null byte is written at the end of the bytes actually written into the array.

But given the wide selection of badly broken C standard libraries published over the years, it's entirely possible that size-1 is slightly more portable.
02 Jun, 2014, Pymeus wrote in the 46th comment:
Votes: 0
syn said:
I think given that I have no formal code or DB experience between volunteered projects at work with no training, or hobbyist projects that I am learning pretty well, and a lot of that is thanks to all of you folks. So to re-iterate, thank you all again!

The same mistakes make it into professional "production" code more often than we'd like, so don't sweat it.
03 Jun, 2014, quixadhal wrote in the 47th comment:
Votes: 0
Pymeus said:
It's hardly worth mentioning, but according to the POSIX spec for snprintf() the exact size of the buffer will do.
POSIX said:
The snprintf() function shall be equivalent to sprintf(), with the addition of the n argument which states the size of the buffer referred to by s. If n is zero, nothing shall be written and s may be a null pointer. Otherwise, output bytes beyond the n-1st shall be discarded instead of being written to the array, and a null byte is written at the end of the bytes actually written into the array.

But given the wide selection of badly broken C standard libraries published over the years, it's entirely possible that size-1 is slightly more portable.


Yep, the key is the POSIX note… not all C libraries are POSIX compliant. Also, using buffer-1 is safer for those times you're going to mix it up and deal with strncat() or strncpy() which do NOT NUL terminate the destination if the length of what's copied is exactly the limit given.

Of course, as always for the last 10 years, I suggest using something other than C when dealing with large amounts of text manipulation. :)
40.0/47