bug: fix error when a link contains a line break (#186)

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

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

lib/qrstorage_web/templates/qr_code/download.html.heex

@@ -62,10 +62,12 @@
     <li><a class="dropdown-item" data-file-type="png" href="#"><%= gettext("PNG") %></a></li>
   </ul>
 
-  <%= 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 %>
 </div>
 
 <br />

lib/qrstorage_web/templates/qr_code/form_link.html.heex

@@ -15,13 +15,12 @@
   <% end %>
 
   <div class="form-group">
-    <%= 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]) %>
     <p class="form-text text-muted">
       <%= gettext(
         "This tool may only be used in Teaching/Learning situations. It is not permitted to enter personal information."

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
 

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) =~ "<a href=\"/\">"
     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) =~ "<a href=\"/\">"
     end
   end