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

Allow comments as iodata. #721

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions lib/postgrex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ defmodule Postgrex do
@max_rows 500
@timeout 15_000

@comment_validation_error Postgrex.Error.exception(
message: "`:comment` option cannot contain sequence \"*/\""
)
@comment_validation_error Postgrex.Error.exception(message: "`:comment` not an iodata term")

### PUBLIC API ###

Expand Down Expand Up @@ -332,15 +330,9 @@ defmodule Postgrex do

defp comment_not_present!(opts) do
case Keyword.get(opts, :comment) do
nil ->
true

comment when is_binary(comment) ->
if String.contains?(comment, "*/") do
raise @comment_validation_error
else
false
end
nil -> true
comment when is_binary(comment) or is_list(comment) -> false
_ -> raise @comment_validation_error
end
end

Expand Down
5 changes: 3 additions & 2 deletions lib/postgrex/protocol.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1595,8 +1595,9 @@ defmodule Postgrex.Protocol do
transaction_error(s, postgres)
end

defp parse_describe_comment_msgs(query, comment, tail) when is_binary(comment) do
statement = [query.statement, "/*", comment, "*/"]
defp parse_describe_comment_msgs(query, comment, tail)
when is_binary(comment) or is_list(comment) do
statement = [query.statement, ";/*", comment, "*/"]
query = %{query | statement: statement}
parse_describe_msgs(query, tail)
end
Expand Down
20 changes: 18 additions & 2 deletions test/query_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1851,14 +1851,30 @@ defmodule QueryTest do
assert [["1", "2"], ["3", "4"]] = query("COPY (VALUES (1, 2), (3, 4)) TO STDOUT", [], opts)
end

test "comment", context do
test "binary comment", context do
assert [[123]] = query("select 123", [], comment: "query comment goes here")
end

test "iodata comment", context do
assert [[123]] = query("select 123", [], comment: ["query", ?=, ?', "comment", ?'])
end

test "comment validation error when comment is not iodata", context do
assert_raise Postgrex.Error, fn ->
query("select 123", [], comment: "*/ DROP TABLE 123 --")
query("select 123", [], comment: 123)
end
end

test "comment error when comment tries sql injection", context do
assert %Postgrex.Error{
postgres: %{
code: :syntax_error,
message: "cannot insert multiple commands into a prepared statement"
}
} =
query("select now()", [], comment: "*/ select sleep(1000)--")
end

@tag :big_binary
test "receive packet with remainder greater than 64MB", context do
# to ensure remainder is more than 64MB use 64MBx2+1
Expand Down
Loading