ecto_fields
ecto_fields copied to clipboard
EctoFields.URL does not support URLs with ports in them
For example:
EctoFields.URL.cast("http://example.com:1234/")
:error
Problem appears to be at https://github.com/jerel/ecto_fields/blob/master/lib/fields/url.ex#L60
Actually, looking at the code it seems like there are probably a great deal of valid URLs that wouldn't be correctly treated. However what EctoFields is doing is a sensible default for a great deal of use cases, was that the original intention?
Suitable fixes that I can think of:
- Update
EctoFields.URLto accept all valid URLs. AFAIK this is much more involved than it sounds. But there might be some work that could be borrowed from https://mathiasbynens.be/demo/url-regex for example. - Update
EctoFields.URLto accept some sort of per-field configuration allowing a user to customise the validation behaviour of URLs.
I used something like this:
@spec valid_url?(any()) :: boolean()
def valid_url?(url) when is_binary(url) do
case URI.parse(url) do
%URI{host: nil} -> false
%URI{scheme: nil} -> false
%URI{} -> true
end
end
def valid_url?(_), do: false
EctoFields.URL also doesn't support IP addresses for a host.
EctoFields.IP.cast("http://1.1.1.1")
:error
I've added support for ports and IP addresses via these commits https://github.com/jerel/ecto_fields/compare/7d39c880a68e897bad0d89e97ac0ef0078fe1221...master
@joeapearson Yes, the idea is to have sensible handling for most use cases. In your estimation is the current implementation too restrictive or too permissive?