Add Direct URL and Delta update support to Velopack flow#5548
Add Direct URL and Delta update support to Velopack flow#5548Viscerous wants to merge 1 commit intosecondlife:release/26.1.1from
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Pull request overview
Adds support to the viewer update flow for (1) overriding Velopack’s feed URL via VELOPACK_UPDATE_URL, and (2) pre-downloading multiple update assets to enable delta updates with a full-release fallback path.
Changes:
- Add
VELOPACK_UPDATE_URLhandling to bypass VVM and run Velopack update checks against a direct feed URL. - Update Velopack asset pre-download logic from a single file to a filename→path map, enabling delta asset handling.
- Add a fallback attempt to download the full release if the delta download path fails.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
indra/newview/llvvmquery.cpp |
Adds environment-variable override path to set Velopack update URL and launch an update check. |
indra/newview/llvelopack.cpp |
Pre-downloads multiple assets (deltas/full) and adds a fallback from delta to full release download. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::string get_direct_velopack_update_url() | ||
| { | ||
| auto direct_update_url = LLStringUtil::getoptenv("VELOPACK_UPDATE_URL"); | ||
| if (direct_update_url && direct_update_url->find("http") != std::string::npos) |
There was a problem hiding this comment.
The VELOPACK_UPDATE_URL validation is overly permissive: find("http") != npos will accept strings that aren't actually an http/https URL (e.g. myhttp), and the warning message implies stricter behavior. Consider validating the scheme explicitly (e.g. parse with LLURI or require prefix http:// / https://).
| if (direct_update_url && direct_update_url->find("http") != std::string::npos) | |
| if (direct_update_url && | |
| (direct_update_url->compare(0, 7, "http://") == 0 || | |
| direct_update_url->compare(0, 8, "https://") == 0)) |
| std::string direct_update_url = get_direct_velopack_update_url(); | ||
| if (!direct_update_url.empty()) | ||
| { | ||
| LL_INFOS("VVM") << "Using direct Velopack feed URL from VELOPACK_UPDATE_URL: " | ||
| << direct_update_url << LL_ENDL; | ||
| velopack_set_update_url(direct_update_url); | ||
| LLCoros::instance().launch("VelopackUpdateCheck", | ||
| []() | ||
| { | ||
| velopack_check_for_updates(std::string(), std::string()); | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
In the direct-URL path, the update check runs unconditionally. This bypasses the existing UpdaterServiceSetting gating used for optional updates in the normal VVM flow, so users who have disabled optional update checks may still get prompted when VELOPACK_UPDATE_URL is set. Consider applying the same preference logic before launching velopack_check_for_updates() when required_version is empty.
| LL_INFOS("VVM") << "Using direct Velopack feed URL from VELOPACK_UPDATE_URL: " | ||
| << direct_update_url << LL_ENDL; |
There was a problem hiding this comment.
This log line prints the full VELOPACK_UPDATE_URL value. If the URL contains credentials/tokens (e.g. query params or basic auth), it will end up in viewer logs. Consider redacting sensitive components (at least userinfo and query) before logging, or logging only the host/path.
| LL_INFOS("VVM") << "Using direct Velopack feed URL from VELOPACK_UPDATE_URL: " | |
| << direct_update_url << LL_ENDL; | |
| LL_INFOS("VVM") << "Using direct Velopack feed URL from VELOPACK_UPDATE_URL" << LL_ENDL; |
| std::string local_path = gDirUtilp->getExpandedFilename(LL_PATH_TEMP, asset_filename); | ||
| LL_INFOS("Velopack") << "Pre-downloading asset: " << asset_url |
There was a problem hiding this comment.
asset_filename is sourced from the update feed and is used directly to build local_path under LL_PATH_TEMP. Since LLDir::getExpandedFilename() concatenates the filename verbatim, a filename containing path traversal (e.g. ../) could escape the temp directory. Consider sanitizing to a basename (and rejecting path separators / ..) before constructing local_path.
|
FYI, this is on hold until we get 26.1.1 out the door. |
Adds support for vpk direct URLs (via environment variable:
VELOPACK_UPDATE_URL) and delta updates with retry behaviour.