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
Quote view names #188
Quote view names #188
Conversation
|
||
Search.connection.drop_view :'scenic."search in a haystack"' | ||
|
||
silence_stream(STDOUT) { eval(output) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of eval is a serious security risk.
|
||
Search.connection.drop_view :'scenic."search in a haystack"' | ||
|
||
silence_stream(STDOUT) { eval(output) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of eval is a serious security risk.
it "dumps a create_view for a view in the database" do | ||
view_definition = "SELECT 'needle'::text AS haystack" | ||
Search.connection.execute( | ||
"CREATE SCHEMA scenic; SET search_path TO scenic, public") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
it "dumps a create_view for a view in the database" do | ||
view_definition = "SELECT 'needle'::text AS haystack" | ||
Search.connection.execute( | ||
"CREATE SCHEMA scenic; SET search_path TO scenic, public") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
|
||
Search.connection.drop_view :'"search in a haystack"' | ||
|
||
silence_stream(STDOUT) { eval(output) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of eval is a serious security risk.
|
||
Search.connection.drop_view :'"search in a haystack"' | ||
|
||
silence_stream(STDOUT) { eval(output) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of eval is a serious security risk.
context "with views using unexpected characters in name" do | ||
it "dumps a create_view for a view in the database" do | ||
view_definition = "SELECT 'needle'::text AS haystack" | ||
Search.connection.create_view '"search in a haystack"', sql_definition: view_definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [93/80]
context "with views using unexpected characters in name" do | ||
it "dumps a create_view for a view in the database" do | ||
view_definition = "SELECT 'needle'::text AS haystack" | ||
Search.connection.create_view '"search in a haystack"', sql_definition: view_definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [93/80]
@@ -43,9 +43,9 @@ def to_scenic_view(result) | |||
namespace, viewname = result.values_at "namespace", "viewname" | |||
|
|||
if namespace != "public" | |||
namespaced_viewname = "#{namespace}.#{viewname}" | |||
namespaced_viewname = "#{pg_identifier(namespace)}.#{pg_identifier(viewname)}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [90/80]
@@ -43,9 +43,9 @@ def to_scenic_view(result) | |||
namespace, viewname = result.values_at "namespace", "viewname" | |||
|
|||
if namespace != "public" | |||
namespaced_viewname = "#{namespace}.#{viewname}" | |||
namespaced_viewname = "#{pg_identifier(namespace)}.#{pg_identifier(viewname)}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [90/80]
Closes #172 |
6aee010
to
6ceb522
Compare
@@ -54,6 +54,11 @@ def to_scenic_view(result) | |||
materialized: result["kind"] == "m", | |||
) | |||
end | |||
|
|||
def pg_identifier(name) | |||
return name if name =~ /^[a-zA-Z_][a-zA-Z0-9_]*$/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not safely quote the identifier in any case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this line caused double quoting in in-memory representations of the names (failing tests) and in the SQL dump (failing more tests, and breaking things). I wasn't able to fix that in ~an hour, so I reverted my attempts.
I tried removing this and it ended up causing double quoting because we'd quote on the way out of the db and when it goes in. I think it's only code, but it does break the scheme dump.
… On Feb 9, 2017, at 7:34 PM, Derek Prior ***@***.***> wrote:
@derekprior commented on this pull request.
In lib/scenic/adapters/postgres/views.rb:
> @@ -54,6 +54,11 @@ def to_scenic_view(result)
materialized: result["kind"] == "m",
)
end
+
+ def pg_identifier(name)
+ return name if name =~ /^[a-zA-Z_][a-zA-Z0-9_]*$/
Can we not safely quote the identifier in any case?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
29bfad8
to
a56338c
Compare
a56338c
to
b46e5ef
Compare
No description provided.