From 9526f3c3737f0fffb8bb722a5693b1e9121ae8dd Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Mon, 8 May 2017 17:26:50 +0200 Subject: [PATCH 1/4] Remove any use of Regex in the Version.Parser module --- lib/elixir/lib/version.ex | 110 +++++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 42 deletions(-) diff --git a/lib/elixir/lib/version.ex b/lib/elixir/lib/version.ex index 8714bb8739b..34e105ed8ab 100644 --- a/lib/elixir/lib/version.ex +++ b/lib/elixir/lib/version.ex @@ -392,67 +392,93 @@ defmodule Version do Enum.reverse(acc) end - @version_regex ~r/^ - (\d+) # major - (?:\.(\d+))? # minor - (?:\.(\d+))? # patch - (?:\-([\d\w\.\-]+))? # pre - (?:\+([\d\w\.\-]+))? # build - $/x - @spec parse_requirement(String.t) :: {:ok, term} | :error def parse_requirement(source) do lexed = lexer(source, []) to_matchspec(lexed) end - defp nillify(""), do: nil - defp nillify(o), do: o - @spec parse_version(String.t) :: {:ok, Version.matchable} | :error def parse_version(string, approximate? \\ false) when is_binary(string) do - if parsed = Regex.run(@version_regex, string) do - destructure [_, major, minor, patch, pre], parsed - patch = nillify(patch) - pre = nillify(pre) - - if is_nil(minor) or (is_nil(patch) and not approximate?) do - :error - else - major = String.to_integer(major) - minor = String.to_integer(minor) - patch = patch && String.to_integer(patch) - - case parse_pre(pre) do - {:ok, pre} -> - {:ok, {major, minor, patch, pre}} - :error -> - :error - end - end + destructure [version_with_pre, build_metadata], String.split(string, "+", parts: 2) + destructure [version, pre], String.split(version_with_pre, "-", parts: 2) + destructure [major, minor, patch], String.split(version, ".") + + with {:ok, major} <- major && parse_digits(major), + {:ok, minor} <- minor && parse_digits(minor), + {:ok, patch} <- parse_patch(patch, approximate?), + {:ok, pre_parts} <- pre != "" && parse_dot_separated(pre), + {:ok, pre_parts} <- convert_integers_in_pre(pre_parts, _acc = []), + {:ok, _build_parts} <- build_metadata != "" && parse_dot_separated(build_metadata) do + {:ok, {major, minor, patch, pre_parts}} else - :error + _other -> :error end end - defp parse_pre(nil), do: {:ok, []} - defp parse_pre(pre), do: parse_pre(String.split(pre, "."), []) + defp parse_digits(string, acc \\ <<>>) + + defp parse_digits(<>, acc) when char in ?0..?9, + do: parse_digits(rest, <>) + defp parse_digits(<<>>, acc) when byte_size(acc) > 0, + do: {:ok, String.to_integer(acc)} + defp parse_digits(<<_::binary>>, _acc), + do: :error + + defp parse_patch(patch, _approximate? = true) when patch in [nil, ""], do: {:ok, nil} + defp parse_patch(patch, _approximate?) when patch in [nil, ""], do: :error + defp parse_patch(patch, _approximate?), do: parse_digits(patch) + + defp parse_dot_separated(nil) do + {:ok, []} + end + + defp parse_dot_separated(string) do + parts = String.split(string, ".") + if Enum.all?(parts, &(&1 != "" and valid_identifier?(&1))) do + {:ok, parts} + else + :error + end + end - defp parse_pre([piece | t], acc) do - cond do - piece =~ ~r/^(0|[1-9][0-9]*)$/ -> - parse_pre(t, [String.to_integer(piece) | acc]) - piece =~ ~r/^[0-9]*$/ -> - :error - true -> - parse_pre(t, [piece | acc]) + defp convert_integers_in_pre([part | rest], acc) do + case parse_digits(part) do + {:ok, int_part} -> + if int_part != 0 and String.starts_with?(part, "0") do + :error + else + convert_integers_in_pre(rest, [int_part | acc]) + end + _other -> + if valid_identifier?(part) do + convert_integers_in_pre(rest, [part | acc]) + else + :error + end end end - defp parse_pre([], acc) do + defp convert_integers_in_pre([], acc) do {:ok, Enum.reverse(acc)} end + defp valid_identifier?(<>) + when char in ?0..?9 + when char in ?a..?z + when char in ?A..?Z + when char == ?- do + valid_identifier?(rest) + end + + defp valid_identifier?(<<>>) do + true + end + + defp valid_identifier?(_other) do + false + end + defp valid_requirement?([]), do: false defp valid_requirement?([a | next]), do: valid_requirement?(a, next) From 78894d90af87157bb26c1d5c7bb66518deaf8b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 8 May 2017 20:48:30 +0200 Subject: [PATCH 2/4] Tidy up naming and nil checks in Version parsing --- lib/elixir/lib/version.ex | 55 +++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/lib/elixir/lib/version.ex b/lib/elixir/lib/version.ex index 34e105ed8ab..d1ba2e202e3 100644 --- a/lib/elixir/lib/version.ex +++ b/lib/elixir/lib/version.ex @@ -400,40 +400,38 @@ defmodule Version do @spec parse_version(String.t) :: {:ok, Version.matchable} | :error def parse_version(string, approximate? \\ false) when is_binary(string) do - destructure [version_with_pre, build_metadata], String.split(string, "+", parts: 2) + destructure [version_with_pre, build], String.split(string, "+", parts: 2) destructure [version, pre], String.split(version_with_pre, "-", parts: 2) destructure [major, minor, patch], String.split(version, ".") - with {:ok, major} <- major && parse_digits(major), - {:ok, minor} <- minor && parse_digits(minor), - {:ok, patch} <- parse_patch(patch, approximate?), - {:ok, pre_parts} <- pre != "" && parse_dot_separated(pre), - {:ok, pre_parts} <- convert_integers_in_pre(pre_parts, _acc = []), - {:ok, _build_parts} <- build_metadata != "" && parse_dot_separated(build_metadata) do + with {:ok, major} <- require_digits(major), + {:ok, minor} <- require_digits(minor), + {:ok, patch} <- maybe_patch(patch, approximate?), + {:ok, pre_parts} <- optional_dot_separated(pre), + {:ok, pre_parts} <- convert_parts_to_integer(pre_parts, []), + {:ok, _build_parts} <- optional_dot_separated(build) do {:ok, {major, minor, patch, pre_parts}} else _other -> :error end end - defp parse_digits(string, acc \\ <<>>) + defp require_digits(nil), do: :error + defp require_digits(string), do: parse_digits(string, "") defp parse_digits(<>, acc) when char in ?0..?9, do: parse_digits(rest, <>) defp parse_digits(<<>>, acc) when byte_size(acc) > 0, do: {:ok, String.to_integer(acc)} - defp parse_digits(<<_::binary>>, _acc), + defp parse_digits(_, _acc), do: :error - defp parse_patch(patch, _approximate? = true) when patch in [nil, ""], do: {:ok, nil} - defp parse_patch(patch, _approximate?) when patch in [nil, ""], do: :error - defp parse_patch(patch, _approximate?), do: parse_digits(patch) + defp maybe_patch(patch, approximate?) + defp maybe_patch(nil, true), do: {:ok, nil} + defp maybe_patch(patch, _), do: require_digits(patch) - defp parse_dot_separated(nil) do - {:ok, []} - end - - defp parse_dot_separated(string) do + defp optional_dot_separated(nil), do: {:ok, []} + defp optional_dot_separated(string) do parts = String.split(string, ".") if Enum.all?(parts, &(&1 != "" and valid_identifier?(&1))) do {:ok, parts} @@ -442,24 +440,19 @@ defmodule Version do end end - defp convert_integers_in_pre([part | rest], acc) do - case parse_digits(part) do - {:ok, int_part} -> - if int_part != 0 and String.starts_with?(part, "0") do - :error - else - convert_integers_in_pre(rest, [int_part | acc]) - end - _other -> - if valid_identifier?(part) do - convert_integers_in_pre(rest, [part | acc]) - else - :error + defp convert_parts_to_integer([part | rest], acc) do + case parse_digits(part, "") do + {:ok, integer} -> + case part do + <> -> :error + _ -> convert_parts_to_integer(rest, [integer | acc]) end + :error -> + convert_parts_to_integer(rest, [part | acc]) end end - defp convert_integers_in_pre([], acc) do + defp convert_parts_to_integer([], acc) do {:ok, Enum.reverse(acc)} end From 81c619f915b39e2e7c423b26b999789d11a30f15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 8 May 2017 20:54:59 +0200 Subject: [PATCH 3/4] Do not allow non leading zeros in version parts --- lib/elixir/lib/version.ex | 14 ++++++++++---- lib/elixir/test/elixir/version_test.exs | 4 ++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/elixir/lib/version.ex b/lib/elixir/lib/version.ex index d1ba2e202e3..8ca28fa4e06 100644 --- a/lib/elixir/lib/version.ex +++ b/lib/elixir/lib/version.ex @@ -417,7 +417,12 @@ defmodule Version do end defp require_digits(nil), do: :error - defp require_digits(string), do: parse_digits(string, "") + defp require_digits(string) do + if leading_zero?(string), do: :error, else: parse_digits(string, "") + end + + defp leading_zero?(<>), do: true + defp leading_zero?(_), do: false defp parse_digits(<>, acc) when char in ?0..?9, do: parse_digits(rest, <>) @@ -443,9 +448,10 @@ defmodule Version do defp convert_parts_to_integer([part | rest], acc) do case parse_digits(part, "") do {:ok, integer} -> - case part do - <> -> :error - _ -> convert_parts_to_integer(rest, [integer | acc]) + if leading_zero?(part) do + :error + else + convert_parts_to_integer(rest, [integer | acc]) end :error -> convert_parts_to_integer(rest, [part | acc]) diff --git a/lib/elixir/test/elixir/version_test.exs b/lib/elixir/test/elixir/version_test.exs index a26d3315c02..ae28b88018e 100644 --- a/lib/elixir/test/elixir/version_test.exs +++ b/lib/elixir/test/elixir/version_test.exs @@ -70,6 +70,10 @@ defmodule VersionTest do assert :error = V.parse("2.3") assert :error = V.parse("2") assert :error = V.parse("2.3.0-01") + assert :error = V.parse("2.3.00-1") + assert :error = V.parse("2.3.00") + assert :error = V.parse("2.03.0") + assert :error = V.parse("02.3.0") end test "Kernek.to_string/1" do From 7e0dbf5b76939dd1473f67f9d4b3b145cce07e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 8 May 2017 20:58:12 +0200 Subject: [PATCH 4/4] More test cases --- lib/elixir/test/elixir/version_test.exs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/elixir/test/elixir/version_test.exs b/lib/elixir/test/elixir/version_test.exs index ae28b88018e..1e43695f1e6 100644 --- a/lib/elixir/test/elixir/version_test.exs +++ b/lib/elixir/test/elixir/version_test.exs @@ -67,8 +67,12 @@ defmodule VersionTest do assert {:ok, %V{major: 1, minor: 4, patch: 5, pre: ["6-g3318bd5"]}} = V.parse("1.4.5-6-g3318bd5+ignore") assert :error = V.parse("foobar") - assert :error = V.parse("2.3") assert :error = V.parse("2") + assert :error = V.parse("2.") + assert :error = V.parse("2.3") + assert :error = V.parse("2.3.") + assert :error = V.parse("2.3.0-") + assert :error = V.parse("2.3.0+") assert :error = V.parse("2.3.0-01") assert :error = V.parse("2.3.00-1") assert :error = V.parse("2.3.00")