Skip to content

Commit 2cb3c1c

Browse files
committed
Add server-side URL validation for absolute URLs to reduce flicker
Broken Gravatar links (absolute URLs) now render fallback SVG immediately instead of showing a brief broken image before client-side JS swaps it.
1 parent be01160 commit 2cb3c1c

7 files changed

Lines changed: 105 additions & 17 deletions

File tree

app/components/primer/open_project/avatar_with_fallback.rb

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# frozen_string_literal: true
22

3+
require "net/http"
4+
require "uri"
5+
36
module Primer
47
module OpenProject
58
# OpenProject-specific Avatar component that extends Primer::Beta::Avatar
@@ -16,6 +19,9 @@ module OpenProject
1619
class AvatarWithFallback < Primer::Beta::Avatar
1720
status :open_project
1821

22+
# Set to false to disable server-side URL validation (useful for tests)
23+
mattr_accessor :validate_urls, default: true
24+
1925
# @see
2026
# - https://primer.style/foundations/typography/
2127
# - https://github.com/primer/css/blob/main/src/support/variables/typography.scss
@@ -33,7 +39,7 @@ def initialize(src: nil, alt: nil, size: DEFAULT_SIZE, shape: DEFAULT_SHAPE, hre
3339

3440
@unique_id = unique_id
3541
@fallback_svg = generate_fallback_svg(alt, size)
36-
final_src = src.blank? ? @fallback_svg : src
42+
final_src = resolve_src(src)
3743

3844
super(src: final_src, alt: alt, size: size, shape: shape, href: href, **system_arguments)
3945
end
@@ -59,6 +65,33 @@ def require_src_or_alt_arguments(src, alt)
5965
raise ArgumentError, "`src` or `alt` is required"
6066
end
6167

68+
def resolve_src(src)
69+
return @fallback_svg if src.blank?
70+
71+
if validate_urls
72+
return @fallback_svg if absolute_url?(src) && !url_accessible?(src)
73+
end
74+
75+
src
76+
end
77+
78+
def absolute_url?(url)
79+
url.to_s.match?(%r{\Ahttps?://}i)
80+
end
81+
82+
def url_accessible?(url)
83+
uri = URI.parse(url)
84+
http = Net::HTTP.new(uri.host, uri.port)
85+
http.use_ssl = uri.scheme == "https"
86+
http.open_timeout = 2
87+
http.read_timeout = 2
88+
89+
response = http.head(uri.request_uri)
90+
response.is_a?(Net::HTTPSuccess)
91+
rescue StandardError
92+
false
93+
end
94+
6295
def generate_fallback_svg(alt, size)
6396
svg_content = content_tag(
6497
:svg,

previews/primer/open_project/avatar_with_fallback_preview.rb

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,37 @@ def fallback_as_link
6969

7070
# @!group Error Handling (404 Fallback)
7171
#
72-
# @label Broken image (404)
72+
# @label Broken absolute URL (server-side fallback)
7373
# @snapshot
74-
def broken_image_404
75-
# Uses a non-existent URL - will trigger error handler and show fallback SVG
74+
def broken_absolute_url
75+
# Absolute URLs are validated server-side via HEAD request
76+
# If inaccessible, fallback SVG is rendered immediately (no flicker)
7677
render(Primer::OpenProject::AvatarWithFallback.new(
77-
src: "/non-existent-avatar.png",
78+
src: "https://example.com/non-existent-avatar.png",
7879
alt: "User With Missing Avatar",
7980
unique_id: 42
8081
))
8182
end
8283

83-
# @label Multiple broken images
84-
def multiple_broken_images
84+
# @label Multiple broken absolute URLs (server-side)
85+
def multiple_broken_absolute_urls
86+
render_with_template(locals: {})
87+
end
88+
89+
# @label Broken relative URL (client-side fallback)
90+
# @snapshot
91+
def broken_relative_url
92+
# Relative URLs cannot be validated server-side (no host context)
93+
# Client-side JS handles the error and swaps to fallback SVG
94+
render(Primer::OpenProject::AvatarWithFallback.new(
95+
src: "/non-existent-avatar.png",
96+
alt: "User With Missing Avatar",
97+
unique_id: 43
98+
))
99+
end
100+
101+
# @label Multiple broken relative URLs (client-side)
102+
def multiple_broken_relative_urls
85103
render_with_template(locals: {})
86104
end
87105
#
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<div class="d-flex gap-3 flex-items-center">
2+
<p class="color-fg-muted">Absolute URLs are validated server-side. Broken URLs render fallback immediately (no flicker):</p>
3+
</div>
4+
<div class="d-flex gap-3 flex-items-center mt-2">
5+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/missing/alice.png", alt: "Alice Johnson", unique_id: 10)) %>
6+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/missing/bob.png", alt: "Bob Smith", unique_id: 20)) %>
7+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://invalid-domain-12345.com/avatar.png", alt: "Charlie Brown", unique_id: 30)) %>
8+
</div>

previews/primer/open_project/avatar_with_fallback_preview/multiple_broken_images.html.erb

Lines changed: 0 additions & 10 deletions
This file was deleted.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<div class="d-flex gap-3 flex-items-center">
2+
<p class="color-fg-muted">Relative URLs are handled client-side. JS swaps to fallback on error (may show brief flicker):</p>
3+
</div>
4+
<div class="d-flex gap-3 flex-items-center mt-2">
5+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "/missing/alice.png", alt: "Alice Johnson", unique_id: 10)) %>
6+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "/missing/bob.png", alt: "Bob Smith", unique_id: 20)) %>
7+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "/avatars/charlie.png", alt: "Charlie Brown", unique_id: 30)) %>
8+
</div>

test/components/open_project/avatar_stack_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@
55
class PrimerOpenProjectAvatarStackTest < Minitest::Test
66
include Primer::ComponentTestHelpers
77

8+
def setup
9+
@original_validate_urls = Primer::OpenProject::AvatarWithFallback.validate_urls
10+
Primer::OpenProject::AvatarWithFallback.validate_urls = false
11+
end
12+
13+
def teardown
14+
Primer::OpenProject::AvatarWithFallback.validate_urls = @original_validate_urls
15+
end
16+
817
def test_renders_with_image_avatars
918
render_inline(Primer::OpenProject::AvatarStack.new) do |component|
1019
component.with_avatar_with_fallback(src: "https://github.com/github.png", alt: "@github")

test/components/open_project/avatar_with_fallback_test.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@
55
class PrimerOpenProjectAvatarWithFallbackTest < Minitest::Test
66
include Primer::ComponentTestHelpers
77

8+
def setup
9+
# Disable URL validation by default in tests to avoid HTTP requests
10+
@original_validate_urls = Primer::OpenProject::AvatarWithFallback.validate_urls
11+
Primer::OpenProject::AvatarWithFallback.validate_urls = false
12+
end
13+
14+
def teardown
15+
Primer::OpenProject::AvatarWithFallback.validate_urls = @original_validate_urls
16+
end
17+
818
def test_renders_image_avatar_with_src
919
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://github.com/github.png", alt: "github"))
1020

@@ -31,6 +41,18 @@ def test_image_avatar_error_handling_setup
3141
assert_includes svg_content, ">AJ<", "Fallback SVG should contain initials 'AJ'"
3242
end
3343

44+
def test_falls_back_when_absolute_url_is_inaccessible
45+
Primer::OpenProject::AvatarWithFallback.validate_urls = true
46+
Primer::OpenProject::AvatarWithFallback.any_instance.stubs(:url_accessible?).returns(false)
47+
48+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://broken.example.com/avatar.png", alt: "Test User", unique_id: 42))
49+
50+
# Should render fallback SVG when URL is inaccessible
51+
assert_selector("avatar-fallback[data-unique-id='42']") do
52+
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
53+
end
54+
end
55+
3456
def test_renders_fallback_when_src_is_nil
3557
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "OpenProject Admin"))
3658

0 commit comments

Comments
 (0)