Using the MySQL API: mysql_real_escape_string - Bug?

[Tim Johnson]

There seems to be a problem with the use of the C MySQL API to
escape query strings in some of the newlisp code that I have looked at.
In the mysql5 module that comes with the distribution and also in Jeff's
own modifications, I am finding the following form in the (escape)
procedure:

Code: Select all

(set 'safe-value (dup " " (+ 1 (length value))))

If you examine this, all this does is allocate a blank string with one more
byte than the source. There should be enough bytes to escape _all_
characters (factor of 2)- if necessary, plus a byte for the terminating
NULL byte that is standard for C. I.E. length*2+1
unless I'm wrong, the form should read:

Code: Select all

(set 'safe-value (dup " " (+ 1 (* 2 (length value)))))

This provides the C function with twice the allocated bytes of
the source, plus a byte for the terminating NULL.
My modification shows predictable results, the original code
resulted in much strange behavior. :-)
references:
http://dev.mysql.com/doc/refman/5.1/en/ ... tring.html
Tim
(an 'old '(in more ways than one) 'C dog)

[Tim Johnson]

Also: I recommend that Jeff Ober's changes at
http://www.artfulcode.net/articles/upda ... l5-module/
be merged into the mysql5 module in the distro. He's added
several features that I couldn't do without. :-)
Tim

[Lutz]

If you can help and do the merge and write some additional test routines and extend the docs, I can publish it in the newlisp.org/modules section, and when we are confident its working fine it can be put in the source and binary distributions replacing the old one. But it would be to late for 9.4.

[Tim Johnson]

Can do.

In fact, as far as I call tell, all of Jeff's changes but 'fetch-table
have been merged already. I will write a routine to test escaping
and fetch-table.
This is the first time that I have worked directly with the MySQL/c api,

Looks like there may be no need to do any _unescaping_. I posted the
subject to the Mysql mailing list and got answers that indicated that
unescaping is not necessary.

thanks
Tim

[Tim Johnson]

OK. see: http://www.johnsons-web.com/demo/newlis ... pdated.lsp
To the best of my ability, I've added:
1)Two functions from Jeff Ober,
2)Modified the 'escape function with the proper memory allocation
and 'trim'ed the 'safe-value data.
3)Moved import routines into the 'init function so that the list
of shared libraries can be modified by calling code prior to
initialization.
4)Modified the test function to test both Jeff's 'fetch-table function,
and the 'escape function.

By searching on "@_THJ", you can find all of my changes.
thanks
Tim

[Jeff]

Here is a more optimized version of fetch-table, renamed to fetch-assoc:

Code: Select all

(import libmysqlclient "mysql_fetch_field")
(define (query-fields , (field-list '()))
  (when (num-fields)
    (dotimes (i (num-fields))
      (push (get-string (get-int (mysql_fetch_field MYSQL_RES))) field-list -1))
    field-list))

(define (fetch-assoc , fields)
  (when MYSQL_RES
	 (setq fields (query-fields))
	 (map 'list fields (fetch-row))))

(define (fetch-all-assoc , fields (rows '()))
  (when MYSQL_RES
     (setq fields (query-fields))
	 (setq rows '())
	 (dotimes (i (num-rows))
	   (push (map 'list fields (fetch-row)) rows -1))
	 rows))

[Tim Johnson]

Great. I would think that if one were to use the API, then one would
want to make the _best_ use of the API, as Jeff is doing here.

BTW: I've used rebol for years doing database programming with
mysql, on a a TCP/IP connection, rather than with libraries.

Using rebol I have coded character escaping based on
http://dev.mysql.com/doc/refman/5.0/en/ ... yntax.html,

and there seems to be some differences between docs at the
site above and the docs here:
http://dev.mysql.com/doc/refman/5.1/en/ ... tring.html

something I need to resolve before I deploy newlisp for DB programming....

[Jeff]

Generally, just trust mysql_real_escape_string. If you need escaping without a connection and character set doesn't matter, you can use mysql_escape_string as well.

[Jeff]

Here is a complete block that I am currently using to modify MySQL5:

Code: Select all

(context MySQL)

(define (MySQL:query sql)
  (if MYSQL_RES (mysql_free_result MYSQL_RES))
  (set 'result  (= (mysql_query MYSQL sql) 0))
  (set 'MYSQL_RES (mysql_store_result MYSQL)) 
  (if (= MYSQL_RES 0) (set 'MYSQL_RES nil))
  (set 'fields (if MYSQL_RES (query-fields) nil))
  (if (and result (find "insert into" sql 1)) (set 'result (inserted-id)))
  result)

(define (fetch-assoc)
  (when MYSQL_RES
	 (map 'list MySQL:fields (fetch-row))))

(define (fetch-all-assoc , (rows '()))
  (when MYSQL_RES
	 (setq rows '())
	 (dotimes (i (num-rows))
	   (push (map 'list MySQL:fields (fetch-row)) rows -1))
	 rows))

(import libmysqlclient "mysql_escape_string")
(define (simple-escape value , safe-value)
  (setq safe-value (pack (format "s%d" (+ 1 (* 2 (length value)))) " "))
  (if (string? value)
      (mysql_escape_string (address safe-value) (address value) (length value))
      (setq safe-value vaue)) 
  safe-value)

(import libmysqlclient "mysql_fetch_field")
(define (query-fields , (field-list '()))
  (when (num-fields)
    (dotimes (i (num-fields))
      (push (get-string (get-int (mysql_fetch_field MYSQL_RES))) field-list -1))
    field-list))

(define (sql format-string)
  (let ((format-args (cons format-string (map 'sql-value (args)))))
    (apply 'format format-args)))

(define (sql-value value)
  (case (MAIN:type-of value)
    ("string" (format "'%s'" (MySQL:simple-escape value)))
    ("symbol" (format "'%s'" (MySQL:simple-escape (name value))))
    ("integer" (format "'%d'" value))
    ("float" (format "'%f'" value))
    ("boolean" (if (true? value) "'1'" "'0'"))
    (true "NULL")))

(context MAIN)

First, query needed to be changed to set a context-global 'fields, since query-fields can only be called once per query (otherwise you get a bus error). It clears the value on each query.

The sql-value function escapes a function as necessary for use in a sql query. The sql function works exactly like format, but uses sql-value to escape. These both do a simple escape. It could easily be altered to use mysql_real_escape_string instead, but I personally needed offline escaping when I wrote it.

[Tim Johnson]

Jeff:
I found that I also had to trim the escaped string.
does

Code: Select all

(setq safe-value (pack (format "s%d" (+ 1 (* 2 (length value)))) " ")) 

take care of that?
Also:

Code: Select all

(setq safe-value vaue) ;; Typo?

Thanks
Tim

[Jeff]

Tim,

Thanks for catching that typo. I don't know how that didn't break my code :).

Also, I believe get-string is more appropriate, since it truncates the string at the first null terminator. I worry that trim might get valid characters if the string fills the entire array but, say, ends in a space when unpacked.

Here is a fixed version:

Code: Select all

(import libmysqlclient "mysql_escape_string")
(define (simple-escape value , safe-value)
  (setq safe-value (pack (format "s%d" (+ 1 (* 2 (length value)))) " "))
  (if (string? value)
      (begin
        (mysql_escape_string (address safe-value) (address value) (length value))
        (setq safe-value (get-string safe-value)))
      (setq safe-value value))
  safe-value)

[Jeff]

Also, there is a typo in the MySQL module newlispdoc comments, with the function 'tables' appearing as 'table'.

[Jeff]

Sometime in the next few days (I am planning) to release a Mysql class that wraps the MySQL module using FOOP methods and allows multiple instances to safely coexist. I'm just busy documenting everything.

PS Thanks Lutz for getting those fixes out so quickly!

[Tim Johnson]

Also, I believe get-string is more appropriate, since it truncates the string at the first null terminator. I worry that trim might get valid characters if the string fills the entire array but, say, ends in a space when unpacked.

Yes!
(Just read docs for 'get-string)
Great function....