diff --git a/lib/qrstorage/qr_codes/qr_code.ex b/lib/qrstorage/qr_codes/qr_code.ex index 91c95c2..921af7b 100644 --- a/lib/qrstorage/qr_codes/qr_code.ex +++ b/lib/qrstorage/qr_codes/qr_code.ex @@ -178,13 +178,27 @@ defmodule Qrstorage.QrCodes.QrCode do String.length(text) end - # This is a very simple check - it just verifies host/scheme. defp valid_url?(url) when is_binary(url) do - case URI.parse(url) do - %URI{host: nil} -> false - %URI{scheme: nil} -> false - %URI{} -> true - end + # links should have a host and a scheme - this is not covered by uri_new: + uri_parse = + case URI.parse(url) do + %URI{host: nil} -> + false + + %URI{scheme: nil} -> + false + + %URI{} -> + true + end + + uri_new = + case URI.new(url) do + {:ok, _} -> true + {:error, _} -> false + end + + uri_parse && uri_new end defp valid_url?(_), do: false diff --git a/lib/qrstorage_web/controllers/qr_code_controller.ex b/lib/qrstorage_web/controllers/qr_code_controller.ex index 9c0e086..cee290a 100644 --- a/lib/qrstorage_web/controllers/qr_code_controller.ex +++ b/lib/qrstorage_web/controllers/qr_code_controller.ex @@ -45,7 +45,8 @@ defmodule QrstorageWeb.QrCodeController do qr_code = QrCodes.get_qr_code!(id) if qr_code.content_type == :link do - redirect(conn, external: qr_code.text) + # we don't want to redirect to external links: + redirect(conn, to: "/") else render(conn, "show.html", qr_code: qr_code) end @@ -55,7 +56,8 @@ defmodule QrstorageWeb.QrCodeController do qr_code = QrCodes.get_qr_code!(id) if qr_code.content_type == :link do - redirect(conn, external: qr_code.text) + # we don't want to redirect to links: + redirect(conn, to: "/") else render(conn, "preview.html", qr_code: qr_code) end diff --git a/lib/qrstorage_web/templates/qr_code/download.html.heex b/lib/qrstorage_web/templates/qr_code/download.html.heex index 59c305a..77e5abf 100644 --- a/lib/qrstorage_web/templates/qr_code/download.html.heex +++ b/lib/qrstorage_web/templates/qr_code/download.html.heex @@ -62,10 +62,12 @@
  • <%= gettext("PNG") %>
  • - <%= link(gettext("Show"), - class: "btn btn-primary", - to: Routes.qr_code_path(@conn, :preview, @qr_code.id) - ) %> + <%= if @qr_code.content_type !== :link do + link(gettext("Show"), + class: "btn btn-primary", + to: Routes.qr_code_path(@conn, :preview, @qr_code.id) + ) + end %>
    diff --git a/lib/qrstorage_web/templates/qr_code/form_link.html.heex b/lib/qrstorage_web/templates/qr_code/form_link.html.heex index 7b3229e..bdd2a1f 100644 --- a/lib/qrstorage_web/templates/qr_code/form_link.html.heex +++ b/lib/qrstorage_web/templates/qr_code/form_link.html.heex @@ -15,13 +15,12 @@ <% end %>
    - <%= textarea(f, :text, + <%= text_input(f, :text, class: "form-control", required: true, maxlength: QrCode.text_length_limits()[:link], placeholder: gettext("Type your link here") ) %> - <%= render("form_character_counter.html", max_length: QrCode.text_length_limits()[:link]) %>

    <%= gettext( "This tool may only be used in Teaching/Learning situations. It is not permitted to enter personal information." diff --git a/test/qrstorage/qr_codes_test.exs b/test/qrstorage/qr_codes_test.exs index af17fff..2d32e69 100644 --- a/test/qrstorage/qr_codes_test.exs +++ b/test/qrstorage/qr_codes_test.exs @@ -28,6 +28,16 @@ defmodule Qrstorage.QrCodesTest do dots_type: "dots" } + @valid_link_attrs %{ + delete_after: ~D[2010-04-17], + text: "https://kits.blog", + hide_text: false, + content_type: "link", + language: nil, + hp: nil, + dots_type: "dots" + } + @attrs_without_hide_text %{ delete_after: ~D[2010-04-17], text: "some text", @@ -82,19 +92,31 @@ defmodule Qrstorage.QrCodesTest do end test "create_qr_code/1 with type link and invalid url returns error changeset" do - invalid_link_attrs = %{@valid_attrs | content_type: "link", text: "invalid"} + invalid_link_attrs = %{@valid_link_attrs | text: "invalid"} + + assert {:error, %Ecto.Changeset{}} = QrCodes.create_qr_code(invalid_link_attrs) + end + + test "create_qr_code/1 with type link and invalid url with line break returns error changeset" do + invalid_link_attrs = %{@valid_link_attrs | text: "https://kits.blog\n\r"} + + assert {:error, %Ecto.Changeset{}} = QrCodes.create_qr_code(invalid_link_attrs) + end + + test "create_qr_code/1 with type link and invalid url with white spaces returns error changeset" do + invalid_link_attrs = %{@valid_link_attrs | text: "https://kits.blog/ abc"} assert {:error, %Ecto.Changeset{}} = QrCodes.create_qr_code(invalid_link_attrs) end test "create_qr_code/1 with type link and valid url returns ok" do - valid_link_attrs = %{@valid_attrs | content_type: "link", text: "https://kits.blog"} + valid_link_attrs = %{@valid_link_attrs | text: "https://kits.blog"} assert {:ok, %QrCode{} = _qr_code} = QrCodes.create_qr_code(valid_link_attrs) end test "create_qr_code/1 with link with uppercase letters is valid" do - valid_link_attrs = %{@valid_attrs | content_type: "link", text: "HTTPS://KITS.blog"} + valid_link_attrs = %{@valid_link_attrs | text: "HTTPS://KITS.blog"} assert {:ok, %QrCode{} = _qr_code} = QrCodes.create_qr_code(valid_link_attrs) end @@ -208,7 +230,7 @@ defmodule Qrstorage.QrCodesTest do end test "create_qr_code/1 with link type and honeypot set returns error changeset" do - invalid_link_attrs = %{@valid_attrs | content_type: "link", hp: "set"} + invalid_link_attrs = %{@valid_link_attrs | hp: "set"} assert {:error, %Ecto.Changeset{}} = QrCodes.create_qr_code(invalid_link_attrs) end diff --git a/test/qrstorage_web/controllers/qr_code_controller_test.exs b/test/qrstorage_web/controllers/qr_code_controller_test.exs index 327e2cc..feec441 100644 --- a/test/qrstorage_web/controllers/qr_code_controller_test.exs +++ b/test/qrstorage_web/controllers/qr_code_controller_test.exs @@ -194,12 +194,12 @@ defmodule QrstorageWeb.QrCodeControllerTest do assert html_response(conn, 200) =~ audio_qr_code.text end - test "that links redirect to the link", %{ + test "that links redirect to the default page", %{ conn: conn, link_qr_code: link_qr_code } do conn = get(conn, Routes.qr_code_path(conn, :show, link_qr_code.id)) - assert html_response(conn, 302) =~ link_qr_code.text + assert html_response(conn, 302) =~ "" end test "that codes with translated text prefer the translation", %{ @@ -263,12 +263,12 @@ defmodule QrstorageWeb.QrCodeControllerTest do assert html_response(conn, 200) =~ text_qr_code.text end - test "that links redirect to the link", %{ + test "that links redirect to the default page", %{ conn: conn, link_qr_code: link_qr_code } do conn = get(conn, Routes.qr_code_path(conn, :preview, link_qr_code.id)) - assert html_response(conn, 302) =~ link_qr_code.text + assert html_response(conn, 302) =~ "" end end