Skip to content

Commit

Permalink
FIX: Wrong argument error being thrown in UrlHelper (discourse#24506)
Browse files Browse the repository at this point in the history
We were throwing ArgumentError in UrlHelper.normalised_encode,
but it was incorrect -- we were passing ArgumentError.new
2 arguments which is not supported. Fix this and have a hint
of which URL is causing the issue for debugging.
  • Loading branch information
martin-brennan authored Nov 22, 2023
1 parent 86da47f commit 4e7929a
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
4 changes: 3 additions & 1 deletion lib/url_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ def self.secure_proxy_without_cdn(url)
def self.normalized_encode(uri)
url = uri.to_s

raise ArgumentError.new(:uri, "URL is too long") if url.length > MAX_URL_LENGTH
if url.length > MAX_URL_LENGTH
raise ArgumentError.new("URL starting with #{url[0..100]} is too long")
end

# Ideally we will jump straight to `Addressable::URI.normalized_encode`. However,
# that implementation has some edge-case issues like https://github.com/sporkmonger/addressable/issues/472.
Expand Down
4 changes: 3 additions & 1 deletion spec/lib/url_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,10 @@
end

it "raises error if too long" do
expect do UrlHelper.normalized_encode("https://#{"a" * 2_000}.com") end.to raise_error(
long_url = "https://#{"a" * 2_000}.com"
expect do UrlHelper.normalized_encode(long_url) end.to raise_error(
ArgumentError,
"URL starting with #{long_url[0..100]} is too long",
)
end
end
Expand Down

0 comments on commit 4e7929a

Please sign in to comment.