diff --git a/app/javascript/pages/Projects/Index.svelte b/app/javascript/pages/Projects/Index.svelte index 9e5214c72..d5776218c 100644 --- a/app/javascript/pages/Projects/Index.svelte +++ b/app/javascript/pages/Projects/Index.svelte @@ -154,7 +154,7 @@ restoring ? myProjectRepoMappings.unarchive : myProjectRepoMappings.archive - ).path({ projectName: project.project_key }); + ).path({ projectName: encodeURIComponent(project.project_key) }); pendingStatusAction = { path, diff --git a/app/javascript/pages/Projects/components/ProjectCard.svelte b/app/javascript/pages/Projects/components/ProjectCard.svelte index bee75bd41..6d5ee36f1 100644 --- a/app/javascript/pages/Projects/components/ProjectCard.svelte +++ b/app/javascript/pages/Projects/components/ProjectCard.svelte @@ -34,11 +34,30 @@ } = $props(); const key = $derived(project.url_safe ? project.project_key : null); + + // URL-encode the project name before passing to `js_from_routes`' path + // helpers. `interpolateUrl` misreads a colon followed by letters (e.g. + // ":R") as an unfulfilled route parameter and throws — encoding the colon + // to %3A avoids that. Rails decodes %3A back to ":" when matching the + // route, so the controller receives the original name. Wrap in try/catch + // as defense-in-depth so any future edge case degrades to "no link" + // instead of nuking the whole Projects virtualizer (#1414). + const safePath = (build: () => string): string | null => { + if (!key) return null; + try { + return build(); + } catch { + return null; + } + }; + + const encodedKey = $derived(key ? encodeURIComponent(key) : null); + const showPath = $derived( - key ? myProjectRepoMappings.show.path({ projectName: key }) : null, + safePath(() => myProjectRepoMappings.show.path({ projectName: encodedKey! })), ); const updatePath = $derived( - key ? myProjectRepoMappings.update.path({ projectName: key }) : null, + safePath(() => myProjectRepoMappings.update.path({ projectName: encodedKey! })), ); const projectHref = $derived( showPath diff --git a/app/javascript/pages/Projects/components/ShareModal.svelte b/app/javascript/pages/Projects/components/ShareModal.svelte index afeee3227..263df9f58 100644 --- a/app/javascript/pages/Projects/components/ShareModal.svelte +++ b/app/javascript/pages/Projects/components/ShareModal.svelte @@ -31,7 +31,7 @@ const toggleShare = () => { toggling = true; router.patch( - myProjectRepoMappings.toggleShare.path({ projectName }), + myProjectRepoMappings.toggleShare.path({ projectName: encodeURIComponent(projectName) }), {}, { preserveScroll: true, onFinish: () => (toggling = false) }, ); diff --git a/test/system/projects_with_unsafe_names_test.rb b/test/system/projects_with_unsafe_names_test.rb new file mode 100644 index 000000000..94c0826f4 --- /dev/null +++ b/test/system/projects_with_unsafe_names_test.rb @@ -0,0 +1,123 @@ +require "application_system_test_case" + +class ProjectsWithUnsafeNamesTest < ApplicationSystemTestCase + setup do + @user = User.create!(timezone: "UTC") + sign_in_as(@user) + end + + # Each unsafe character that previously broke the Projects page or archive + # action. The fix URL-encodes the project name on the client before passing + # it to `js_from_routes`' path helpers, so the card stays clickable and the + # server receives the original name after Rails decodes the path param. + UNSAFE_NAMES = [ + "myproject:R", # #1414 — colon + letter crashes js_from_routes + "foo:Hello", # another colon + letter variant + ":bar", # leading colon + "a:b:c", # multiple colons + "slash/project", # #1442 — slash breaks Rails routing on archive + "foo?bar", # question mark (query delimiter) + "foo#bar", # hash (fragment delimiter) + "foo%bar", # literal percent (would double-decode with old CGI.unescape) + "Aritmética", # unicode (should always work) + "normal-project" # control — normal name + ].freeze + + test "projects page renders all unsafe-named projects as clickable cards" do + UNSAFE_NAMES.each do |name| + create_project_heartbeats(@user, name, started_at: 2.days.ago.noon) + end + DashboardRollupRefreshService.new(user: @user).call + + visit my_projects_path + + # Every project should render — no card should crash the virtualizer. + UNSAFE_NAMES.each { |name| assert_text name } + + # None should show the "broken name" badge — URL-unsafe chars are not + # structurally invalid, they're handled by URL-encoding on the client. + assert_no_text "Time can't be used in Hack Club programs" + + # Each card should have a clickable link overlay with the URL-encoded + # project name in the href. + UNSAFE_NAMES.each do |name| + link = find("a[aria-label='View #{name}']", wait: 5) + encoded = URI.encode_www_form_component(name) + assert_includes link[:href], encoded, + "expected link href for #{name.inspect} to contain #{encoded}, got #{link[:href]}" + end + end + + test "clicking a colon-named project navigates to its detail page (#1414)" do + create_project_heartbeats(@user, "myproject:R", started_at: 2.days.ago.noon) + DashboardRollupRefreshService.new(user: @user).call + + visit my_projects_path + + assert_text "myproject:R" + assert_no_text "Time can't be used in Hack Club programs" + + link = find("a[aria-label='View myproject:R']") + assert_includes link[:href], "myproject%3AR" + + visit link[:href] + assert_text "myproject:R" + end + + test "clicking a slash-named project navigates to its detail page (#1442)" do + create_project_heartbeats(@user, "slash/project", started_at: 2.days.ago.noon) + DashboardRollupRefreshService.new(user: @user).call + + visit my_projects_path + + assert_text "slash/project" + assert_no_text "Time can't be used in Hack Club programs" + + link = find("a[aria-label='View slash/project']") + assert_includes link[:href], "slash%2Fproject" + + visit link[:href] + assert_text "slash/project" + end + + test "archiving a slash-named project works (#1442)" do + @user.project_repo_mappings.create!(project_name: "slash/project") + create_project_heartbeats(@user, "slash/project", started_at: 2.days.ago.noon) + DashboardRollupRefreshService.new(user: @user).call + + visit my_projects_path + + assert_text "slash/project" + + within find("article", text: "slash/project") do + find("button[title='Archive project']").click + end + + within find("div[role='dialog']", wait: 5) do + click_on "Archive project" + end + + assert_text "Away it goes!" + end + + private + + def create_project_heartbeats(user, project_name, started_at:) + user.project_repo_mappings.find_or_create_by!(project_name: project_name) + + Heartbeat.create!( + user: user, + project: project_name, + category: "coding", + time: started_at.to_i, + source_type: :test_entry + ) + Heartbeat.create!( + user: user, + project: project_name, + category: "coding", + time: (started_at + 30.minutes).to_i, + source_type: :test_entry + ) + end +end