Skip to content

feat: SVG text truncation and overflow protection #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
starknetdev merged 2 commits into from
Mar 23, 2026
Merged

feat: SVG text truncation and overflow protection #81

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f89986
feat: add text truncation and clip-path overflow protection to SVG re…
claude Mar 23, 2026
fdcd0f4
fix: remove unused `Into` import in svg.cairo
claude Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 155 additions & 17 deletions packages/utilities/src/renderer/svg.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use core::array::SpanTrait;
use core::clone::Clone;
use core::num::traits::Zero;
use core::traits::Into;
use game_components_embeddable_game_standard::metagame::extensions::context::structs::GameContextDetails;
use game_components_embeddable_game_standard::minigame::extensions::objectives::structs::GameObjectiveDetails;
use game_components_embeddable_game_standard::minigame::extensions::settings::structs::GameSettingDetails;
Expand Down Expand Up @@ -145,6 +144,26 @@ fn uri_encode(input: ByteArray) -> ByteArray {
output
}

/// Truncate a `ByteArray` to `max_chars` visible characters, appending "..." if truncated.
/// Apply BEFORE `uri_encode` so limits are based on visible characters, not encoded bytes.
fn truncate_with_ellipsis(text: ByteArray, max_chars: u32) -> ByteArray {
if text.len() <= max_chars {
return text;
}
let mut result: ByteArray = "";
let limit = max_chars - 3;
let mut i: u32 = 0;
loop {
if i >= limit {
break;
}
result.append_byte(text.at(i).unwrap());
i += 1;
}
result.append(@"...");
result
}
Comment on lines +149 to +165
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of truncate_with_ellipsis has a potential integer underflow issue. If max_chars is less than 3, the expression max_chars - 3 will underflow and cause a panic, as max_chars is a u32.

While the current usages in create_default_svg all use values greater than 3, the function is not robust for general use. It's better to handle this edge case to prevent future panics.

I've refactored the function to handle cases where max_chars <= 3 by simply truncating the text without adding an ellipsis. This version is safer and also slightly more readable by using a while loop.

fn truncate_with_ellipsis(text: ByteArray, max_chars: u32) -> ByteArray {
    if text.len() <= max_chars {
        return text;
    }

    let mut result: ByteArray = "";
    let limit = if max_chars > 3 { max_chars - 3 } else { max_chars };
    let mut i: u32 = 0;
    while i < limit {
        result.append_byte(text.at(i).unwrap());
        i += 1;
    }

    if max_chars > 3 {
        result.append(@"...");
    }
    
    result
}

Comment on lines +147 to +165
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential underflow when max_chars < 3.

If max_chars is less than 3 and text.len() > max_chars, the expression max_chars - 3 will underflow (u32 wraps around), causing the loop to iterate an extremely large number of times.

Current call sites use values ≥ 12, so this is safe in practice. However, consider adding a guard for defensive coding:

🛡️ Proposed defensive fix 
 fn truncate_with_ellipsis(text: ByteArray, max_chars: u32) -> ByteArray {
-    if text.len() <= max_chars {
+    if text.len() <= max_chars || max_chars < 4 {
         return text;
     }

Also note: the doc comment mentions "visible characters" but ByteArray.len() counts bytes. For multi-byte UTF-8 characters, truncation could occur mid-character. This is likely acceptable given the ASCII-dominated inputs (game names, player names), but worth documenting.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/utilities/src/renderer/svg.cairo` around lines 147 - 165, In
truncate_with_ellipsis, guard against underflow when max_chars < 3 by computing
the prefix length with a safe subtraction (e.g., use a saturating or conditional
subtraction: let limit = if max_chars > 3 { max_chars - 3 } else { 0 }) and then
handle the small-max case by returning an appropriate ellipsis-truncated result
(e.g., return the first max_chars bytes of "..." when max_chars <= 3); update
the logic in truncate_with_ellipsis to use this safe limit computation and to
avoid iterating when limit == 0.


fn icon_check(
x: ByteArray, y: ByteArray, w: ByteArray, h: ByteArray, color: @ByteArray,
) -> ByteArray {
Expand Down Expand Up @@ -230,17 +249,17 @@ pub fn create_default_svg(
};
let _game_id = format!("{}", token_metadata.game_id);
let _score = format!("{}", score);
let _game_name = uri_encode(format!("{}", game_metadata.name));
let _developer = uri_encode(format!("{}", game_metadata.developer));
let _genre = uri_encode(format!("{}", game_metadata.genre));
let _game_name = uri_encode(truncate_with_ellipsis(format!("{}", game_metadata.name), 17));
let _developer = uri_encode(truncate_with_ellipsis(format!("{}", game_metadata.developer), 21));
let _genre = uri_encode(truncate_with_ellipsis(format!("{}", game_metadata.genre), 29));
Comment on lines +252 to +254
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The character limits for truncation (17, 21, 29, etc.) are used as magic numbers. This makes the code harder to read and maintain, as the purpose of these specific numbers isn't immediately clear. It's also prone to errors if values need to be updated, as they are scattered throughout the function.

To improve readability and maintainability, please define these limits as constants with descriptive names at the top of the create_default_svg function or in a dedicated constants block. For example:

const GAME_NAME_MAX_CHARS: u32 = 17;
const DEVELOPER_MAX_CHARS: u32 = 21;
const GENRE_MAX_CHARS: u32 = 29;

// ... then use them
let _game_name = uri_encode(truncate_with_ellipsis(format!("{}", game_metadata.name), GAME_NAME_MAX_CHARS));
let _developer = uri_encode(truncate_with_ellipsis(format!("{}", game_metadata.developer), DEVELOPER_MAX_CHARS));
let _genre = uri_encode(truncate_with_ellipsis(format!("{}", game_metadata.genre), GENRE_MAX_CHARS));

This should be applied to all magic numbers used with truncate_with_ellipsis in this function.

let desc_raw = game_metadata.description;
let _settings_name: ByteArray = if settings_details.name.len() > 0 {
uri_encode(settings_details.name)
uri_encode(truncate_with_ellipsis(settings_details.name, 24))
} else {
"---"
};
let _objective_name: ByteArray = if objective_details.name.len() > 0 {
uri_encode(objective_details.name)
uri_encode(truncate_with_ellipsis(objective_details.name, 24))
} else {
"---"
};
Expand All @@ -250,7 +269,7 @@ pub fn create_default_svg(

let mut _player_name: ByteArray = Default::default();
if player_name.is_non_zero() {
_player_name = uri_encode(felt252_to_byte_array(player_name));
_player_name = uri_encode(truncate_with_ellipsis(felt252_to_byte_array(player_name), 12));
} else {
_player_name = "---";
}
Expand All @@ -266,7 +285,7 @@ pub fn create_default_svg(

// Context details
let _context_name: ByteArray = if context_details.name.len() > 0 {
uri_encode(context_details.name.clone())
uri_encode(truncate_with_ellipsis(context_details.name.clone(), 42))
} else {
"---"
};
Expand Down Expand Up @@ -360,6 +379,19 @@ pub fn create_default_svg(

// Clip path for inward-only border stroke
svg.append(@"<clipPath id='card-clip'><rect width='470' height='600' rx='16'/></clipPath>");
svg.append(@"<clipPath id='pn-clip'><rect x='302' y='16' width='150' height='56'/></clipPath>");
svg
.append(
@"<clipPath id='lp-clip'><rect x='25' y='276' width='205' height='108'/></clipPath>",
);
svg
.append(
@"<clipPath id='rp-clip'><rect x='240' y='276' width='205' height='108'/></clipPath>",
);
svg
.append(
@"<clipPath id='ctx-clip'><rect x='25' y='458' width='420' height='68'/></clipPath>",
);

// Styles with card rotation and edge depth
svg
Expand Down Expand Up @@ -422,7 +454,7 @@ pub fn create_default_svg(
// Player name (top right)
svg
.append(
@"<text x='440' y='41' text-anchor='end' style='fill:%23fff;font-size:18px;letter-spacing:1px'>",
@"<text x='440' y='41' text-anchor='end' clip-path='url(%23pn-clip)' style='fill:%23fff;font-size:18px;letter-spacing:1px'>",
);
svg += _player_name.clone();
svg.append(@"</text>");
Expand Down Expand Up @@ -706,9 +738,12 @@ pub fn create_default_svg(
@"<svg x='252' y='285' width='14' height='14' viewBox='0 0 16 16'><path fill='none' stroke='%23c9c9d1' stroke-width='2' stroke-linecap='round' d='M6.5 9.5a3.5 3.5 0 005 0l2-2a3.5 3.5 0 00-5-5l-1 1'/><path fill='none' stroke='%23c9c9d1' stroke-width='2' stroke-linecap='round' d='M9.5 6.5a3.5 3.5 0 00-5 0l-2 2a3.5 3.5 0 005 5l1-1'/></svg>",
);
svg.append(@"<text x='271' y='297' class='l'>CLIENT URL</text>");
svg.append(@"<text x='252' y='316' class='vs' style='font-size:9px'>");
svg
.append(
@"<text x='252' y='316' clip-path='url(%23rp-clip)' class='vs' style='font-size:9px'>",
);
if client_url.len() > 0 {
svg += uri_encode(client_url);
svg += uri_encode(truncate_with_ellipsis(client_url, 35));
} else {
svg.append(@"---");
}
Expand All @@ -728,7 +763,7 @@ pub fn create_default_svg(
@"<svg x='37' y='343' width='14' height='14' viewBox='0 0 16 16'><path fill='%23c9c9d1' d='M6.8 1h2.4l.4 2 .7.3 1.7-1.1 1.7 1.7-1.1 1.7.3.7 2 .4v2.4l-2 .4-.3.7 1.1 1.7-1.7 1.7-1.7-1.1-.7.3-.4 2H6.8l-.4-2-.7-.3-1.7 1.1-1.7-1.7 1.1-1.7-.3-.7-2-.4V6.8l2-.4.3-.7L3.2 4l1.7-1.7 1.7 1.1.7-.3z'/><circle fill='%231e1e22' cx='8' cy='8' r='2.5'/></svg>",
);
svg.append(@"<text x='56' y='355' class='l'>SETTINGS</text>");
svg.append(@"<text x='37' y='374' class='vs'>");
svg.append(@"<text x='37' y='374' clip-path='url(%23lp-clip)' class='vs'>");
svg += _settings_name;
svg.append(@"</text>");

Expand All @@ -742,7 +777,7 @@ pub fn create_default_svg(
svg.append(@"'/>");
svg += icon_target("252", "343", "14", "14", @"%23c9c9d1");
svg.append(@"<text x='271' y='355' class='l'>OBJECTIVE</text>");
svg.append(@"<text x='252' y='374' class='vs'>");
svg.append(@"<text x='252' y='374' clip-path='url(%23rp-clip)' class='vs'>");
svg += _objective_name;
svg.append(@"</text>");

Expand Down Expand Up @@ -793,7 +828,7 @@ pub fn create_default_svg(
svg.append(@accent);
svg.append(@"'/>");
svg.append(@"<text x='37' y='476' class='l'>CONTEXT</text>");
svg.append(@"<text x='112' y='476' class='vs'>");
svg.append(@"<text x='112' y='476' clip-path='url(%23ctx-clip)' class='vs'>");
svg += _context_name;
svg.append(@"</text>");

Expand All @@ -817,9 +852,9 @@ pub fn create_default_svg(
svg.append(@"<text x='37' y='");
svg += format!("{}", y_pos);
svg.append(@"' style='fill:%23888;font-size:10px'>");
svg += uri_encode(felt252_to_byte_array(*entry.name));
svg += uri_encode(truncate_with_ellipsis(felt252_to_byte_array(*entry.name), 15));
svg.append(@": ");
svg += uri_encode(felt252_to_byte_array(*entry.value));
svg += uri_encode(truncate_with_ellipsis(felt252_to_byte_array(*entry.value), 35));
svg.append(@"</text>");
ctx_i += 1;
};
Expand Down Expand Up @@ -866,7 +901,10 @@ mod tests {
use game_components_embeddable_game_standard::registry::interface::GameMetadata;
use game_components_embeddable_game_standard::token::structs::{Lifecycle, TokenMetadata};
use snforge_std::{start_cheat_block_timestamp_global, stop_cheat_block_timestamp_global};
use super::{calculate_timeline_progress, create_default_svg, timestamp_to_datetime};
use super::{
calculate_timeline_progress, create_default_svg, timestamp_to_datetime,
truncate_with_ellipsis,
};

fn default_token_metadata() -> TokenMetadata {
TokenMetadata {
Expand Down Expand Up @@ -1057,4 +1095,104 @@ mod tests {

println!("Loot Survivor SVG: {}", svg_result);
}

#[test]
fn test_truncate_with_ellipsis_short_text() {
let result = truncate_with_ellipsis("Hello", 10);
assert!(result == "Hello", "Short text should not be truncated");
}

#[test]
fn test_truncate_with_ellipsis_exact_length() {
let result = truncate_with_ellipsis("1234567890", 10);
assert!(result == "1234567890", "Exact length should not be truncated");
}

#[test]
fn test_truncate_with_ellipsis_truncated() {
let result = truncate_with_ellipsis("Hello World!!", 10);
assert!(result == "Hello W...", "Should truncate with ellipsis");
}

#[test]
fn test_truncate_with_ellipsis_boundary() {
let result = truncate_with_ellipsis("12345678901", 10);
assert!(result == "1234567...", "One over limit should truncate");
}

#[test]
fn test_truncate_with_ellipsis_empty() {
let result = truncate_with_ellipsis("", 10);
assert!(result == "", "Empty text should remain empty");
}

#[test]
fn test_svg_overflow_stress() {
start_cheat_block_timestamp_global(1656763200);

let game_metadata = GameMetadata {
contract_address: 0x1234567890123456789012345678901234567890.try_into().unwrap(),
name: "Extremely Long Game Name That Overflows",
description: "A purposefully verbose description to verify word wrapping still works after the truncation changes are applied to the card.",
developer: "Very Long Developer Studio Name",
publisher: "Publisher",
genre: "Action-Adventure-RPG-Roguelike-Sim",
image: "https://example.com/img.png",
color: "#FF6600",
client_url: "https://example.com",
renderer_address: 0x9876543210987654321098765432109876543210.try_into().unwrap(),
royalty_fraction: 1250,
skills_address: 0.try_into().unwrap(),
created_at: 0,
version: 0,
};

let mut token_metadata = default_token_metadata();
token_metadata.soulbound = true;
token_metadata.objective_id = 1;
token_metadata.paymaster = true;

let settings = GameSettingDetails {
name: "Ultra Nightmare Difficulty Mode",
description: "Maximum difficulty",
settings: array![GameSetting { name: 'Difficulty', value: 'Nightmare' }].span(),
};

let objectives = GameObjectiveDetails {
name: "Collect Every Single Artifact",
description: "Find all artifacts",
objectives: array![GameObjective { name: 'Artifacts', value: '100' }].span(),
};

let context = GameContextDetails {
name: "Season 42 Championship Grand Finals Invitational Tournament",
description: "The big one",
id: Option::Some(42),
context: array![
GameContext { name: 'TournamentRound', value: 'GrandChampionshipFinal' },
GameContext { name: 'Region', value: 'NorthAmericaEastCoast' },
GameContext { name: 'Bracket', value: 'WinnersSemiFinalRound' },
]
.span(),
};

// 29-byte player name — near felt252 max
let long_player_name: felt252 = 'xXxDarkLordSlayer9000ProMaxxXx';

let svg_result = create_default_svg(
game_metadata,
token_metadata,
999999999,
long_player_name,
settings,
objectives,
context,
"https://very-long-client-url-that-should-definitely-be-truncated.example.com/game/play",
);

stop_cheat_block_timestamp_global();

assert!(svg_result.len() > 0, "SVG should be generated with overflow inputs");
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The assertion assert!(svg_result.len() > 0, ...) in test_svg_overflow_stress is quite weak. It only confirms that an SVG string was generated, but doesn't verify that truncation is actually working as expected.

To make this test more meaningful, you could add assertions to check that the generated SVG string contains the ellipsis (...), which would indicate that truncation has occurred. Since ByteArray doesn't have a built-in contains method, you might need a small helper for this.

A stronger assertion would provide more confidence that the overflow protection is effective. For example, you could check for the presence of ... which is expected due to multiple fields being truncated.

println!("OVERFLOW STRESS SVG:\n{}", svg_result);
}
}
Loading