Skip to content
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

add validation of update mirror urls #17310

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
16 changes: 16 additions & 0 deletions source/gui/settingsDialogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,7 @@ def onChangeMirrorURL(self, evt: wx.CommandEvent | wx.KeyEvent):
configPath=("update", "serverURL"),
helpId="SetURLDialog",
urlTransformer=lambda url: f"{url}?versionType=stable",
responseValidator=_isResponseUpdateMirrorValid,
)
ret = changeMirror.ShowModal()
if ret == wx.ID_OK:
Expand Down Expand Up @@ -5595,3 +5596,18 @@ def _isResponseAddonStoreCacheHash(response: requests.Response) -> bool:
# While the NV Access Add-on Store cache hash is a git commit hash as a string, other implementations may use a different format.
# Therefore, we only check if the data is a non-empty string.
return isinstance(data, str) and bool(data)


def _isResponseUpdateMirrorValid(response: requests.Response) -> bool:
SaschaCowley marked this conversation as resolved.
Show resolved Hide resolved
christopherpross marked this conversation as resolved.
Show resolved Hide resolved
if not response.ok:
return False

responseContent = response.text
christopherpross marked this conversation as resolved.
Show resolved Hide resolved

try:
parsedResponse = updateCheck.parseUpdateCheckResponse(responseContent)
except Exception as e:
log.error(f"Error parsing update mirror response: {e}")
return False

return parsedResponse is not None
34 changes: 25 additions & 9 deletions source/updateCheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,27 @@ def getQualifiedDriverClassNameForStats(cls):
return "%s (core)" % name


def parseUpdateCheckResponse(data: str) -> dict[str, str] | None:
christopherpross marked this conversation as resolved.
Show resolved Hide resolved
"""
Parses the update response and returns a dictionary with metadata.

:param data: The raw server response as a UTF-8 decoded string.
:return: A dictionary containing the update metadata, or None if the format is invalid.
"""
if not data.strip():
return None

metadata = {}
for line in data.splitlines():
try:
key, val = line.split(": ", 1)
metadata[key] = val
except ValueError:
return None # Invalid format

return metadata


UPDATE_FETCH_TIMEOUT_S = 30 # seconds


Expand Down Expand Up @@ -187,15 +208,10 @@ def checkForUpdate(auto: bool = False) -> Optional[Dict]:
raise
if res.code != 200:
raise RuntimeError("Checking for update failed with code %d" % res.code)
info = {}
for line in res:
# #9819: update description resource returns bytes, so make it Unicode.
line = line.decode("utf-8").rstrip()
try:
key, val = line.split(": ", 1)
except ValueError:
raise RuntimeError("Error in update check output")
info[key] = val

data = res.text
info = parseUpdateCheckResponse(data)
christopherpross marked this conversation as resolved.
Show resolved Hide resolved

if not info:
return None
return info
Expand Down
2 changes: 1 addition & 1 deletion user_docs/en/changes.md
Copy link
Member

Choose a reason for hiding this comment

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

Please document API breaking changes (by my count, checkForUpdate, UpdateResultDialog.__init__, UpdateDownloader.__init__)

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ To use this feature, "allow NVDA to control the volume of other applications" mu
* NVDA can now report when a link destination points to the current page. (#141, @LeonarddeR, @nvdaes)
* Added an action in the Add-on Store to cancel the install of add-ons. (#15578, @hwf1324)
* Added an action in the Add-on Store to retry the installation if the download/installation of an add-on fails. (#17090, @hwf1324)
* It is now possible to specify mirror URLs to use for NVDA updates and the Add-on Store. (#14974, #17151)
* It is now possible to specify mirror URLs to use for NVDA updates and the Add-on Store. (#14974, #17151, #17310)
christopherpross marked this conversation as resolved.
Show resolved Hide resolved
* The add-ons lists in the Add-on Store can be sorted by columns, including publication date, in ascending and descending order. (#15277, #16681, @nvdaes)
* When decreasing or increasing the font size in LibreOffice Writer using the corresponding keyboard shortcuts, NVDA announces the new font size. (#6915, @michaelweghorn)
* When applying the "Body Text" or a heading paragraph style using the corresponding keyboard shortcut in LibreOffice Writer 25.2 or newer, NVDA announces the new paragraph style. (#6915, @michaelweghorn)
Expand Down