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

postgres.db_create: fix handling of empty string #33261

Conversation

orymate
Copy link
Contributor

@orymate orymate commented May 15, 2016

What does this PR do?

Fix handling of empty strings as parameters of postgres.db_create.

What issues does this PR fix or reference?

The code responsible for collecting the parameters used in the CREATE
DATABASE query works unexpectedly in case for values which are evaluated
to False as bool, but are not None.

This caused queries with missing rvalues like this one for db_create(..., lc_collate=""):

2016-05-13 16:04:05,243 [salt.loaded.int.module.cmdmod][ERROR   ][2990]
stderr: ERROR:  syntax error at or near "OWNER"
LINE 1: ..._production" WITH ENCODING = 'utf8' LC_COLLATE =  OWNER =
"g...

Please note that proper escaping or a different approach would be needed
here.

Previous Behavior

stderr: ERROR:  syntax error at or near "OWNER"
LINE 1: ..._production" WITH ENCODING = 'utf8' LC_COLLATE =  OWNER =
"g...

New Behavior

Success.

Tests written?

Yes

return None
if quote in value: # detect trivial sqli
raise SaltInvocationError(
'Unsupported character {0} in value: {0}'.format(quote, value))
Copy link
Contributor

Choose a reason for hiding this comment

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

The second interpolation here should be {1}.

@cachedout
Copy link
Contributor

Hi @orymate

This looks good. Just one small concern that I noted inline and the rest should be good. Thanks!

@cachedout cachedout added Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged Awesome labels May 16, 2016
@orymate orymate force-pushed the postgres-db_create-fix-handling-empty-string-args branch from 27fd64d to 7dfc9ea Compare May 16, 2016 14:17
@orymate
Copy link
Contributor Author

orymate commented May 16, 2016

Thanks, fixed the format string.

Mate Ory added 4 commits May 16, 2016 16:24
The code responsible for collecting the parameters used in the CREATE
DATABASE query works unexpectedly in case for values which are evaluated
to False as bool, but are not None.

This caused queries with missing rvalues like this one (lc_collate=""):
2016-05-13 16:04:05,243 [salt.loaded.int.module.cmdmod][ERROR   ][2990]
stderr: ERROR:  syntax error at or near "OWNER"
LINE 1: ..._production" WITH ENCODING = 'utf8' LC_COLLATE =  OWNER =
"g...

Please note that proper escaping or a different approach would be needed
here.
@orymate orymate force-pushed the postgres-db_create-fix-handling-empty-string-args branch from 7dfc9ea to 4b01d5d Compare May 16, 2016 14:24
@cachedout cachedout merged commit 0de5410 into saltstack:develop May 16, 2016
@cachedout
Copy link
Contributor

Super! Thank you, @orymate

@orymate orymate deleted the postgres-db_create-fix-handling-empty-string-args branch May 16, 2016 16:29
gitebra pushed a commit to gitebra/salt that referenced this pull request May 16, 2016
* commit '319a4d8288697de13b52b6f9e8ccf680820ea60f':
  Files without pkg.owner are not skipped (saltstack#33202)
  Fix two pure python errors causing UnboundLocalErrors (saltstack#33268)
  postgres.db_create: fix handling of empty string (saltstack#33261)
  new runner for vistara (saltstack#33263)
  linux_acl: Allow '-' as a separation character in ACL permissions. Fixes saltstack#31270 (saltstack#33172)
  Unbreak passwd change/expire reading on OpenBSD (saltstack#33227)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awesome Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants