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

Multistatements and multi results #411

Merged
merged 10 commits into from
Jan 30, 2016
Merged

Multistatements and multi results #411

merged 10 commits into from
Jan 30, 2016

Conversation

julienschmidt
Copy link
Member

The daily PR:

  • Adds new DSN param to allow multiple statements in one query
  • Enables usage of stored procedures (even without multiStatements=true)

Fixes #66
Fixes #163
Fixes #325
Fixes #338
Fixes #339

lucalooz and others added 7 commits January 20, 2016 17:29
- packets.go: flag clientMultiResults, update status when receiving an
EOF packet, discard additional results on readRow when EOF is reached
- statement.go: currently a nil rows.mc is used as an eof, don’t set it
if there are no columns to avoid that Next() waits indefinitely
- rows.go: discard additional results on close and avoid panic on
Columns()
discard additional OK response after Multi Statement Exec Calls
@julienschmidt julienschmidt added this to the v1.3 milestone Jan 20, 2016
Default: false
```

Allow multiple statements in one query. While this allows batch queries, it also greatly increases the risk of SQL injections.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add something like "results are returned for the first query only, others are silently discarded".

@julienschmidt
Copy link
Member Author

PTAL

@arnehormann
Copy link
Member

I'd like some tests and/or documentation for what happens if you run

  • db.Query("UPDATE ...; SELECT ...")
  • db.Exec("SELECT ...; UPDATE ...")

And I'd prefer discardResults or at least discardOtherResults (as in "all results not already read") to discardMoreResultsIfExists.
LGTM otherwise.
Good job 👍, sorry for slow reviews...

julienschmidt added a commit that referenced this pull request Jan 30, 2016
Multistatements and multi results
@julienschmidt julienschmidt merged commit b4db83c into master Jan 30, 2016
@julienschmidt julienschmidt deleted the multistmt branch January 30, 2016 00:03
@wuranbo
Copy link

wuranbo commented Feb 27, 2016

@julienschmidt thanks so much. Love you superman!

arp242 added a commit to arp242/mysql that referenced this pull request May 16, 2023
I can't really find any reference to the risk of SQL injections. This
sets the clientMultiStatements flag (or CLIENT_MULTI_STATEMENTS in the C
API).

This comment was added in go-sql-driver#411, but without much explanation, and I
can't find anything in e.g. go-sql-driver#66 or other issues either.

The documentation for MySQL[1] or MariaDB[2] doesn't warn for SQL
injections, and after some internet searching the only reference I found
was in the PHP Docs[3]:

	The API functions mysqli::query() and mysqli::real_query() do
	not set a connection flag necessary for activating multi queries
	in the server. An extra API call is used for multiple statements
	to reduce the damage of accidental SQL injection attacks. An
	attacker may try to add statements such as ; DROP DATABASE mysql
	or ; SELECT SLEEP(999).

So I assume this is what this comment refers to.

This removes the comment, as discussed in go-sql-driver#1206.

[1]: https://dev.mysql.com/doc/c-api/8.0/en/c-api-multiple-queries.html
[2]: https://mariadb.com/kb/en/mysql_real_connect/
[3]: https://www.php.net/manual/de/mysqli.quickstart.multiple-statement.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants