Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide select_row(s)_with_bindings and update_rows_with_bindings functions #188

Open
majdisorder opened this issue Oct 19, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@majdisorder
Copy link

Is your feature request related to a problem? Please describe.

The approach used for the methods select_row, select_rows and update_rows is inherently flawed and vulnerable to SQL injection. In my opinion, this makes these methods unusable for all but the most trivial use cases.

Describe the solution you'd like

The solution is already partly implemented through the query_with_bindings(...) function.
Introducing new quality of life functions providing similar functionality would be great.

The functions in question:

  • select_row_with_bindings
  • select_rows_with_bindings
  • update_rows_with_bindings

Personally I would just let the existing functions accept a param_bindings Array and let this be the default behavior, however I understand this may not be feasible due to backward compatibility constraints.

From a quick look at the source code, it seems to me that most of the systems to achieve this are already in place.
For example in the update_rows function in gdsqlite.cpp, lines 584 to 593:

for (int64_t i = 0; i <= number_of_keys - 1; i++) {
	query_string += (const String &)keys[i] + String("=?");
	param_bindings.append(values[i]);
	if (i != number_of_keys - 1) {
		query_string += ", ";
	}
}
query_string += " WHERE " + p_conditions + ";";

success = query_with_bindings(query_string, param_bindings); 

It seems to me that appending the values of a newly introduced p_param_bindings parameter to the existing param_bindingsvariable should do the trick. This would allow us to use ? in p_conditions.
Again, this was just a cursory glance, I don't know the ins and outs of the code base.

Describe alternatives you've considered

It's possible to use the query_with_bindings(...) from gdscript. This however requires us to write out the queries by hand.
Alternatively a type checker similar to the one used in query_with_bindings(...) could be implemented in gdscript as well.

@majdisorder majdisorder added the enhancement New feature or request label Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant