-
Notifications
You must be signed in to change notification settings - Fork 1
fix(icons): reduce max_url_length to 3550 and add camo limit warnings #109
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.
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
Conversation
Co-authored-by: ChipWolf <3164166+ChipWolf@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes GitHub camo proxy URL length limit issues by reducing the max_url_length parameter from 3700 to 3550 characters. The tighter limit ensures that badge URLs with embedded SVG data URIs stay under GitHub's camo proxy 8192 character limit, preventing "limit exceeded" errors for large icons like the Amazon AWS icon.
Key Changes
- Reduced default
max_url_lengthfrom 3700 to 3550 insvg_to_base64_data_uri()function - Updated all explicit call sites to use 3550
- Updated test assertions to reflect the new default value
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| badgesort/icons.py | Updated function signature default parameter, docstring calculation, and two explicit call sites to use 3550 char limit |
| tests/test_camo_url_limits.py | Updated test assertions and comments to verify the new 3550 char default value |
badgesort/icons.py
Outdated
| Default 3550 chars ensures badge URLs stay under GitHub's camo proxy | ||
| 8192 char limit (which hex-encodes URLs: 76 + url_len*2 <= 8192). | ||
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3850, with 150 char margin = 3700 | ||
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3949, with 10% margin = 3550 |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The calculation in the docstring could be clearer. Consider specifying what "overhead" refers to:
Calculation: safe_url = (8192 - 76) / 2 - ~109 badge_url_overhead ≈ 3949, with 10% margin = 3550or
Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550This makes it clear that the ~109 char overhead is the badge URL structure (domain, path, parameters) excluding the actual data URI payload.
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3949, with 10% margin = 3550 | |
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550 | |
| (~109 char badge_url_overhead is the badge URL structure: domain, path, parameters, excluding the data URI payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the docstring for better clarity in commit fb06706. The calculation now explicitly labels each overhead component and explains what badge_url_overhead represents.
|
The test failed in CI @copilot |
Co-authored-by: ChipWolf <3164166+ChipWolf@users.noreply.github.com>
... Fixed in commit fb06706. The CI failure was due to missing |
|
@copilot Use the BadgeSort container in CI instead of this |
Co-authored-by: ChipWolf <3164166+ChipWolf@users.noreply.github.com>
Updated in commit 0ba8bc1. CI now builds and uses the BadgeSort Docker container to run tests, which includes all system dependencies (librsvg2-bin) by default. Also added console warnings when badge URLs exceed the GitHub camo limit or when PNG fallback fails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| Default 3550 chars ensures badge URLs stay under GitHub's camo proxy | ||
| 8192 char limit (which hex-encodes URLs: 76 + url_len*2 <= 8192). | ||
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3850, with 150 char margin = 3700 | ||
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550 |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The calculation description states "with 10% margin = 3550" but this is slightly unclear. The actual calculation would be: 3949 × 0.9 ≈ 3554, but the code uses 3550 (which is even more conservative). Consider clarifying the description to either:
- "with ~10% margin (rounded down) = 3550" or
- "with 399 char safety margin (≈10%) = 3550"
This would make it clearer that 3550 is a rounded-down value for safety.
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550 | |
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with ~10% margin (rounded down) = 3550 |
Fix failing camo length test
This PR fixes GitHub camo proxy URL length limit issues by reducing the
max_url_lengthparameter from 3700 to 3550 characters and adding user-facing warnings when URLs exceed safe limits.Changes Made
max_url_length: Changed default from 3700 to 3550 chars insvg_to_base64_data_uri()to account for badge URL overhead and provide 10% safety marginTechnical Details
Calculation:
(8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550The tighter limit ensures that badge URLs with embedded SVG data URIs stay under GitHub's camo proxy 8192 character limit, preventing "limit exceeded" errors for large icons like the Amazon AWS icon. When PNG fallback is available (via rsvg-convert), oversized SVGs are automatically converted to compact PNGs.
Testing
Fixes #108
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.