search

arcapos / luapgsql

Lua binding for PostgreSQL
108 stars 24 forks source link

Check arguments *before* doing allocations #35

Closed daurnimator closed 8 years ago

daurnimator commented 8 years ago

e.g. currently:

> p = require "pgsql"
> c = p.connectdb()
stdin:1: bad argument #1 to 'connectdb' (string expected, got pgsql connection methods)
stack traceback:
    [C]: in function 'pgsql.connectdb'
    stdin:1: in main chunk
    [C]: in ?

This is because in pgsql_connectdb you call pgsql_conn_new before calling luaL_checkstring.

Order should be:

  1. Check all arguments (using e.g. luaL_checkstring)
  2. Allocate all space needed for results
  3. Make the actual call
mbalmer commented 8 years ago

Fixed in 40dc68c3268c6919941c811ed72a2a53a76cdb91

mbalmer commented 8 years ago

grr, now it potentially can leak..

mbalmer commented 8 years ago

Ok, fixed in 103fbe1b8656dffa1e14a54ad83e559e3e98a1e5

daurnimator commented 8 years ago

Thanks. That looks better.

daurnimator commented 8 years ago

Though this is still a problem in e.g. pgsql_encryptPassword. The gcmalloc pushes a userdata onto the stack. You need to move the luaL_checkstrings to the top of the functions.

mbalmer commented 8 years ago

Am 11.05.2016 um 10:28 schrieb daurnimator notifications@github.com:

Though this is still a problem in e.g. pgsql_encryptPassword. The gcmalloc pushes a userdata onto the stack. You need to move the luaL_checkstrings to the top of the functions.

It would do no harm, since I use absolute indexes, but you are right, it's nicer to check first. I'll change that.

daurnimator commented 8 years ago

It would do no harm, since I use absolute indexes, but you are right, it's nicer to check first. I'll change that.

It does create incorrect error messages. e.g. if I call pgsql_encryptPassword with no arguments it will tell me string expected, got pgsql garbage collected memory instead of string expected, got nil.

daurnimator commented 8 years ago

I see you fixed pgsql_encryptPassword in c5254c0d3bb279c327ba6e08e34a5da8e01028dc. It also needs doing to conn_unescapeBytea