Make URLs in the framework configurable#32957
Open
grliszas14 wants to merge 6 commits intomusescore:masterfrom
Open
Make URLs in the framework configurable#32957grliszas14 wants to merge 6 commits intomusescore:masterfrom
grliszas14 wants to merge 6 commits intomusescore:masterfrom
Conversation
igorkorsukov
reviewed
Apr 9, 2026
igorkorsukov
reviewed
Apr 9, 2026
igorkorsukov
reviewed
Apr 9, 2026
igorkorsukov
reviewed
Apr 9, 2026
igorkorsukov
approved these changes
Apr 9, 2026
📝 WalkthroughWalkthroughThis change refactors URL configuration management by centralizing website and privacy policy URLs in the 🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/appshell/internal/appshellconfiguration.cpp`:
- Around line 205-208: AppShellConfiguration::museScoreUrl() returns the raw
updateConfiguration()->appWebSiteUrl() which can end with a '/' and cause double
slashes when callers append paths; change museScoreUrl() to normalize the result
by trimming any trailing '/' (e.g., get the string from
updateConfiguration()->appWebSiteUrl(), if not empty and back() == '/', remove
the trailing slash) and return the normalized base URL so callers that append
"/..." won't produce '//' in URLs.
In `@src/framework/update/internal/updateconfiguration.cpp`:
- Around line 124-132: The appWebSiteUrl() and privacyPolicyUrl() accessors
currently return whatever m_config.value(...).toString() yields, allowing
empty/missing strings to propagate and cause silent broken links; update
UpdateConfiguration::appWebSiteUrl() and UpdateConfiguration::privacyPolicyUrl()
to check the retrieved value (and trim) and return a safe fallback when the
config key is missing or empty (e.g., return a constant DEFAULT_APP_WEBSITE_URL
/ DEFAULT_PRIVACY_POLICY_URL or "about:blank"); add those constants near the
class (or in an anonymous namespace) and use them in the two functions so
callers always get a non-empty, predictable URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad345f37-fc8e-43e4-bbd7-0fc7d795db13
📒 Files selected for processing (21)
src/app/configs/update.cfgsrc/appshell/internal/appshellconfiguration.cppsrc/appshell/internal/appshellconfiguration.hsrc/appshell/qml/MuseScore/AppShell/FirstLaunchSetup/tutorialspagemodel.cppsrc/appshell/qml/MuseScore/AppShell/aboutmodel.cppsrc/framework/global/iglobalconfiguration.hsrc/framework/global/internal/globalconfiguration.cppsrc/framework/global/internal/globalconfiguration.hsrc/framework/global/tests/mocks/globalconfigurationmock.hsrc/framework/stubs/update/updateconfigurationstub.cppsrc/framework/stubs/update/updateconfigurationstub.hsrc/framework/uicomponents/qml/Muse/UiComponents/StyledTextLabel.qmlsrc/framework/update/internal/appupdatescenario.cppsrc/framework/update/internal/updateconfiguration.cppsrc/framework/update/internal/updateconfiguration.hsrc/framework/update/internal/updateuiactions.hsrc/framework/update/iupdateconfiguration.hsrc/framework/update/tests/mocks/updateconfigurationmock.hsrc/musesounds/internal/musesamplercheckupdatescenario.cppsrc/musesounds/internal/musesamplercheckupdatescenario.hsrc/preferences/qml/MuseScore/Preferences/updatepreferencesmodel.cpp
💤 Files with no reviewable changes (4)
- src/framework/global/iglobalconfiguration.h
- src/framework/global/internal/globalconfiguration.h
- src/framework/global/tests/mocks/globalconfigurationmock.h
- src/framework/global/internal/globalconfiguration.cpp
Comment on lines
205
to
208
| std::string AppShellConfiguration::museScoreUrl() const | ||
| { | ||
| return globalConfiguration()->museScoreUrl(); | ||
| return updateConfiguration()->appWebSiteUrl(); | ||
| } |
There was a problem hiding this comment.
Normalize trailing slash in museScoreUrl() to prevent malformed joined links.
Line 207 returns the raw base URL, while other methods append path fragments starting with / (e.g., Line 197/202/212/217). With the current config value ending in /, generated links contain //....
🔧 Proposed fix
std::string AppShellConfiguration::museScoreUrl() const
{
- return updateConfiguration()->appWebSiteUrl();
+ std::string url = updateConfiguration()->appWebSiteUrl();
+ while (!url.empty() && url.back() == '/') {
+ url.pop_back();
+ }
+ return url;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/appshell/internal/appshellconfiguration.cpp` around lines 205 - 208,
AppShellConfiguration::museScoreUrl() returns the raw
updateConfiguration()->appWebSiteUrl() which can end with a '/' and cause double
slashes when callers append paths; change museScoreUrl() to normalize the result
by trimming any trailing '/' (e.g., get the string from
updateConfiguration()->appWebSiteUrl(), if not empty and back() == '/', remove
the trailing slash) and return the normalized base URL so callers that append
"/..." won't produce '//' in URLs.
Comment on lines
+124
to
132
| std::string UpdateConfiguration::appWebSiteUrl() const | ||
| { | ||
| return globalConfiguration()->museScoreUrl(); | ||
| return m_config.value("app_website_url").toString(); | ||
| } | ||
|
|
||
| std::string UpdateConfiguration::museScorePrivacyPolicyUrl() const | ||
| std::string UpdateConfiguration::privacyPolicyUrl() const | ||
| { | ||
| return globalConfiguration()->museScoreUrl() + PRIVACY_POLICY_URL_PATH; | ||
| return m_config.value("privacy_policy_url").toString(); | ||
| } |
There was a problem hiding this comment.
Add defensive fallbacks for missing URL config keys.
On Line 126 and Line 131, missing/empty config values propagate as empty URLs, causing silent link failures in UI flows. Please add safe defaults.
🛠️ Proposed fix
std::string UpdateConfiguration::appWebSiteUrl() const
{
- return m_config.value("app_website_url").toString();
+ const std::string url = m_config.value("app_website_url").toString();
+ return !url.empty() ? url : "https://www.musescore.org/";
}
std::string UpdateConfiguration::privacyPolicyUrl() const
{
- return m_config.value("privacy_policy_url").toString();
+ const std::string url = m_config.value("privacy_policy_url").toString();
+ return !url.empty() ? url : "https://www.musescore.org/about/desktop-privacy-policy";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/framework/update/internal/updateconfiguration.cpp` around lines 124 -
132, The appWebSiteUrl() and privacyPolicyUrl() accessors currently return
whatever m_config.value(...).toString() yields, allowing empty/missing strings
to propagate and cause silent broken links; update
UpdateConfiguration::appWebSiteUrl() and UpdateConfiguration::privacyPolicyUrl()
to check the retrieved value (and trim) and return a safe fallback when the
config key is missing or empty (e.g., return a constant DEFAULT_APP_WEBSITE_URL
/ DEFAULT_PRIVACY_POLICY_URL or "about:blank"); add those constants near the
class (or in an anonymous namespace) and use them in the two functions so
callers always get a non-empty, predictable URL.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/framework/update/internal/appupdatescenario.cpp`:
- Around line 137-141: The message assembly in appupdatescenario.cpp uses
configuration()->appWebSiteUrl() directly and relies on QUrl::host(), which
yields empty for empty or schemeless values; update the logic before building
the muse::qtrc string to validate and normalize the configured URL (e.g.,
construct QUrl from the std::string, check QUrl::isValid() and
QUrl::scheme().isEmpty()), and if the URL is empty/invalid or missing a scheme,
prepend a safe default scheme (https://) or fall back to a
non-clickable/display-only fallback (such as the hostless string or
application()->title()) so the href and link text passed into muse::qtrc are
never blank or broken; ensure you reference configuration()->appWebSiteUrl(),
the QUrl instance (url), and url.host() when implementing the guard and fallback
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 66127c19-d885-4a1d-9e57-412ce3be2d01
📒 Files selected for processing (3)
src/framework/update/internal/appupdatescenario.cppsrc/framework/update/internal/appupdatescenario.hsrc/framework/update/qml/Muse/Update/AppReleaseInfoDialog.qml
Comment on lines
+137
to
+141
| std::string webSiteUrl = configuration()->appWebSiteUrl(); | ||
| QUrl url(QString::fromStdString(webSiteUrl)); | ||
| const QString str = muse::qtrc("update", "You already have the latest version of %1. " | ||
| "Please visit <a href=\"%2\">%3</a> for news on what’s coming next.") | ||
| .arg(application()->title().toQString(), QString::fromStdString(webSiteUrl), url.host()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== update config files =="
fd -i 'update\.cfg$' || true
echo
echo "== URL-related keys in config files =="
fd -i 'update\.cfg$' -x rg -n 'app_website_url|privacy_policy_url' {}
echo
echo "== keys that appear to miss URL scheme =="
python - <<'PY'
import pathlib, re
key_re = re.compile(r'^\s*(app_website_url|privacy_policy_url)\s*[:=]\s*("?)([^"\n#]+)\2', re.I)
for p in pathlib.Path('.').rglob('*update*.cfg'):
try:
lines = p.read_text(encoding='utf-8', errors='ignore').splitlines()
except Exception:
continue
for i, line in enumerate(lines, 1):
m = key_re.search(line)
if not m:
continue
key, val = m.group(1), m.group(3).strip()
if '://' not in val:
print(f"{p}:{i}: {key} -> {val}")
PYRepository: musescore/MuseScore
Length of output: 335
🏁 Script executed:
# Check configuration loading and validation in updateconfiguration.cpp
fd -i 'updateconfiguration.cpp' -x cat -n {} | head -200Repository: musescore/MuseScore
Length of output: 5438
🏁 Script executed:
# Search for any validation or sanitization of URL configuration
rg -n "appWebSiteUrl|app_website_url" -A 5 -B 5Repository: musescore/MuseScore
Length of output: 9885
🏁 Script executed:
# Check if there are other places that might set this URL value
rg -n "setAppWebSiteUrl|appWebSiteUrl\s*=" -A 3Repository: musescore/MuseScore
Length of output: 45
Defend against empty or schemeless configured URLs in user-facing message.
Configuration loading has no validation for URL format. The stub returns empty string, which would render a broken message with blank href and link text. Additionally, schemeless URLs (e.g., musescore.org without https://) cause QUrl::host() to return empty.
Proposed fix
- std::string webSiteUrl = configuration()->appWebSiteUrl();
- QUrl url(QString::fromStdString(webSiteUrl));
+ const QString configuredUrl = QString::fromStdString(configuration()->appWebSiteUrl()).trimmed();
+ const QUrl url = QUrl::fromUserInput(configuredUrl);
+ const QString href = url.isValid() ? url.toString(QUrl::FullyEncoded) : configuredUrl;
+ const QString hostOrUrl = !url.host().isEmpty() ? url.host() : href;
const QString str = muse::qtrc("update", "You already have the latest version of %1. "
"Please visit <a href=\"%2\">%3</a> for news on what's coming next.")
- .arg(application()->title().toQString(), QString::fromStdString(webSiteUrl), url.host());
+ .arg(application()->title().toQString().toHtmlEscaped(),
+ href.toHtmlEscaped(),
+ hostOrUrl.toHtmlEscaped());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/framework/update/internal/appupdatescenario.cpp` around lines 137 - 141,
The message assembly in appupdatescenario.cpp uses
configuration()->appWebSiteUrl() directly and relies on QUrl::host(), which
yields empty for empty or schemeless values; update the logic before building
the muse::qtrc string to validate and normalize the configured URL (e.g.,
construct QUrl from the std::string, check QUrl::isValid() and
QUrl::scheme().isEmpty()), and if the URL is empty/invalid or missing a scheme,
prepend a safe default scheme (https://) or fall back to a
non-clickable/display-only fallback (such as the hostless string or
application()->title()) so the href and link text passed into muse::qtrc are
never blank or broken; ensure you reference configuration()->appWebSiteUrl(),
the QUrl instance (url), and url.host() when implementing the guard and fallback
behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: audacity/audacity#10353