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

DM-10267: Port HSC support for PostgreSQL registries to LSST #121

Merged
merged 6 commits into from May 10, 2017

Conversation

PaulPrice
Copy link
Contributor

No description provided.

if len(self.config.unique) > 0:
cmd += ", UNIQUE(" + ",".join(self.config.unique) + ")"
cmd += ")"
cur.execute(cmd)
Copy link
Contributor

@n8pease n8pease May 9, 2017

Choose a reason for hiding this comment

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

I thought when using dbapi, it was preferred to let the cursor.execute(operation, parameters) method substitute the values into the string, for protection against Bobby Tables. It looks like you're doing the substitutions above. Are you sure you're safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had Little Bobby Tables in my mind as I was working, but it doesn't look like the placeholders work when used for table names or types --- just values.

Copy link
Contributor

Choose a reason for hiding this comment

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

aha, yes. I had that experience too.
I'm not sure what the security requirements are in that case. @timj?

Copy link
Member

Choose a reason for hiding this comment

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

I thought we always used ? placeholders.

Copy link
Member

Choose a reason for hiding this comment

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

does the db library have a method for cleaning external input?

Copy link
Contributor

Choose a reason for hiding this comment

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

when you call execute, and pass a string with placeholders in for operation and parameters in, the parameters will be cleaned before substituted into the operation string.
the placeholder character seems to depend on paramstyle(?)

DB-API (PEP 249) does not have a Connection.execute method, but has
Cursor.execute.

Using DB-API allows us to abstract the database back-end.
INSERT OR IGNORE is SQLite-specific; replace it with something more
standard.
The choice of placeholder (e.g., '?' in SQLite) varies by database
and/or python database module. Make this a class variable.
We need to cast values to the expected type, because not all databases
and/or python database modules are lax about the type (SQLite is,
PostgreSQL is not).
@PaulPrice PaulPrice merged commit f7293e7 into master May 10, 2017
@ktlim ktlim deleted the tickets/DM-10267 branch August 25, 2018 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants