Skip to content

Commit

Permalink
Reject events with long URIs and data URIs (#2536)
Browse files Browse the repository at this point in the history
* Reject events with data URIs

* Reject events with URIs longer than 2,000 characters

* Update CHANGELOG.md
  • Loading branch information
vinibrsl authored Dec 21, 2022
1 parent ecd3ff2 commit 5152e8d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.
## Unreleased

### Changed
- Reject events with long URIs and data URIs plausible/analytics#2536
- Always show direct traffic in sources reports plausible/analytics#2531

## v1.5.1 - 2022-12-06
Expand Down
20 changes: 9 additions & 11 deletions lib/plausible/ingestion/request.ex
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ defmodule Plausible.Ingestion.Request do
:pathname,
:timestamp
])
|> Changeset.validate_length(:pathname, max: 2000)
|> Changeset.apply_action(nil)

{:error, :invalid_json} ->
Expand Down Expand Up @@ -128,18 +129,15 @@ defmodule Plausible.Ingestion.Request do
end
end

@disallowed_schemes ~w(data)
defp put_uri(changeset, %{} = request_body) do
url = request_body["u"] || request_body["url"]

case url do
nil ->
Changeset.add_error(changeset, :url, "is required")

url when is_binary(url) ->
Changeset.put_change(changeset, :uri, URI.parse(url))

_ ->
Changeset.add_error(changeset, :url, "must be a string")
with url when is_binary(url) <- request_body["u"] || request_body["url"],
%URI{} = uri when uri.scheme not in @disallowed_schemes <- URI.parse(url) do
Changeset.put_change(changeset, :uri, uri)
else
nil -> Changeset.add_error(changeset, :url, "is required")
%URI{} -> Changeset.add_error(changeset, :url, "scheme is not allowed")
_ -> Changeset.add_error(changeset, :url, "must be a string")
end
end

Expand Down
26 changes: 26 additions & 0 deletions test/plausible/ingestion/request_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,30 @@ defmodule Plausible.Ingestion.RequestTest do
assert request.query_params["foo"] == "bar"
assert request.query_params["baz"] == "bam"
end

test "returns validation error when using data uris" do
payload = %{
name: "pageview",
domain: "dummy.site",
url: "data:text/html,%3Cscript%3Ealert%28%27hi%27%29%3B%3C%2Fscript%3E"
}

conn = build_conn(:post, "/api/events", payload)
assert {:error, changeset} = Request.build(conn)
assert {"scheme is not allowed", _} = changeset.errors[:url]
end

test "returns validation error when pathname is too long" do
long_string = for _ <- 1..5000, do: "a", into: ""

payload = %{
name: "pageview",
domain: "dummy.site",
url: "https://dummy.site/#{long_string}"
}

conn = build_conn(:post, "/api/events", payload)
assert {:error, changeset} = Request.build(conn)
assert {"should be at most %{count} character(s)", _} = changeset.errors[:pathname]
end
end

0 comments on commit 5152e8d

Please sign in to comment.