Skip to content

Commit b241df7

Browse files
authored
Store addons state as JSON (#19564)
Closes #19559 ### Summary of the issue: Addons state is saved in a pickle file. ### Description of user facing changes: None ### Description of developer facing changes: The add-ons state file is now in JSON. The `fromPickledDict` method on `AddonsState` has been renamed to `fromDict`, but a shim has been introduced. ### Description of development approach: Switch to using lists instead of sets when populating AddonsState from dict and serializing to dict, as json.dump doesn't support serializing sets. Added methods to read in a pickled add-ons state file and emit a good JSON representation of that add-ons state. My approach here was to discard invalid values rather than raising an error, so we have the greatest chance of success. This does mean that the conversion may be lossy. When updating NVDA, convert from pickle to JSON for the system config's add-ons state file. This is done because this is essentially the only context in which we should write to the system config directory and which we know will be executed. User copies perform the migration on first run by migrating if and only if no addonsState.json exists, but addonsState.pickle does. User config migration is done by the installed copy as (a) this is the only time we can guarantee that the user's NVDA config wil be present; and (b) to minimise the amount of pickles we load while elevated. In both cases, a backup of the old state file is kept (as `addonsState.pickle.bak`). When running on secure desktops, the pickled state will not be read, even if it is the only addonsState file in the system config. This should never happen, as the installer is responsible for migrating the system config's add-ons state file. In other cases, the fallback of reading the pickled state will attempt to save the JSON state and back up the pickled state, but this is not required in order for the load to succede. This is so when running in secure mode or with access to the user config restricted, add-ons continue to function properly. This does remove the security benefit of migrating away from pickle, but there's no other option other than ripping pickle support out entirely and breaking everyone's add-ons state entirely and having them manually migrate. ### Testing strategy: Unit and system tests. Manual testing as follows: 1. Installed 2025.3.3 2. Installed add-ons (combination of 2026.1 compatible and incompatible, enabled and disabled) and restarted NVDA 3. Copied user config to system config 4. Upgraded using self-signed launcher 5. Ensured that the installer migrated the system addons state, and the installed copy migrated the user addons state on first run Also "upgraded" my 2026.2 alpha copy with this launcher to ensure that the migration works as expected if updating within the 2026.x series. ### Known issues with pull request: None known (see notes above for some caviats about the approach though)
1 parent aba8e88 commit b241df7

7 files changed

Lines changed: 540 additions & 110 deletions

File tree

source/NVDAState.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,15 @@ def nvdaConfigFile(self) -> str:
7373

7474
@property
7575
def addonStateFile(self) -> str:
76-
from addonHandler import stateFilename
76+
from addonHandler import STATE_FILENAME
7777

78-
return os.path.join(self.configDir, stateFilename)
78+
return os.path.join(self.configDir, STATE_FILENAME)
79+
80+
@property
81+
def _oldAddonStateFile(self) -> str:
82+
from addonHandler import _OLD_STATE_FILENAME
83+
84+
return os.path.join(self.configDir, _OLD_STATE_FILENAME)
7985

8086
@property
8187
def profileTriggersFile(self) -> str:

source/addonHandler/__init__.py

Lines changed: 103 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
from __future__ import annotations # Avoids quoting of forward references
88

99
from abc import abstractmethod, ABC
10-
from collections.abc import Callable
10+
from collections.abc import Callable, Mapping
11+
import json
1112
import sys
1213
import os.path
1314
import gettext
@@ -16,9 +17,10 @@
1617
import collections
1718
import shutil
1819
from io import StringIO
19-
import pickle
2020
from typing import (
2121
IO,
22+
Any,
23+
Final,
2224
Literal,
2325
TYPE_CHECKING,
2426
)
@@ -29,6 +31,7 @@
2931
from config.registry import ADDON_BUNDLE_EXTENSION
3032
import languageHandler
3133
from logHandler import log
34+
from utils.security import isRunningOnSecureDesktop
3235
import winBindings.kernel32
3336
import addonAPIVersion
3437
import importlib
@@ -69,13 +72,32 @@
6972
"NVDA_ADDON_PROG_ID",
7073
"config.registry",
7174
),
75+
MovedSymbol(
76+
"stateFilename",
77+
"addonHandler",
78+
"STATE_FILENAME",
79+
),
7280
)
7381

74-
MANIFEST_FILENAME = "manifest.ini"
75-
stateFilename = "addonsState.pickle"
76-
BUNDLE_MIMETYPE = "application/x-nvda-addon"
77-
ADDON_PENDINGINSTALL_SUFFIX = ".pendingInstall"
78-
DELETEDIR_SUFFIX = ".delete"
82+
MANIFEST_FILENAME: Final[str] = "manifest.ini"
83+
STATE_FILENAME: Final[str] = "addonsState.json"
84+
_OLD_STATE_FILENAME: Final[str] = "addonsState.pickle"
85+
# Decoding JSON can potentially consume a large amount of memory.
86+
# To avoid issues caused by this when reading add-on state,
87+
# limit the maximum size of the file.
88+
# The 2026.1 state JSON with no add-ons is 244 bytes.
89+
# As of 2026-02-24, there are 364 add-ons in the Add-on Store,
90+
# and the longest ID is 40 UTF-8 bytes (mean ~= median ~= 13 bytes).
91+
# Even if we take the empty state to be 300 bytes,
92+
# and assume that all add-on IDs are 100 characters long,
93+
# And those 100 characters are all 4 bytes wide,
94+
# and factor in the 4 extra bytes required for the enclosing quotes and ", " separator between IDs,
95+
# 1MiB is still enough space for a state file containing
96+
# (1024 * 1024) / (4 + 4 * 100) ~= 2594 add-ons.
97+
_MAX_STATE_FILESIZE_BYTES = 1024 * 1024 # 1MiB
98+
BUNDLE_MIMETYPE: Final[str] = "application/x-nvda-addon"
99+
ADDON_PENDINGINSTALL_SUFFIX: Final[str] = ".pendingInstall"
100+
DELETEDIR_SUFFIX: Final[str] = ".delete"
79101

80102

81103
# Allows add-ons to process additional command line arguments when NVDA starts.
@@ -100,6 +122,10 @@ class AddonsState(collections.UserDict[AddonStateCategory, CaseInsensitiveSet[st
100122
Therefore add-on IDs should be treated as case insensitive.
101123
"""
102124

125+
def __init__(self, *args, **kwargs) -> None:
126+
super().__init__(*args, **kwargs)
127+
self.setDefaultStateValues()
128+
103129
@staticmethod
104130
def _generateDefaultStateContent() -> AddonStateDictT:
105131
return {category: CaseInsensitiveSet() for category in AddonStateCategory}
@@ -120,30 +146,41 @@ def setDefaultStateValues(self) -> None:
120146
# where the BACK_COMPAT_TO API version was 2023.1.0.
121147
self.manualOverridesAPIVersion = MajorMinorPatch(2023, 1, 0)
122148

149+
def fromDict(
150+
self,
151+
stateDict: dict[str, Any],
152+
) -> None:
153+
if "backCompatToAPIVersion" in stateDict:
154+
try:
155+
self.manualOverridesAPIVersion = MajorMinorPatch(
156+
*(int(num) for num in stateDict["backCompatToAPIVersion"]),
157+
)
158+
except Exception:
159+
log.error("Unable to deserialise backward compatibility version.", exc_info=True)
160+
for category in AddonStateCategory:
161+
# Make the list of strings unique and case insensitive.
162+
self[AddonStateCategory(category)] = CaseInsensitiveSet(stateDict.get(category, []))
163+
123164
def fromPickledDict(
124165
self,
125166
pickledState: dict[str, set[str] | addonAPIVersion.AddonApiVersionT | MajorMinorPatch],
126167
) -> None:
127-
# Load from pickledState
128-
if "backCompatToAPIVersion" in pickledState:
129-
self.manualOverridesAPIVersion = MajorMinorPatch(*pickledState["backCompatToAPIVersion"])
130-
for category in AddonStateCategory:
131-
# Make pickles case insensitive
132-
self[AddonStateCategory(category)] = CaseInsensitiveSet(pickledState.get(category, set()))
133-
134-
def toDict(self) -> dict[str, set[str] | addonAPIVersion.AddonApiVersionT]:
135-
# We cannot pickle instance of `AddonsState` directly
136-
# since older versions of NVDA aren't aware about this class and they're expecting
137-
# the state to be using inbuilt data types only.
138-
picklableState: dict[str, set[str] | addonAPIVersion.AddonApiVersionT] = dict()
139-
for category in self.data:
140-
picklableState[category.value] = set(self.data[category])
141-
picklableState["backCompatToAPIVersion"] = tuple(self.manualOverridesAPIVersion)
142-
return picklableState
168+
log.warning(
169+
"addonHandler.AddonsState.fromPickledDict is deprecated. Use addonHandler.AddonsState.fromDict instead.",
170+
)
171+
return self.fromDict(pickledState)
172+
173+
def toDict(self) -> dict[str, list[str] | addonAPIVersion.AddonApiVersionT]:
174+
"""Convert state to a dict that can be dumped to JSON."""
175+
serializeableState: dict[str, list[str] | addonAPIVersion.AddonApiVersionT] = dict()
176+
for category, addonIds in self.data.items():
177+
serializeableState[category.value] = list(addonIds)
178+
serializeableState["backCompatToAPIVersion"] = tuple(self.manualOverridesAPIVersion)
179+
return serializeableState
143180

144181
def load(self) -> None:
145182
"""Populates state with the default content and then loads values from the config."""
146-
self._load(self.statePath)
183+
self._loadWithFallback()
147184
if self.manualOverridesAPIVersion != addonAPIVersion.BACK_COMPAT_TO:
148185
log.debug(
149186
"BACK_COMPAT_TO API version for manual compatibility overrides has changed. "
@@ -169,21 +206,51 @@ def _load(self, statePath: os.PathLike) -> None:
169206
170207
:param statePath: Path from which to load the addons state file.
171208
"""
172-
self.setDefaultStateValues()
173209
try:
174-
# #9038: Python 3 requires binary format when working with pickles.
175-
with open(statePath, "rb") as f:
176-
pickledState: dict[str, set[str] | addonAPIVersion.AddonApiVersionT] = pickle.load(f)
210+
with open(statePath, "rt", encoding="utf-8") as file:
211+
if (size := os.stat(file.fileno()).st_size) > _MAX_STATE_FILESIZE_BYTES:
212+
log.error(f"Add-ons state file too large. {size=}B; {_MAX_STATE_FILESIZE_BYTES=}B")
213+
return
214+
stateDict = json.load(file)
177215
except FileNotFoundError:
178216
pass # Clean config - no point logging in this case
179-
except IOError:
180-
log.debug("Error when reading state file", exc_info=True)
181-
except pickle.UnpicklingError:
182-
log.debugWarning("Failed to unpickle state", exc_info=True)
217+
except OSError:
218+
log.error("Error when reading state file", exc_info=True)
219+
except json.JSONDecodeError:
220+
log.error("Failed to deserialize add-ons state", exc_info=True)
183221
except Exception:
184222
log.exception()
185223
else:
186-
self.fromPickledDict(pickledState)
224+
if not isinstance(stateDict, Mapping):
225+
log.error(
226+
f"Expected parsed state dictionary to be a mapping type; got {type(stateDict)} instead.",
227+
)
228+
return
229+
self.fromDict(stateDict)
230+
231+
def _loadWithFallback(self) -> None:
232+
if os.path.isfile(self.statePath):
233+
self._load(self.statePath)
234+
else:
235+
if not isRunningOnSecureDesktop() and os.path.isfile(WritePaths._oldAddonStateFile):
236+
# Only import if absolutely necessary.
237+
from ._pickleToJsonMigration import _getAddonsStateDictFromPickle
238+
239+
log.warning("Loading add-ons state from pickle.")
240+
try:
241+
self.fromDict(_getAddonsStateDictFromPickle(WritePaths._oldAddonStateFile))
242+
except Exception:
243+
log.error("Failed to load pickled add-ons state.", exc_info=True)
244+
else:
245+
if NVDAState.shouldWriteToDisk():
246+
self.save()
247+
finally:
248+
if NVDAState.shouldWriteToDisk():
249+
log.debug("Backing up pickled add-ons state.")
250+
try:
251+
os.replace(WritePaths._oldAddonStateFile, WritePaths._oldAddonStateFile + ".bak")
252+
except Exception:
253+
log.debug("Unable to backup old add-ons state pickle file.", exc_info=True)
187254

188255
def removeStateFile(self) -> None:
189256
if not NVDAState.shouldWriteToDisk():
@@ -223,10 +290,9 @@ def _save(self, statePath: os.PathLike) -> bool:
223290
raise RuntimeError("Should not write to disk.")
224291
if any(self.values()):
225292
try:
226-
# #9038: Python 3 requires binary format when working with pickles.
227-
with open(statePath, "wb") as f:
228-
pickle.dump(self.toDict(), f, protocol=0)
229-
except (IOError, pickle.PicklingError):
293+
with open(statePath, "wt", encoding="utf-8") as file:
294+
json.dump(self.toDict(), file)
295+
except (IOError, TypeError):
230296
log.debugWarning("Error saving state", exc_info=True)
231297
return True
232298
return False
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
# A part of NonVisual Desktop Access (NVDA)
2+
# Copyright (C) 2026 NV Access Limited
3+
# This file may be used under the terms of the GNU General Public License, version 2 or later, as modified by the NVDA license.
4+
# For full terms and any additional permissions, see the NVDA license file: https://github.com/nvaccess/nvda/blob/master/copying.txt
5+
6+
"""Utilities for migrating add-on state from the legacy pickle format to JSON.
7+
8+
Prior to NVDA 2026.1, add-on state (which add-ons are disabled, pending install/removal, etc.) was persisted as a pickle file (``addonsState.pickle``).
9+
This module provides helpers that read such a pickle file, validate its contents, and return a JSON-serialisable dictionary so the state can be persisted as JSON instead (``addonsState.json``).
10+
11+
.. warning:
12+
This module is scheduled for removal in NVDA 2027.1 once all users have had sufficient opportunity to migrate.
13+
"""
14+
15+
from __future__ import annotations
16+
17+
from typing import TYPE_CHECKING, Any
18+
19+
import NVDAState
20+
from addonAPIVersion import BACK_COMPAT_TO
21+
from addonStore.models.status import AddonStateCategory
22+
from logHandler import log
23+
24+
if TYPE_CHECKING:
25+
import os
26+
27+
28+
# This module and its callers should be removed in 2027.1.
29+
# To ensure this isn't missed, log an error about using the module on 2027.1 if deprecated APIs are allowed, and fail if they're not.
30+
if BACK_COMPAT_TO >= (2027, 1, 0):
31+
DEPRECATION_STRING = "addonHandler._pickleToJsonMigration is deprecated. All internal use of pickle should be removed in NVDA 2027.1."
32+
if NVDAState._allowDeprecatedAPI():
33+
log.error(DEPRECATION_STRING, stack_info=True)
34+
else:
35+
raise RuntimeError(DEPRECATION_STRING)
36+
37+
38+
def _pickledStateDictToJsonStateDict(pickledState: Any) -> dict[str, list[str] | tuple[int, int, int]]:
39+
"""Convert an unpickled add-on state dictionary to a JSON-compatible form.
40+
41+
Keys and values are validated, and unrecognised or malformed entries are logged and silently discarded.
42+
This allows partially corrupt pickles to be migrated.
43+
44+
:param pickledState: The raw object obtained from ``pickle.load``.
45+
Expected to be a ``dict`` mapping :class:`AddonStateCategory` string keys to ``set[str]`` values.
46+
May also include a ``"backCompatToAPIVersion"`` key with a 2- or 3-element integer tuple.
47+
:return: A new dictionary suitable for JSON serialisation.
48+
May be empty depending on the validity of ``pickledDict``.
49+
"""
50+
if not isinstance(pickledState, dict):
51+
log.debug(f"Invalid pickled state: {pickledState!r}")
52+
return {}
53+
jsonState: dict[str, list[str] | tuple[int, int, int]] = {}
54+
for key, value in pickledState.items():
55+
if key == "backCompatToAPIVersion":
56+
# backCompatToAPIVersion is an API version tuple, e.g. (2024, 1, 0).
57+
# The patch version is optional and defaults to 0.
58+
if (
59+
isinstance(value, tuple)
60+
and 2 <= len(value) <= 3
61+
and all(isinstance(part, int) for part in value)
62+
):
63+
jsonState[key] = value
64+
else:
65+
log.debug(f"Invalid backCompatToAPIVersion: {value!r}. Discarding.")
66+
elif key in AddonStateCategory:
67+
# Add-on category values were stored as sets of add-on name strings.
68+
# Since JSON doesn't have a set type, convert each to a list, filtering out any non-string values.
69+
if not isinstance(value, set):
70+
log.debug(f"Invalid category {key}: {value!r}. Discarding.")
71+
continue
72+
addons = jsonState[key] = []
73+
for addon in value:
74+
if isinstance(addon, str):
75+
addons.append(addon)
76+
else:
77+
log.debug(f"Invalid add-on in category {key}: {addon!r}. Discarding")
78+
else:
79+
log.debug(f"Unrecognised key-value pair: {key!r}: {value!r}. Discarding")
80+
return jsonState
81+
82+
83+
def _getAddonsStateDictFromPickle(picklePath: os.PathLike) -> dict[str, list[str] | tuple[int, int, int]]:
84+
"""Load a legacy pickled add-on state file and return a JSON-compatible dict.
85+
86+
This is the main entry point used by :mod:`addonHandler` and :mod:`installer` when an ``addonsState.pickle`` file exists and hasn't yet been migrated to ``addonsState.json``.
87+
88+
:param picklePath: Path to the ``addonsState.pickle`` file.
89+
:return: A validated, JSON-serialisable dictionary of add-on state.
90+
:raises Exception: Any exception raised by :func:`open` or :func:`pickle.load`.
91+
E.g. ``FileNotFoundError``, ``pickle.UnpicklingError``.
92+
"""
93+
# pickle is imported locally to avoid loading it at module level, since this migration path will eventually be removed entirely.
94+
import pickle
95+
96+
with open(picklePath, "rb") as pickleFile:
97+
return _pickledStateDictToJsonStateDict(pickle.load(pickleFile))

0 commit comments

Comments
 (0)