- tbb-commits - lists.torproject.org

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 2 commits: fixup! Add CI for Tor Browser
by Pier Angelo Vendrame (＠pierov) 22 Jan '25

22 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 7cffb14a by Henry Wilkes at 2025-01-22T13:59:46+00:00 fixup! Add CI for Tor Browser MB 324: Move update-translations from tor-browser to base-browser. Should be dropped at the next rebase. - - - - - dbca320b by Henry Wilkes at 2025-01-22T13:59:46+00:00 fixup! Add CI for Base Browser MB 324: Move update-translations from tor-browser to base-browser. - - - - - 0 changed files: Changes: View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/2dfd29… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/2dfd29… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/mullvad-browser][mullvad-browser-128.6.0esr-14.5-1] fixup! MB 1: Mullvad Browser branding
by morgan (＠morgan) 22 Jan '25

22 Jan '25

morgan pushed to branch mullvad-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Mullvad Browser Commits: 9db51048 by Pier Angelo Vendrame at 2025-01-22T13:47:45+00:00 fixup! MB 1: Mullvad Browser branding TB 43323: Add an asset for sanity tests. - - - - - 2 changed files: - + toolkit/themes/shared/icons/mullvadbrowser.png - toolkit/themes/shared/minimal-toolkit.jar.inc.mn Changes: ===================================== toolkit/themes/shared/icons/mullvadbrowser.png ===================================== Binary files /dev/null and b/toolkit/themes/shared/icons/mullvadbrowser.png differ ===================================== toolkit/themes/shared/minimal-toolkit.jar.inc.mn ===================================== @@ -45,3 +45,6 @@ toolkit.jar: # Text recognition widget skin/classic/global/media/textrecognition.css (../../shared/media/textrecognition.css) + +# For testing purposes + skin/classic/global/icons/mullvadbrowser.png (../../shared/icons/mullvadbrowser.png) View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/commit/9db… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/commit/9db… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] fixup! TB 2176: Rebrand Firefox to TorBrowser
by morgan (＠morgan) 22 Jan '25

22 Jan '25

morgan pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 2dfd2952 by Pier Angelo Vendrame at 2025-01-22T13:28:45+00:00 fixup! TB 2176: Rebrand Firefox to TorBrowser TB 43323: Add an asset for sanity tests. - - - - - 2 changed files: - + toolkit/themes/shared/icons/torbrowser.png - toolkit/themes/shared/minimal-toolkit.jar.inc.mn Changes: ===================================== toolkit/themes/shared/icons/torbrowser.png ===================================== Binary files /dev/null and b/toolkit/themes/shared/icons/torbrowser.png differ ===================================== toolkit/themes/shared/minimal-toolkit.jar.inc.mn ===================================== @@ -45,3 +45,6 @@ toolkit.jar: # Text recognition widget skin/classic/global/media/textrecognition.css (../../shared/media/textrecognition.css) + +# For testing purposes + skin/classic/global/icons/torbrowser.png (../../shared/icons/torbrowser.png) View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/2dfd295… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/2dfd295… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/mullvad-browser][mullvad-browser-128.6.0esr-14.5-1] 3 commits: fixup! Add CI for Base Browser
by Pier Angelo Vendrame (＠pierov) 22 Jan '25

22 Jan '25

Pier Angelo Vendrame pushed to branch mullvad-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Mullvad Browser Commits: d7762a48 by Henry Wilkes at 2025-01-22T11:32:35+00:00 fixup! Add CI for Base Browser MB 324: Move update-translations CI to base-browser. - - - - - e30c4e62 by Henry Wilkes at 2025-01-22T11:32:36+00:00 BB 42305: Add script to combine translation files across versions. - - - - - 7edacb63 by Henry Wilkes at 2025-01-22T11:32:36+00:00 Add CI for Mullvad Browser - - - - - 11 changed files: - .gitlab-ci.yml - + .gitlab/ci/jobs/update-translations.yml - + tools/base-browser/l10n/combine-translation-versions.py - + tools/base-browser/l10n/combine/__init__.py - + tools/base-browser/l10n/combine/combine.py - + tools/base-browser/l10n/combine/tests/README - + tools/base-browser/l10n/combine/tests/__init__.py - + tools/base-browser/l10n/combine/tests/test_android.py - + tools/base-browser/l10n/combine/tests/test_dtd.py - + tools/base-browser/l10n/combine/tests/test_fluent.py - + tools/base-browser/l10n/combine/tests/test_properties.py Changes: ===================================== .gitlab-ci.yml ===================================== @@ -1,5 +1,6 @@ stages: - lint + - update-translations variables: IMAGE_PATH: containers.torproject.org/tpo/applications/tor-browser/base:latest ===================================== .gitlab/ci/jobs/update-translations.yml ===================================== @@ -0,0 +1,95 @@ +.update-translation-base: + stage: update-translations + rules: + - if: ($TRANSLATION_FILES != "" && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push") + changes: + - "**/*.ftl" + - "**/*.properties" + - "**/*.dtd" + - "**/*strings.xml" + - "**/update-translations.yml" + - "**/l10n/combine/combine.py" + - "**/l10n/combine-translation-versions.py" + - if: ($TRANSLATION_FILES != "" && $FORCE_UPDATE_TRANSLATIONS == "true") + variables: + COMBINED_FILES_JSON: "combined-translation-files.json" + TRANSLATION_FILES: '[ + { + "name": "brand.ftl", + "where": ["browser/branding/mb-release"], + "branch": "mullvad-browser", + "directory": "browser/branding/mb-release" + }, + { + "name": "brand.properties", + "where": ["browser/branding/mb-release"], + "branch": "mullvad-browser", + "directory": "browser/branding/mb-release" + }, + { + "name": "brand.ftl", + "where": ["browser/branding/mb-alpha"], + "branch": "mullvad-browser", + "directory": "browser/branding/mb-alpha" + }, + { + "name": "brand.properties", + "where": ["browser/branding/mb-alpha"], + "branch": "mullvad-browser", + "directory": "browser/branding/mb-alpha" + }, + { + "name": "brand.ftl", + "where": ["browser/branding/mb-nightly"], + "branch": "mullvad-browser", + "directory": "browser/branding/mb-nightly" + }, + { + "name": "brand.properties", + "where": ["browser/branding/mb-nightly"], + "branch": "mullvad-browser", + "directory": "browser/branding/mb-nightly" + }, + { + "name": "mullvad-browser.ftl", + "branch": "mullvad-browser", + "directory": "toolkit/toolkit/global" + }, + ]' + + +combine-en-US-translations: + extends: .update-translation-base + needs: [] + image: python + variables: + PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip" + cache: + paths: + - .cache/pip + # Artifact is for translation project job + artifacts: + paths: + - "$COMBINED_FILES_JSON" + expire_in: "60 min" + reports: + dotenv: job_id.env + # Don't load artifacts for this job. + dependencies: [] + script: + # Save this CI_JOB_ID to the dotenv file to be used in the variables for the + # push-en-US-translations job. + - echo 'COMBINE_TRANSLATIONS_JOB_ID='"$CI_JOB_ID" >job_id.env + - pip install compare_locales + - python ./tools/base-browser/l10n/combine-translation-versions.py "$CI_COMMIT_BRANCH" "$TRANSLATION_FILES" "$COMBINED_FILES_JSON" + +push-en-US-translations: + extends: .update-translation-base + needs: + - job: combine-en-US-translations + variables: + COMBINED_FILES_JSON_URL: "${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/jobs/${COMBINE_TRANSLATIONS_JOB_ID}/artifacts/${COMBINED_FILES_JSON}" + trigger: + strategy: depend + project: tor-browser-translation-bot/translation + branch: tor-browser-ci ===================================== tools/base-browser/l10n/combine-translation-versions.py ===================================== @@ -0,0 +1,368 @@ +import argparse +import json +import logging +import os +import re +import subprocess + +from combine import combine_files + +arg_parser = argparse.ArgumentParser( + description="Combine a translation file across two different versions" +) + +arg_parser.add_argument( + "current_branch", metavar="<current-branch>", help="branch for the newest version" +) +arg_parser.add_argument( + "files", metavar="<files>", help="JSON specifying the translation files" +) +arg_parser.add_argument("outname", metavar="<json>", help="name of the json output") + +args = arg_parser.parse_args() + +logging.basicConfig() +logger = logging.getLogger("combine-translation-versions") +logger.setLevel(logging.INFO) + + +def in_pink(msg: str) -> str: + """Present a message as pink in the terminal output. + + :param msg: The message to wrap in pink. + :returns: The message to print to terminal. + """ + # Pink and bold. + return f"\x1b[1;38;5;212m{msg}\x1b[0m" + + +def git_run(git_args: list[str]) -> None: + """Run a git command. + + :param git_args: The arguments that should follow "git". + """ + # Add some text to give context to git's stderr appearing in log. + logger.info("Running: " + in_pink("git " + " ".join(git_args))) + subprocess.run(["git", *git_args], check=True) + + +def git_text(git_args: list[str]) -> str: + """Get the text output for a git command. + + :param git_args: The arguments that should follow "git". + :returns: The stdout of the command. + """ + logger.info("Running: " + in_pink("git " + " ".join(git_args))) + return subprocess.run( + ["git", *git_args], text=True, check=True, stdout=subprocess.PIPE + ).stdout + + +def git_lines(git_args: list[str]) -> list[str]: + """Get the lines from a git command. + + :param git_args: The arguments that should follow "git". + :returns: The non-empty lines from stdout of the command. + """ + return [line for line in git_text(git_args).split("\n") if line] + + +class TranslationFile: + """Represents a translation file.""" + + def __init__(self, path: str, content: str) -> None: + self.path = path + self.content = content + + +class BrowserBranch: + """Represents a browser git branch.""" + + def __init__(self, branch_name: str, is_head: bool = False) -> None: + """Create a new instance. + + :param branch_name: The branch's git name. + :param is_head: Whether the branch matches "HEAD". + """ + version_match = re.match( + r"(?P<prefix>[a-z]+\-browser)\-" + r"(?P<firefox>[0-9]+(?:\.[0-9]+){1,2})esr\-" + r"(?P<browser>[0-9]+\.[05])\-" + r"(?P<number>[0-9]+)$", + branch_name, + ) + + if not version_match: + raise ValueError(f"Unable to parse the version from the ref {branch_name}") + + self.name = branch_name + self.prefix = version_match.group("prefix") + self.browser_version = version_match.group("browser") + self._is_head = is_head + self._ref = "HEAD" if is_head else f"origin/{branch_name}" + + firefox_nums = [int(n) for n in version_match.group("firefox").split(".")] + if len(firefox_nums) == 2: + firefox_nums.append(0) + browser_nums = [int(n) for n in self.browser_version.split(".")] + branch_number = int(version_match.group("number")) + # Prioritise the firefox ESR version, then the browser version then the + # branch number. + self._ordered = ( + firefox_nums[0], + firefox_nums[1], + firefox_nums[2], + browser_nums[0], + browser_nums[1], + branch_number, + ) + + # Minor version for browser is only ever "0" or "5", so we can convert + # the version to an integer. + self._browser_int_version = int(2 * float(self.browser_version)) + + self._file_paths: list[str] | None = None + + def release_below(self, other: "BrowserBranch", num: int) -> bool: + """Determine whether another branch is within range of a previous + browser release. + + The browser versions are expected to increment by "0.5", and a previous + release branch's version is expected to be `num * 0.5` behind the + current one. + + :param other: The branch to compare. + :param num: The number of "0.5" releases behind to test with. + """ + return other._browser_int_version == self._browser_int_version - num + + def __lt__(self, other: "BrowserBranch") -> bool: + return self._ordered < other._ordered + + def __gt__(self, other: "BrowserBranch") -> bool: + return self._ordered > other._ordered + + def _matching_dirs(self, path: str, dir_list: list[str]) -> bool: + """Test that a path is contained in the list of dirs. + + :param path: The path to check. + :param dir_list: The list of directories to check against. + :returns: Whether the path matches. + """ + for dir_path in dir_list: + if os.path.commonpath([dir_path, path]) == dir_path: + return True + return False + + def get_file( + self, filename: str, search_dirs: list[str] | None + ) -> TranslationFile | None: + """Fetch the file content for the named file in this branch. + + :param filename: The name of the file to fetch the content for. + :param search_dirs: The directories to restrict the search to, or None + to search for the file anywhere. + :returns: The file, or `None` if no file could be found. + """ + if self._file_paths is None: + if not self._is_head: + # Minimal fetch of non-HEAD branch to get the file paths. + # Individual file blobs will be downloaded as needed. + git_run( + ["fetch", "--depth=1", "--filter=blob:none", "origin", self.name] + ) + self._file_paths = git_lines( + ["ls-tree", "-r", "--format=%(path)", self._ref] + ) + + matching = [ + path + for path in self._file_paths + if os.path.basename(path) == filename + and (search_dirs is None or self._matching_dirs(path, search_dirs)) + ] + if not matching: + return None + if len(matching) > 1: + raise Exception(f"Multiple occurrences of {filename}") + + path = matching[0] + + return TranslationFile( + path=path, content=git_text(["cat-file", "blob", f"{self._ref}:{path}"]) + ) + + +def get_stable_branch( + compare_version: BrowserBranch, +) -> tuple[BrowserBranch, BrowserBranch | None]: + """Find the most recent stable branch in the origin repository. + + :param compare_version: The development branch to compare against. + :returns: The stable and legacy branches. If no legacy branch is found, + `None` will be returned instead. + """ + # We search for build1 tags. These are added *after* the rebase of browser + # commits, so the corresponding branch should contain our strings. + # Moreover, we *assume* that the branch with the most recent ESR version + # with such a tag will be used in the *next* stable build in + # tor-browser-build. + tag_glob = f"{compare_version.prefix}-*-build1" + + # To speed up, only fetch the tags without blobs. + git_run( + ["fetch", "--depth=1", "--filter=object:type=tag", "origin", "tag", tag_glob] + ) + stable_branches = [] + legacy_branches = [] + stable_annotation_regex = re.compile(r"\bstable\b") + legacy_annotation_regex = re.compile(r"\blegacy\b") + tag_pattern = re.compile( + rf"^{re.escape(compare_version.prefix)}-[^-]+esr-[^-]+-[^-]+-build1$" + ) + + for build_tag, annotation in ( + line.split(" ", 1) for line in git_lines(["tag", "-n1", "--list", tag_glob]) + ): + if not tag_pattern.match(build_tag): + continue + is_stable = bool(stable_annotation_regex.search(annotation)) + is_legacy = bool(legacy_annotation_regex.search(annotation)) + if not is_stable and not is_legacy: + continue + try: + # Branch name is the same as the tag, minus "-build1". + branch = BrowserBranch(re.sub(r"-build1$", "", build_tag)) + except ValueError: + logger.warning(f"Could not read the version for {build_tag}") + continue + if branch.prefix != compare_version.prefix: + continue + if is_stable: + # Stable can be one release version behind. + # NOTE: In principle, when switching between versions there may be a + # window of time where the development branch has not yet progressed + # to the next "0.5" release, so has the same browser version as the + # stable branch. So we also allow for matching browser versions. + # NOTE: + # 1. The "Will be unused in" message will not make sense, but we do + # not expect string differences in this scenario. + # 2. We do not expect this scenario to last for long. + if not ( + compare_version.release_below(branch, 1) + or compare_version.release_below(branch, 0) + ): + continue + stable_branches.append(branch) + elif is_legacy: + # Legacy can be two release versions behind. + # We also allow for being just one version behind. + if not ( + compare_version.release_below(branch, 2) + or compare_version.release_below(branch, 1) + ): + continue + legacy_branches.append(branch) + + if not stable_branches: + raise Exception("No stable build1 branch found") + + return ( + # Return the stable branch with the highest version. + max(stable_branches), + max(legacy_branches) if legacy_branches else None, + ) + + +current_branch = BrowserBranch(args.current_branch, is_head=True) + +stable_branch, legacy_branch = get_stable_branch(current_branch) + +if os.environ.get("TRANSLATION_INCLUDE_LEGACY", "") != "true": + legacy_branch = None + +files_list = [] + +for file_dict in json.loads(args.files): + name = file_dict["name"] + where_dirs = file_dict.get("where", None) + current_file = current_branch.get_file(name, where_dirs) + stable_file = stable_branch.get_file(name, where_dirs) + + if current_file is None and stable_file is None: + # No file in either branch. + logger.warning(f"{name} does not exist in either the current or stable branch") + elif current_file is None: + logger.warning(f"{name} deleted in the current branch") + elif stable_file is None: + logger.warning(f"{name} does not exist in the stable branch") + elif current_file.path != stable_file.path: + logger.warning( + f"{name} has different paths in the current and stable branch. " + f"{current_file.path} : {stable_file.path}" + ) + + content = combine_files( + name, + None if current_file is None else current_file.content, + None if stable_file is None else stable_file.content, + f"Will be unused in Tor Browser {current_branch.browser_version}!", + ) + + if legacy_branch and not file_dict.get("exclude-legacy", False): + legacy_file = legacy_branch.get_file(name, where_dirs) + if legacy_file is not None and current_file is None and stable_file is None: + logger.warning(f"{name} still exists in the legacy branch") + elif legacy_file is None: + logger.warning(f"{name} does not exist in the legacy branch") + elif stable_file is not None and legacy_file.path != stable_file.path: + logger.warning( + f"{name} has different paths in the stable and legacy branch. " + f"{stable_file.path} : {legacy_file.path}" + ) + elif current_file is not None and legacy_file.path != current_file.path: + logger.warning( + f"{name} has different paths in the current and legacy branch. " + f"{current_file.path} : {legacy_file.path}" + ) + + content = combine_files( + name, + content, + legacy_file.content, + f"Unused in Tor Browser {stable_branch.browser_version}!", + ) + elif legacy_branch: + logger.info(f"Excluding legacy branch for {name}") + + files_list.append( + { + "name": name, + # If "directory" is unspecified, we place the file directly beneath + # en-US/ in the translation repository. i.e. "". + "directory": file_dict.get("directory", ""), + "branch": file_dict["branch"], + "content": content, + } + ) + + +ci_commit = os.environ.get("CI_COMMIT_SHA", "") +ci_url_base = os.environ.get("CI_PROJECT_URL", "") + +json_data = { + "commit": ci_commit, + "commit-url": f"{ci_url_base}/-/commit/{ci_commit}" + if (ci_commit and ci_url_base) + else "", + "project-path": os.environ.get("CI_PROJECT_PATH", ""), + "current-branch": current_branch.name, + "stable-branch": stable_branch.name, + "files": files_list, +} + +if legacy_branch: + json_data["legacy-branch"] = legacy_branch.name + +with open(args.outname, "w") as file: + json.dump(json_data, file) ===================================== tools/base-browser/l10n/combine/__init__.py ===================================== @@ -0,0 +1,3 @@ +# flake8: noqa + +from .combine import combine_files ===================================== tools/base-browser/l10n/combine/combine.py ===================================== @@ -0,0 +1,181 @@ +import re +from typing import TYPE_CHECKING, Any + +from compare_locales.parser import getParser +from compare_locales.parser.android import AndroidEntity, DocumentWrapper +from compare_locales.parser.base import Comment, Entity, Junk, Whitespace +from compare_locales.parser.dtd import DTDEntity +from compare_locales.parser.fluent import FluentComment, FluentEntity +from compare_locales.parser.properties import PropertiesEntity + +if TYPE_CHECKING: + from collections.abc import Iterable + + +def combine_files( + filename: str, + new_content: str | None, + old_content: str | None, + comment_prefix: str, +) -> str | None: + """Combine two translation files into one to include all strings from both. + The new content is presented first, and any strings only found in the old + content are placed at the end with an additional comment. + + :param filename: The filename for the file, determines the format. + :param new_content: The new content for the file, or None if it has been + deleted. + :param old_content: The old content for the file, or None if it did not + exist before. + :comment_prefix: A comment to include for any strings that are only found in + the old content. This will be placed before any other comments for the + string. + + :returns: The combined content, or None if both given contents are None. + """ + if new_content is None and old_content is None: + return None + + # getParser from compare_locale returns the same instance for the same file + # extension. + parser = getParser(filename) + + is_android = filename.endswith(".xml") + if new_content is None: + if is_android: + # File was deleted, add some document parts. + content_start = ( + '<?xml version="1.0" encoding="utf-8" standalone="yes"?>\n<resources>\n' + ) + content_end = "</resources>\n" + else: + # Treat as an empty file. + content_start = "" + content_end = "" + existing_keys = [] + else: + parser.readUnicode(new_content) + + # Start with the same content as the current file. + # For android strings, we want to keep the final "</resources>" until after. + if is_android: + closing_match = re.match( + r"^(.*)(</resources>\s*)$", parser.ctx.contents, re.DOTALL + ) + if not closing_match: + raise ValueError("Missing a final </resources>") + content_start = closing_match.group(1) + content_end = closing_match.group(2) + else: + content_start = parser.ctx.contents + content_end = "" + existing_keys = [entry.key for entry in parser.walk(only_localizable=True)] + + # For Fluent, we want to prefix the strings using GroupComments. + # On weblate this will cause all the strings that fall under the GroupComment's + # scope to have the prefix added to their "notes". + # We set up an initial GroupComment for the first string we find. This will also + # end the scope of the last GroupComment in the new translation file. + # This will be replaced with a the next GroupComment when it is found. + fluent_group_comment_prefix = f"\n## {comment_prefix}\n" + fluent_group_comment: str | None = fluent_group_comment_prefix + + # For other formats, we want to keep all the comment lines that come directly + # before the string. + # In compare_locales.parser, only the comment line directly before an Entity + # counts as the pre_comment for that Entity. I.e. only this line will be + # included in Entity.all + # However, in weblate every comment line that comes before the Entity is + # included as a comment. So we also want to keep these additional comments to + # preserve them for weblate. + # We gather these extra comments in stacked_comments, and clear them whenever we + # reach an Entity or a blank line (Whitespace is more than "\n"). + stacked_comments: list[str] = [] + + additions: list[str] = [] + + entry_iter: Iterable[Any] = () + # If the file does not exist in the old branch, don't make any additions. + if old_content is not None: + parser.readUnicode(old_content) + entry_iter = parser.walk(only_localizable=False) + for entry in entry_iter: + if isinstance(entry, Junk): + raise ValueError(f"Unexpected Junk: {entry.all}") + if isinstance(entry, Whitespace): + # Clear stacked comments if more than one empty line. + if entry.all != "\n": + stacked_comments.clear() + continue + if isinstance(entry, Comment): + if isinstance(entry, FluentComment): + # Don't stack Fluent comments. + # Only the comments included in Entity.pre_comment count towards + # that Entity's comment. + if entry.all.startswith("##"): + # A Fluent GroupComment + if entry.all == "##": + # Empty GroupComment. Used to end the scope of a previous + # GroupComment. + # Replace this with our prefix comment. + fluent_group_comment = fluent_group_comment_prefix + else: + # Prefix the group comment. + fluent_group_comment = ( + f"{fluent_group_comment_prefix}{entry.all}\n" + ) + else: + stacked_comments.append(entry.all) + continue + if isinstance(entry, DocumentWrapper): + # Not needed. + continue + + if not isinstance(entry, Entity): + raise ValueError(f"Unexpected type: {entry.__class__.__name__}") + + if entry.key in existing_keys: + # Already included this string in the new translation file. + # Drop the gathered comments for this Entity. + stacked_comments.clear() + continue + + if isinstance(entry, FluentEntity): + if fluent_group_comment is not None: + # We have a found GroupComment which has not been included yet. + # All following Entity's will be under its scope, until the next + # GroupComment. + additions.append(fluent_group_comment) + # Added GroupComment, so don't need to add again. + fluent_group_comment = None + elif isinstance(entry, DTDEntity): + # Include our additional comment before we print the rest for this + # Entity. + additions.append(f"") + elif isinstance(entry, PropertiesEntity): + additions.append(f"# {comment_prefix}") + elif isinstance(entry, AndroidEntity): + additions.append(f"") + else: + raise ValueError(f"Unexpected Entity type: {entry.__class__.__name__}") + + # Add any other comment lines that came directly before this Entity. + additions.extend(stacked_comments) + stacked_comments.clear() + additions.append(entry.all) + + content_middle = "" + + if additions: + # New line before and after the additions + additions.insert(0, "") + additions.append("") + if is_android: + content_middle = "\n ".join(additions) + else: + content_middle = "\n".join(additions) + + # Remove " " in otherwise blank lines. + content_middle = re.sub("^ +$", "", content_middle, flags=re.MULTILINE) + + return content_start + content_middle + content_end ===================================== tools/base-browser/l10n/combine/tests/README ===================================== @@ -0,0 +1,2 @@ +python tests to be run with pytest. +Requires the compare-locales package. ===================================== tools/base-browser/l10n/combine/tests/__init__.py ===================================== ===================================== tools/base-browser/l10n/combine/tests/test_android.py ===================================== @@ -0,0 +1,330 @@ +import textwrap + +from combine import combine_files + + +def wrap_in_xml(content): + if content is None: + return None + # Allow for indents to make the tests more readable. + content = textwrap.dedent(content) + return f"""\ +<?xml version="1.0" encoding="utf-8" standalone="yes"?> +<resources> +{textwrap.indent(content, " ")}</resources> +""" + + +def assert_result(new_content, old_content, expect): + new_content = wrap_in_xml(new_content) + old_content = wrap_in_xml(old_content) + expect = wrap_in_xml(expect) + assert expect == combine_files( + "test_strings.xml", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + None, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ + +  + <string name="string_1">First</string> +  + <string name="string_2">Second</string> + """, + ) + + +def test_no_change(): + content = """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + <string name="string_1">First</string> + <string name="string_new">NEW</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="string_new">NEW</string> + <string name="string_2">Second</string> + """, + ) + + +def test_removed_string(): + assert_result( + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="removed">REMOVED</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + +  + <string name="removed">REMOVED</string> + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + <string name="new_1">New string</string> + <string name="string_1">First</string> + <string name="string_2">Second</string> + <string name="new_2">New string 2</string> + """, + """\ + <string name="string_1">First</string> + <string name="removed_1">First removed</string> + <string name="removed_2">Second removed</string> + <string name="string_2">Second</string> + <string name="removed_3">Third removed</string> + """, + """\ + <string name="new_1">New string</string> + <string name="string_1">First</string> + <string name="string_2">Second</string> + <string name="new_2">New string 2</string> + +  + <string name="removed_1">First removed</string> +  + <string name="removed_2">Second removed</string> +  + <string name="removed_3">Third removed</string> + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + <string name="changed_string">NEW</string> + """, + """\ + <string name="changed_string">OLD</string> + """, + """\ + <string name="changed_string">NEW</string> + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ +  + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + ) + # Comment added. + assert_result( + """\ +  + <string name="changed_string">string</string> + """, + """\ + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + ) + # Comment removed. + assert_result( + """\ + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + """\ + <string name="changed_string">string</string> + """, + ) + + # With file comments + assert_result( + """\ +  + +  + <string name="changed_string">string</string> + """, + """\ +  + +  + <string name="changed_string">string</string> + """, + """\ +  + +  + <string name="changed_string">string</string> + """, + ) + + +def test_reordered(): + # String was re_ordered. + assert_result( + """\ + <string name="string_1">value</string> + <string name="moved_string">move</string> + """, + """\ + <string name="moved_string">move</string> + <string name="string_1">value</string> + """, + """\ + <string name="string_1">value</string> + <string name="moved_string">move</string> + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ +  + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ +  + <string name="string_1">First</string> +  + <string name="removed">REMOVED</string> + <string name="string_2">Second</string> + """, + """\ +  + <string name="string_1">First</string> + <string name="string_2">Second</string> + +  +  + <string name="removed">REMOVED</string> + """, + ) + + # With file comments and multi-line. + # All comments prior to a removed string are moved with it, until another + # entity or blank line is reached. + assert_result( + """\ +  + +  +  + <string name="string_1">First</string> + +  +  + + <string name="string_2">Second</string> + """, + """\ +  + +  +  + <string name="string_1">First</string> + <string name="removed_1">First removed</string> +  + <string name="removed_2">Second removed</string> + +  + +  +  + <string name="removed_3">Third removed</string> + +  +  + + <string name="removed_4">Fourth removed</string> + <string name="string_2">Second</string> + """, + """\ +  + +  +  + <string name="string_1">First</string> + +  +  + + <string name="string_2">Second</string> + +  + <string name="removed_1">First removed</string> +  +  + <string name="removed_2">Second removed</string> +  +  +  + <string name="removed_3">Third removed</string> +  + <string name="removed_4">Fourth removed</string> + """, + ) ===================================== tools/base-browser/l10n/combine/tests/test_dtd.py ===================================== @@ -0,0 +1,325 @@ +import textwrap + +from combine import combine_files + + +def assert_result(new_content, old_content, expect): + # Allow for indents to make the tests more readable. + if new_content is not None: + new_content = textwrap.dedent(new_content) + if old_content is not None: + old_content = textwrap.dedent(old_content) + if expect is not None: + expect = textwrap.dedent(expect) + assert expect == combine_files( + "test.dtd", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + None, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ + +  + <!ENTITY string.1 "First"> +  + <!ENTITY string.2 "Second"> + """, + ) + + +def test_no_change(): + content = """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.new "NEW"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.new "NEW"> + <!ENTITY string.2 "Second"> + """, + ) + + +def test_removed_string(): + assert_result( + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY removed "REMOVED"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + +  + <!ENTITY removed "REMOVED"> + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + <!ENTITY new.1 "New string"> + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + <!ENTITY new.2 "New string 2"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY removed.1 "First removed"> + <!ENTITY removed.2 "Second removed"> + <!ENTITY string.2 "Second"> + <!ENTITY removed.3 "Third removed"> + """, + """\ + <!ENTITY new.1 "New string"> + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + <!ENTITY new.2 "New string 2"> + +  + <!ENTITY removed.1 "First removed"> +  + <!ENTITY removed.2 "Second removed"> +  + <!ENTITY removed.3 "Third removed"> + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + <!ENTITY changed.string "NEW"> + """, + """\ + <!ENTITY changed.string "OLD"> + """, + """\ + <!ENTITY changed.string "NEW"> + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ +  + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + ) + # Comment added. + assert_result( + """\ +  + <!ENTITY changed.string "string"> + """, + """\ + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + ) + # Comment removed. + assert_result( + """\ + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + """\ + <!ENTITY changed.string "string"> + """, + ) + + # With multiple comments + assert_result( + """\ +  + +  + <!ENTITY changed.string "string"> + """, + """\ +  + +  + <!ENTITY changed.string "string"> + """, + """\ +  + +  + <!ENTITY changed.string "string"> + """, + ) + + +def test_reordered(): + # String was re.ordered. + assert_result( + """\ + <!ENTITY string.1 "value"> + <!ENTITY moved.string "move"> + """, + """\ + <!ENTITY moved.string "move"> + <!ENTITY string.1 "value"> + """, + """\ + <!ENTITY string.1 "value"> + <!ENTITY moved.string "move"> + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ +  + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ +  + <!ENTITY string.1 "First"> +  + <!ENTITY removed "REMOVED"> + <!ENTITY string.2 "Second"> + """, + """\ +  + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + +  +  + <!ENTITY removed "REMOVED"> + """, + ) + + # With multiple lines of comments. + + assert_result( + """\ +  + +  +  + <!ENTITY string.1 "First"> + +  + + <!ENTITY string.2 "Second"> + """, + """\ +  + +  + <!ENTITY string.1 "First"> + <!ENTITY removed.1 "First removed"> +  + <!ENTITY removed.2 "Second removed"> + +  + +  +  + <!ENTITY removed.3 "Third removed"> + +  + + <!ENTITY removed.4 "Fourth removed"> + <!ENTITY string.2 "Second"> + """, + """\ +  + +  +  + <!ENTITY string.1 "First"> + +  + + <!ENTITY string.2 "Second"> + +  + <!ENTITY removed.1 "First removed"> +  +  + <!ENTITY removed.2 "Second removed"> +  +  +  + <!ENTITY removed.3 "Third removed"> +  + <!ENTITY removed.4 "Fourth removed"> + """, + ) ===================================== tools/base-browser/l10n/combine/tests/test_fluent.py ===================================== @@ -0,0 +1,344 @@ +import textwrap + +from combine import combine_files + + +def assert_result(new_content, old_content, expect): + # Allow for indents to make the tests more readable. + if new_content is not None: + new_content = textwrap.dedent(new_content) + if old_content is not None: + old_content = textwrap.dedent(old_content) + if expect is not None: + expect = textwrap.dedent(expect) + assert expect == combine_files( + "test.ftl", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + string-1 = First + string-2 = Second + """, + None, + """\ + string-1 = First + string-2 = Second + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + string-1 = First + string-2 = Second + """, + """\ + + + ## REMOVED STRING + + string-1 = First + string-2 = Second + """, + ) + + +def test_no_change(): + content = """\ + string-1 = First + string-2 = Second + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + string-1 = First + string-new = NEW + string-2 = Second + """, + """\ + string-1 = First + string-2 = Second + """, + """\ + string-1 = First + string-new = NEW + string-2 = Second + """, + ) + + +def test_removed_string(): + assert_result( + """\ + string-1 = First + string-2 = Second + """, + """\ + string-1 = First + removed = REMOVED + string-2 = Second + """, + """\ + string-1 = First + string-2 = Second + + + ## REMOVED STRING + + removed = REMOVED + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + new-1 = New string + string-1 = + .attr = First + string-2 = Second + new-2 = + .title = New string 2 + """, + """\ + string-1 = + .attr = First + removed-1 = First removed + removed-2 = + .attr = Second removed + string-2 = Second + removed-3 = Third removed + """, + """\ + new-1 = New string + string-1 = + .attr = First + string-2 = Second + new-2 = + .title = New string 2 + + + ## REMOVED STRING + + removed-1 = First removed + removed-2 = + .attr = Second removed + removed-3 = Third removed + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + changed-string = NEW + """, + """\ + changed-string = OLD + """, + """\ + changed-string = NEW + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ + # NEW + changed-string = string + """, + """\ + # OLD + changed-string = string + """, + """\ + # NEW + changed-string = string + """, + ) + # Comment added. + assert_result( + """\ + # NEW + changed-string = string + """, + """\ + changed-string = string + """, + """\ + # NEW + changed-string = string + """, + ) + # Comment removed. + assert_result( + """\ + changed-string = string + """, + """\ + # OLD + changed-string = string + """, + """\ + changed-string = string + """, + ) + + # With group comments. + assert_result( + """\ + ## GROUP NEW + + # NEW + changed-string = string + """, + """\ + ## GROUP OLD + + # OLD + changed-string = string + """, + """\ + ## GROUP NEW + + # NEW + changed-string = string + """, + ) + + +def test_reordered(): + # String was re-ordered. + assert_result( + """\ + string-1 = value + moved-string = move + """, + """\ + moved-string = move + string-1 = value + """, + """\ + string-1 = value + moved-string = move + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ + # Comment for first. + string-1 = First + string-2 = Second + """, + """\ + # Comment for first. + string-1 = First + # Comment for removed. + removed = REMOVED + string-2 = Second + """, + """\ + # Comment for first. + string-1 = First + string-2 = Second + + + ## REMOVED STRING + + # Comment for removed. + removed = REMOVED + """, + ) + + # Group comments are combined with the "REMOVED STRING" comments. + # If strings have no group comment, then a single "REMOVED STRING" is + # included for them. + assert_result( + """\ + ## First Group comment + + # Comment for first. + string-1 = First + + ## + + no-group = No group comment + + ## Second + ## Group comment + + string-2 = Second + """, + """\ + ## First Group comment + + # Comment for first. + string-1 = First + removed-1 = First removed + # Comment for second removed. + removed-2 = Second removed + + ## + + no-group = No group comment + removed-3 = Third removed + + ## Second + ## Group comment + + removed-4 = Fourth removed + string-2 = Second + """, + """\ + ## First Group comment + + # Comment for first. + string-1 = First + + ## + + no-group = No group comment + + ## Second + ## Group comment + + string-2 = Second + + + ## REMOVED STRING + ## First Group comment + + removed-1 = First removed + # Comment for second removed. + removed-2 = Second removed + + ## REMOVED STRING + + removed-3 = Third removed + + ## REMOVED STRING + ## Second + ## Group comment + + removed-4 = Fourth removed + """, + ) ===================================== tools/base-browser/l10n/combine/tests/test_properties.py ===================================== @@ -0,0 +1,322 @@ +import textwrap + +from combine import combine_files + + +def assert_result(new_content, old_content, expect): + # Allow for indents to make the tests more readable. + if new_content is not None: + new_content = textwrap.dedent(new_content) + if old_content is not None: + old_content = textwrap.dedent(old_content) + if expect is not None: + expect = textwrap.dedent(expect) + assert expect == combine_files( + "test.properties", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + string.1 = First + string.2 = Second + """, + None, + """\ + string.1 = First + string.2 = Second + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + string.1 = First + string.2 = Second + """, + """\ + + # REMOVED STRING + string.1 = First + # REMOVED STRING + string.2 = Second + """, + ) + + +def test_no_change(): + content = """\ + string.1 = First + string.2 = Second + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + string.1 = First + string.new = NEW + string.2 = Second + """, + """\ + string.1 = First + string.2 = Second + """, + """\ + string.1 = First + string.new = NEW + string.2 = Second + """, + ) + + +def test_removed_string(): + assert_result( + """\ + string.1 = First + string.2 = Second + """, + """\ + string.1 = First + removed = REMOVED + string.2 = Second + """, + """\ + string.1 = First + string.2 = Second + + # REMOVED STRING + removed = REMOVED + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + new.1 = New string + string.1 = First + string.2 = Second + new.2 = New string 2 + """, + """\ + string.1 = First + removed.1 = First removed + removed.2 = Second removed + string.2 = Second + removed.3 = Third removed + """, + """\ + new.1 = New string + string.1 = First + string.2 = Second + new.2 = New string 2 + + # REMOVED STRING + removed.1 = First removed + # REMOVED STRING + removed.2 = Second removed + # REMOVED STRING + removed.3 = Third removed + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + changed.string = NEW + """, + """\ + changed.string = OLD + """, + """\ + changed.string = NEW + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ + # NEW + changed.string = string + """, + """\ + # OLD + changed.string = string + """, + """\ + # NEW + changed.string = string + """, + ) + # Comment added. + assert_result( + """\ + # NEW + changed.string = string + """, + """\ + changed.string = string + """, + """\ + # NEW + changed.string = string + """, + ) + # Comment removed. + assert_result( + """\ + changed.string = string + """, + """\ + # OLD + changed.string = string + """, + """\ + changed.string = string + """, + ) + + # With file comments + assert_result( + """\ + # NEW file comment + + # NEW + changed.string = string + """, + """\ + # OLD file comment + + # OLD + changed.string = string + """, + """\ + # NEW file comment + + # NEW + changed.string = string + """, + ) + + +def test_reordered(): + # String was re.ordered. + assert_result( + """\ + string.1 = value + moved.string = move + """, + """\ + moved.string = move + string.1 = value + """, + """\ + string.1 = value + moved.string = move + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ + # Comment for first. + string.1 = First + string.2 = Second + """, + """\ + # Comment for first. + string.1 = First + # Comment for removed. + removed = REMOVED + string.2 = Second + """, + """\ + # Comment for first. + string.1 = First + string.2 = Second + + # REMOVED STRING + # Comment for removed. + removed = REMOVED + """, + ) + + # With file comments and multi-line. + # All comments prior to a removed string are moved with it, until another + # entity or blank line is reached. + assert_result( + """\ + # First File comment + + # Comment for first. + # Comment 2 for first. + string.1 = First + + # Second + # File comment + + string.2 = Second + """, + """\ + # First File comment + + # Comment for first. + # Comment 2 for first. + string.1 = First + removed.1 = First removed + # Comment for second removed. + removed.2 = Second removed + + # Removed file comment + + # Comment 1 for third removed + # Comment 2 for third removed + removed.3 = Third removed + + # Second + # File comment + + removed.4 = Fourth removed + string.2 = Second + """, + """\ + # First File comment + + # Comment for first. + # Comment 2 for first. + string.1 = First + + # Second + # File comment + + string.2 = Second + + # REMOVED STRING + removed.1 = First removed + # REMOVED STRING + # Comment for second removed. + removed.2 = Second removed + # REMOVED STRING + # Comment 1 for third removed + # Comment 2 for third removed + removed.3 = Third removed + # REMOVED STRING + removed.4 = Fourth removed + """, + ) View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/compare/76… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/compare/76… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] fixup! TB 42247: Android helpers for the TorProvider
by Pier Angelo Vendrame (＠pierov) 22 Jan '25

22 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 1323ab5f by Pier Angelo Vendrame at 2025-01-22T12:11:01+01:00 fixup! TB 42247: Android helpers for the TorProvider Add a missing return to #getAllSettings(). - - - - - 1 changed file: - toolkit/modules/TorAndroidIntegration.sys.mjs Changes: ===================================== toolkit/modules/TorAndroidIntegration.sys.mjs ===================================== @@ -97,6 +97,7 @@ class TorAndroidIntegrationImpl { #getAllSettings() { const settings = lazy.TorSettings.getSettings(); settings.quickstart = { enabled: lazy.TorConnect.quickstart }; + return settings; } observe(subj, topic) { View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/1323ab5… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/1323ab5… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][base-browser-128.6.0esr-14.5-1] 2 commits: fixup! Add CI for Base Browser
by Pier Angelo Vendrame (＠pierov) 22 Jan '25

22 Jan '25

Pier Angelo Vendrame pushed to branch base-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 5f316b25 by Henry Wilkes at 2025-01-22T12:13:24+01:00 fixup! Add CI for Base Browser MB 324: Move update-translations CI to base-browser. - - - - - 9fc48603 by Henry Wilkes at 2025-01-22T12:13:34+01:00 BB 42305: Add script to combine translation files across versions. - - - - - 11 changed files: - .gitlab-ci.yml - + .gitlab/ci/jobs/update-translations.yml - + tools/base-browser/l10n/combine-translation-versions.py - + tools/base-browser/l10n/combine/__init__.py - + tools/base-browser/l10n/combine/combine.py - + tools/base-browser/l10n/combine/tests/README - + tools/base-browser/l10n/combine/tests/__init__.py - + tools/base-browser/l10n/combine/tests/test_android.py - + tools/base-browser/l10n/combine/tests/test_dtd.py - + tools/base-browser/l10n/combine/tests/test_fluent.py - + tools/base-browser/l10n/combine/tests/test_properties.py Changes: ===================================== .gitlab-ci.yml ===================================== @@ -1,5 +1,6 @@ stages: - lint + - update-translations variables: IMAGE_PATH: containers.torproject.org/tpo/applications/tor-browser/base:latest ===================================== .gitlab/ci/jobs/update-translations.yml ===================================== @@ -0,0 +1,53 @@ +.update-translation-base: + stage: update-translations + rules: + - if: ($TRANSLATION_FILES != "" && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push") + changes: + - "**/*.ftl" + - "**/*.properties" + - "**/*.dtd" + - "**/*strings.xml" + - "**/update-translations.yml" + - "**/l10n/combine/combine.py" + - "**/l10n/combine-translation-versions.py" + - if: ($TRANSLATION_FILES != "" && $FORCE_UPDATE_TRANSLATIONS == "true") + variables: + COMBINED_FILES_JSON: "combined-translation-files.json" + TRANSLATION_FILES: '' + + +combine-en-US-translations: + extends: .update-translation-base + needs: [] + image: python + variables: + PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip" + cache: + paths: + - .cache/pip + # Artifact is for translation project job + artifacts: + paths: + - "$COMBINED_FILES_JSON" + expire_in: "60 min" + reports: + dotenv: job_id.env + # Don't load artifacts for this job. + dependencies: [] + script: + # Save this CI_JOB_ID to the dotenv file to be used in the variables for the + # push-en-US-translations job. + - echo 'COMBINE_TRANSLATIONS_JOB_ID='"$CI_JOB_ID" >job_id.env + - pip install compare_locales + - python ./tools/base-browser/l10n/combine-translation-versions.py "$CI_COMMIT_BRANCH" "$TRANSLATION_FILES" "$COMBINED_FILES_JSON" + +push-en-US-translations: + extends: .update-translation-base + needs: + - job: combine-en-US-translations + variables: + COMBINED_FILES_JSON_URL: "${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/jobs/${COMBINE_TRANSLATIONS_JOB_ID}/artifacts/${COMBINED_FILES_JSON}" + trigger: + strategy: depend + project: tor-browser-translation-bot/translation + branch: tor-browser-ci ===================================== tools/base-browser/l10n/combine-translation-versions.py ===================================== @@ -0,0 +1,368 @@ +import argparse +import json +import logging +import os +import re +import subprocess + +from combine import combine_files + +arg_parser = argparse.ArgumentParser( + description="Combine a translation file across two different versions" +) + +arg_parser.add_argument( + "current_branch", metavar="<current-branch>", help="branch for the newest version" +) +arg_parser.add_argument( + "files", metavar="<files>", help="JSON specifying the translation files" +) +arg_parser.add_argument("outname", metavar="<json>", help="name of the json output") + +args = arg_parser.parse_args() + +logging.basicConfig() +logger = logging.getLogger("combine-translation-versions") +logger.setLevel(logging.INFO) + + +def in_pink(msg: str) -> str: + """Present a message as pink in the terminal output. + + :param msg: The message to wrap in pink. + :returns: The message to print to terminal. + """ + # Pink and bold. + return f"\x1b[1;38;5;212m{msg}\x1b[0m" + + +def git_run(git_args: list[str]) -> None: + """Run a git command. + + :param git_args: The arguments that should follow "git". + """ + # Add some text to give context to git's stderr appearing in log. + logger.info("Running: " + in_pink("git " + " ".join(git_args))) + subprocess.run(["git", *git_args], check=True) + + +def git_text(git_args: list[str]) -> str: + """Get the text output for a git command. + + :param git_args: The arguments that should follow "git". + :returns: The stdout of the command. + """ + logger.info("Running: " + in_pink("git " + " ".join(git_args))) + return subprocess.run( + ["git", *git_args], text=True, check=True, stdout=subprocess.PIPE + ).stdout + + +def git_lines(git_args: list[str]) -> list[str]: + """Get the lines from a git command. + + :param git_args: The arguments that should follow "git". + :returns: The non-empty lines from stdout of the command. + """ + return [line for line in git_text(git_args).split("\n") if line] + + +class TranslationFile: + """Represents a translation file.""" + + def __init__(self, path: str, content: str) -> None: + self.path = path + self.content = content + + +class BrowserBranch: + """Represents a browser git branch.""" + + def __init__(self, branch_name: str, is_head: bool = False) -> None: + """Create a new instance. + + :param branch_name: The branch's git name. + :param is_head: Whether the branch matches "HEAD". + """ + version_match = re.match( + r"(?P<prefix>[a-z]+\-browser)\-" + r"(?P<firefox>[0-9]+(?:\.[0-9]+){1,2})esr\-" + r"(?P<browser>[0-9]+\.[05])\-" + r"(?P<number>[0-9]+)$", + branch_name, + ) + + if not version_match: + raise ValueError(f"Unable to parse the version from the ref {branch_name}") + + self.name = branch_name + self.prefix = version_match.group("prefix") + self.browser_version = version_match.group("browser") + self._is_head = is_head + self._ref = "HEAD" if is_head else f"origin/{branch_name}" + + firefox_nums = [int(n) for n in version_match.group("firefox").split(".")] + if len(firefox_nums) == 2: + firefox_nums.append(0) + browser_nums = [int(n) for n in self.browser_version.split(".")] + branch_number = int(version_match.group("number")) + # Prioritise the firefox ESR version, then the browser version then the + # branch number. + self._ordered = ( + firefox_nums[0], + firefox_nums[1], + firefox_nums[2], + browser_nums[0], + browser_nums[1], + branch_number, + ) + + # Minor version for browser is only ever "0" or "5", so we can convert + # the version to an integer. + self._browser_int_version = int(2 * float(self.browser_version)) + + self._file_paths: list[str] | None = None + + def release_below(self, other: "BrowserBranch", num: int) -> bool: + """Determine whether another branch is within range of a previous + browser release. + + The browser versions are expected to increment by "0.5", and a previous + release branch's version is expected to be `num * 0.5` behind the + current one. + + :param other: The branch to compare. + :param num: The number of "0.5" releases behind to test with. + """ + return other._browser_int_version == self._browser_int_version - num + + def __lt__(self, other: "BrowserBranch") -> bool: + return self._ordered < other._ordered + + def __gt__(self, other: "BrowserBranch") -> bool: + return self._ordered > other._ordered + + def _matching_dirs(self, path: str, dir_list: list[str]) -> bool: + """Test that a path is contained in the list of dirs. + + :param path: The path to check. + :param dir_list: The list of directories to check against. + :returns: Whether the path matches. + """ + for dir_path in dir_list: + if os.path.commonpath([dir_path, path]) == dir_path: + return True + return False + + def get_file( + self, filename: str, search_dirs: list[str] | None + ) -> TranslationFile | None: + """Fetch the file content for the named file in this branch. + + :param filename: The name of the file to fetch the content for. + :param search_dirs: The directories to restrict the search to, or None + to search for the file anywhere. + :returns: The file, or `None` if no file could be found. + """ + if self._file_paths is None: + if not self._is_head: + # Minimal fetch of non-HEAD branch to get the file paths. + # Individual file blobs will be downloaded as needed. + git_run( + ["fetch", "--depth=1", "--filter=blob:none", "origin", self.name] + ) + self._file_paths = git_lines( + ["ls-tree", "-r", "--format=%(path)", self._ref] + ) + + matching = [ + path + for path in self._file_paths + if os.path.basename(path) == filename + and (search_dirs is None or self._matching_dirs(path, search_dirs)) + ] + if not matching: + return None + if len(matching) > 1: + raise Exception(f"Multiple occurrences of {filename}") + + path = matching[0] + + return TranslationFile( + path=path, content=git_text(["cat-file", "blob", f"{self._ref}:{path}"]) + ) + + +def get_stable_branch( + compare_version: BrowserBranch, +) -> tuple[BrowserBranch, BrowserBranch | None]: + """Find the most recent stable branch in the origin repository. + + :param compare_version: The development branch to compare against. + :returns: The stable and legacy branches. If no legacy branch is found, + `None` will be returned instead. + """ + # We search for build1 tags. These are added *after* the rebase of browser + # commits, so the corresponding branch should contain our strings. + # Moreover, we *assume* that the branch with the most recent ESR version + # with such a tag will be used in the *next* stable build in + # tor-browser-build. + tag_glob = f"{compare_version.prefix}-*-build1" + + # To speed up, only fetch the tags without blobs. + git_run( + ["fetch", "--depth=1", "--filter=object:type=tag", "origin", "tag", tag_glob] + ) + stable_branches = [] + legacy_branches = [] + stable_annotation_regex = re.compile(r"\bstable\b") + legacy_annotation_regex = re.compile(r"\blegacy\b") + tag_pattern = re.compile( + rf"^{re.escape(compare_version.prefix)}-[^-]+esr-[^-]+-[^-]+-build1$" + ) + + for build_tag, annotation in ( + line.split(" ", 1) for line in git_lines(["tag", "-n1", "--list", tag_glob]) + ): + if not tag_pattern.match(build_tag): + continue + is_stable = bool(stable_annotation_regex.search(annotation)) + is_legacy = bool(legacy_annotation_regex.search(annotation)) + if not is_stable and not is_legacy: + continue + try: + # Branch name is the same as the tag, minus "-build1". + branch = BrowserBranch(re.sub(r"-build1$", "", build_tag)) + except ValueError: + logger.warning(f"Could not read the version for {build_tag}") + continue + if branch.prefix != compare_version.prefix: + continue + if is_stable: + # Stable can be one release version behind. + # NOTE: In principle, when switching between versions there may be a + # window of time where the development branch has not yet progressed + # to the next "0.5" release, so has the same browser version as the + # stable branch. So we also allow for matching browser versions. + # NOTE: + # 1. The "Will be unused in" message will not make sense, but we do + # not expect string differences in this scenario. + # 2. We do not expect this scenario to last for long. + if not ( + compare_version.release_below(branch, 1) + or compare_version.release_below(branch, 0) + ): + continue + stable_branches.append(branch) + elif is_legacy: + # Legacy can be two release versions behind. + # We also allow for being just one version behind. + if not ( + compare_version.release_below(branch, 2) + or compare_version.release_below(branch, 1) + ): + continue + legacy_branches.append(branch) + + if not stable_branches: + raise Exception("No stable build1 branch found") + + return ( + # Return the stable branch with the highest version. + max(stable_branches), + max(legacy_branches) if legacy_branches else None, + ) + + +current_branch = BrowserBranch(args.current_branch, is_head=True) + +stable_branch, legacy_branch = get_stable_branch(current_branch) + +if os.environ.get("TRANSLATION_INCLUDE_LEGACY", "") != "true": + legacy_branch = None + +files_list = [] + +for file_dict in json.loads(args.files): + name = file_dict["name"] + where_dirs = file_dict.get("where", None) + current_file = current_branch.get_file(name, where_dirs) + stable_file = stable_branch.get_file(name, where_dirs) + + if current_file is None and stable_file is None: + # No file in either branch. + logger.warning(f"{name} does not exist in either the current or stable branch") + elif current_file is None: + logger.warning(f"{name} deleted in the current branch") + elif stable_file is None: + logger.warning(f"{name} does not exist in the stable branch") + elif current_file.path != stable_file.path: + logger.warning( + f"{name} has different paths in the current and stable branch. " + f"{current_file.path} : {stable_file.path}" + ) + + content = combine_files( + name, + None if current_file is None else current_file.content, + None if stable_file is None else stable_file.content, + f"Will be unused in Tor Browser {current_branch.browser_version}!", + ) + + if legacy_branch and not file_dict.get("exclude-legacy", False): + legacy_file = legacy_branch.get_file(name, where_dirs) + if legacy_file is not None and current_file is None and stable_file is None: + logger.warning(f"{name} still exists in the legacy branch") + elif legacy_file is None: + logger.warning(f"{name} does not exist in the legacy branch") + elif stable_file is not None and legacy_file.path != stable_file.path: + logger.warning( + f"{name} has different paths in the stable and legacy branch. " + f"{stable_file.path} : {legacy_file.path}" + ) + elif current_file is not None and legacy_file.path != current_file.path: + logger.warning( + f"{name} has different paths in the current and legacy branch. " + f"{current_file.path} : {legacy_file.path}" + ) + + content = combine_files( + name, + content, + legacy_file.content, + f"Unused in Tor Browser {stable_branch.browser_version}!", + ) + elif legacy_branch: + logger.info(f"Excluding legacy branch for {name}") + + files_list.append( + { + "name": name, + # If "directory" is unspecified, we place the file directly beneath + # en-US/ in the translation repository. i.e. "". + "directory": file_dict.get("directory", ""), + "branch": file_dict["branch"], + "content": content, + } + ) + + +ci_commit = os.environ.get("CI_COMMIT_SHA", "") +ci_url_base = os.environ.get("CI_PROJECT_URL", "") + +json_data = { + "commit": ci_commit, + "commit-url": f"{ci_url_base}/-/commit/{ci_commit}" + if (ci_commit and ci_url_base) + else "", + "project-path": os.environ.get("CI_PROJECT_PATH", ""), + "current-branch": current_branch.name, + "stable-branch": stable_branch.name, + "files": files_list, +} + +if legacy_branch: + json_data["legacy-branch"] = legacy_branch.name + +with open(args.outname, "w") as file: + json.dump(json_data, file) ===================================== tools/base-browser/l10n/combine/__init__.py ===================================== @@ -0,0 +1,3 @@ +# flake8: noqa + +from .combine import combine_files ===================================== tools/base-browser/l10n/combine/combine.py ===================================== @@ -0,0 +1,181 @@ +import re +from typing import TYPE_CHECKING, Any + +from compare_locales.parser import getParser +from compare_locales.parser.android import AndroidEntity, DocumentWrapper +from compare_locales.parser.base import Comment, Entity, Junk, Whitespace +from compare_locales.parser.dtd import DTDEntity +from compare_locales.parser.fluent import FluentComment, FluentEntity +from compare_locales.parser.properties import PropertiesEntity + +if TYPE_CHECKING: + from collections.abc import Iterable + + +def combine_files( + filename: str, + new_content: str | None, + old_content: str | None, + comment_prefix: str, +) -> str | None: + """Combine two translation files into one to include all strings from both. + The new content is presented first, and any strings only found in the old + content are placed at the end with an additional comment. + + :param filename: The filename for the file, determines the format. + :param new_content: The new content for the file, or None if it has been + deleted. + :param old_content: The old content for the file, or None if it did not + exist before. + :comment_prefix: A comment to include for any strings that are only found in + the old content. This will be placed before any other comments for the + string. + + :returns: The combined content, or None if both given contents are None. + """ + if new_content is None and old_content is None: + return None + + # getParser from compare_locale returns the same instance for the same file + # extension. + parser = getParser(filename) + + is_android = filename.endswith(".xml") + if new_content is None: + if is_android: + # File was deleted, add some document parts. + content_start = ( + '<?xml version="1.0" encoding="utf-8" standalone="yes"?>\n<resources>\n' + ) + content_end = "</resources>\n" + else: + # Treat as an empty file. + content_start = "" + content_end = "" + existing_keys = [] + else: + parser.readUnicode(new_content) + + # Start with the same content as the current file. + # For android strings, we want to keep the final "</resources>" until after. + if is_android: + closing_match = re.match( + r"^(.*)(</resources>\s*)$", parser.ctx.contents, re.DOTALL + ) + if not closing_match: + raise ValueError("Missing a final </resources>") + content_start = closing_match.group(1) + content_end = closing_match.group(2) + else: + content_start = parser.ctx.contents + content_end = "" + existing_keys = [entry.key for entry in parser.walk(only_localizable=True)] + + # For Fluent, we want to prefix the strings using GroupComments. + # On weblate this will cause all the strings that fall under the GroupComment's + # scope to have the prefix added to their "notes". + # We set up an initial GroupComment for the first string we find. This will also + # end the scope of the last GroupComment in the new translation file. + # This will be replaced with a the next GroupComment when it is found. + fluent_group_comment_prefix = f"\n## {comment_prefix}\n" + fluent_group_comment: str | None = fluent_group_comment_prefix + + # For other formats, we want to keep all the comment lines that come directly + # before the string. + # In compare_locales.parser, only the comment line directly before an Entity + # counts as the pre_comment for that Entity. I.e. only this line will be + # included in Entity.all + # However, in weblate every comment line that comes before the Entity is + # included as a comment. So we also want to keep these additional comments to + # preserve them for weblate. + # We gather these extra comments in stacked_comments, and clear them whenever we + # reach an Entity or a blank line (Whitespace is more than "\n"). + stacked_comments: list[str] = [] + + additions: list[str] = [] + + entry_iter: Iterable[Any] = () + # If the file does not exist in the old branch, don't make any additions. + if old_content is not None: + parser.readUnicode(old_content) + entry_iter = parser.walk(only_localizable=False) + for entry in entry_iter: + if isinstance(entry, Junk): + raise ValueError(f"Unexpected Junk: {entry.all}") + if isinstance(entry, Whitespace): + # Clear stacked comments if more than one empty line. + if entry.all != "\n": + stacked_comments.clear() + continue + if isinstance(entry, Comment): + if isinstance(entry, FluentComment): + # Don't stack Fluent comments. + # Only the comments included in Entity.pre_comment count towards + # that Entity's comment. + if entry.all.startswith("##"): + # A Fluent GroupComment + if entry.all == "##": + # Empty GroupComment. Used to end the scope of a previous + # GroupComment. + # Replace this with our prefix comment. + fluent_group_comment = fluent_group_comment_prefix + else: + # Prefix the group comment. + fluent_group_comment = ( + f"{fluent_group_comment_prefix}{entry.all}\n" + ) + else: + stacked_comments.append(entry.all) + continue + if isinstance(entry, DocumentWrapper): + # Not needed. + continue + + if not isinstance(entry, Entity): + raise ValueError(f"Unexpected type: {entry.__class__.__name__}") + + if entry.key in existing_keys: + # Already included this string in the new translation file. + # Drop the gathered comments for this Entity. + stacked_comments.clear() + continue + + if isinstance(entry, FluentEntity): + if fluent_group_comment is not None: + # We have a found GroupComment which has not been included yet. + # All following Entity's will be under its scope, until the next + # GroupComment. + additions.append(fluent_group_comment) + # Added GroupComment, so don't need to add again. + fluent_group_comment = None + elif isinstance(entry, DTDEntity): + # Include our additional comment before we print the rest for this + # Entity. + additions.append(f"") + elif isinstance(entry, PropertiesEntity): + additions.append(f"# {comment_prefix}") + elif isinstance(entry, AndroidEntity): + additions.append(f"") + else: + raise ValueError(f"Unexpected Entity type: {entry.__class__.__name__}") + + # Add any other comment lines that came directly before this Entity. + additions.extend(stacked_comments) + stacked_comments.clear() + additions.append(entry.all) + + content_middle = "" + + if additions: + # New line before and after the additions + additions.insert(0, "") + additions.append("") + if is_android: + content_middle = "\n ".join(additions) + else: + content_middle = "\n".join(additions) + + # Remove " " in otherwise blank lines. + content_middle = re.sub("^ +$", "", content_middle, flags=re.MULTILINE) + + return content_start + content_middle + content_end ===================================== tools/base-browser/l10n/combine/tests/README ===================================== @@ -0,0 +1,2 @@ +python tests to be run with pytest. +Requires the compare-locales package. ===================================== tools/base-browser/l10n/combine/tests/__init__.py ===================================== ===================================== tools/base-browser/l10n/combine/tests/test_android.py ===================================== @@ -0,0 +1,330 @@ +import textwrap + +from combine import combine_files + + +def wrap_in_xml(content): + if content is None: + return None + # Allow for indents to make the tests more readable. + content = textwrap.dedent(content) + return f"""\ +<?xml version="1.0" encoding="utf-8" standalone="yes"?> +<resources> +{textwrap.indent(content, " ")}</resources> +""" + + +def assert_result(new_content, old_content, expect): + new_content = wrap_in_xml(new_content) + old_content = wrap_in_xml(old_content) + expect = wrap_in_xml(expect) + assert expect == combine_files( + "test_strings.xml", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + None, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ + +  + <string name="string_1">First</string> +  + <string name="string_2">Second</string> + """, + ) + + +def test_no_change(): + content = """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + <string name="string_1">First</string> + <string name="string_new">NEW</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="string_new">NEW</string> + <string name="string_2">Second</string> + """, + ) + + +def test_removed_string(): + assert_result( + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="removed">REMOVED</string> + <string name="string_2">Second</string> + """, + """\ + <string name="string_1">First</string> + <string name="string_2">Second</string> + +  + <string name="removed">REMOVED</string> + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + <string name="new_1">New string</string> + <string name="string_1">First</string> + <string name="string_2">Second</string> + <string name="new_2">New string 2</string> + """, + """\ + <string name="string_1">First</string> + <string name="removed_1">First removed</string> + <string name="removed_2">Second removed</string> + <string name="string_2">Second</string> + <string name="removed_3">Third removed</string> + """, + """\ + <string name="new_1">New string</string> + <string name="string_1">First</string> + <string name="string_2">Second</string> + <string name="new_2">New string 2</string> + +  + <string name="removed_1">First removed</string> +  + <string name="removed_2">Second removed</string> +  + <string name="removed_3">Third removed</string> + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + <string name="changed_string">NEW</string> + """, + """\ + <string name="changed_string">OLD</string> + """, + """\ + <string name="changed_string">NEW</string> + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ +  + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + ) + # Comment added. + assert_result( + """\ +  + <string name="changed_string">string</string> + """, + """\ + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + ) + # Comment removed. + assert_result( + """\ + <string name="changed_string">string</string> + """, + """\ +  + <string name="changed_string">string</string> + """, + """\ + <string name="changed_string">string</string> + """, + ) + + # With file comments + assert_result( + """\ +  + +  + <string name="changed_string">string</string> + """, + """\ +  + +  + <string name="changed_string">string</string> + """, + """\ +  + +  + <string name="changed_string">string</string> + """, + ) + + +def test_reordered(): + # String was re_ordered. + assert_result( + """\ + <string name="string_1">value</string> + <string name="moved_string">move</string> + """, + """\ + <string name="moved_string">move</string> + <string name="string_1">value</string> + """, + """\ + <string name="string_1">value</string> + <string name="moved_string">move</string> + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ +  + <string name="string_1">First</string> + <string name="string_2">Second</string> + """, + """\ +  + <string name="string_1">First</string> +  + <string name="removed">REMOVED</string> + <string name="string_2">Second</string> + """, + """\ +  + <string name="string_1">First</string> + <string name="string_2">Second</string> + +  +  + <string name="removed">REMOVED</string> + """, + ) + + # With file comments and multi-line. + # All comments prior to a removed string are moved with it, until another + # entity or blank line is reached. + assert_result( + """\ +  + +  +  + <string name="string_1">First</string> + +  +  + + <string name="string_2">Second</string> + """, + """\ +  + +  +  + <string name="string_1">First</string> + <string name="removed_1">First removed</string> +  + <string name="removed_2">Second removed</string> + +  + +  +  + <string name="removed_3">Third removed</string> + +  +  + + <string name="removed_4">Fourth removed</string> + <string name="string_2">Second</string> + """, + """\ +  + +  +  + <string name="string_1">First</string> + +  +  + + <string name="string_2">Second</string> + +  + <string name="removed_1">First removed</string> +  +  + <string name="removed_2">Second removed</string> +  +  +  + <string name="removed_3">Third removed</string> +  + <string name="removed_4">Fourth removed</string> + """, + ) ===================================== tools/base-browser/l10n/combine/tests/test_dtd.py ===================================== @@ -0,0 +1,325 @@ +import textwrap + +from combine import combine_files + + +def assert_result(new_content, old_content, expect): + # Allow for indents to make the tests more readable. + if new_content is not None: + new_content = textwrap.dedent(new_content) + if old_content is not None: + old_content = textwrap.dedent(old_content) + if expect is not None: + expect = textwrap.dedent(expect) + assert expect == combine_files( + "test.dtd", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + None, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ + +  + <!ENTITY string.1 "First"> +  + <!ENTITY string.2 "Second"> + """, + ) + + +def test_no_change(): + content = """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.new "NEW"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.new "NEW"> + <!ENTITY string.2 "Second"> + """, + ) + + +def test_removed_string(): + assert_result( + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY removed "REMOVED"> + <!ENTITY string.2 "Second"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + +  + <!ENTITY removed "REMOVED"> + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + <!ENTITY new.1 "New string"> + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + <!ENTITY new.2 "New string 2"> + """, + """\ + <!ENTITY string.1 "First"> + <!ENTITY removed.1 "First removed"> + <!ENTITY removed.2 "Second removed"> + <!ENTITY string.2 "Second"> + <!ENTITY removed.3 "Third removed"> + """, + """\ + <!ENTITY new.1 "New string"> + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + <!ENTITY new.2 "New string 2"> + +  + <!ENTITY removed.1 "First removed"> +  + <!ENTITY removed.2 "Second removed"> +  + <!ENTITY removed.3 "Third removed"> + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + <!ENTITY changed.string "NEW"> + """, + """\ + <!ENTITY changed.string "OLD"> + """, + """\ + <!ENTITY changed.string "NEW"> + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ +  + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + ) + # Comment added. + assert_result( + """\ +  + <!ENTITY changed.string "string"> + """, + """\ + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + ) + # Comment removed. + assert_result( + """\ + <!ENTITY changed.string "string"> + """, + """\ +  + <!ENTITY changed.string "string"> + """, + """\ + <!ENTITY changed.string "string"> + """, + ) + + # With multiple comments + assert_result( + """\ +  + +  + <!ENTITY changed.string "string"> + """, + """\ +  + +  + <!ENTITY changed.string "string"> + """, + """\ +  + +  + <!ENTITY changed.string "string"> + """, + ) + + +def test_reordered(): + # String was re.ordered. + assert_result( + """\ + <!ENTITY string.1 "value"> + <!ENTITY moved.string "move"> + """, + """\ + <!ENTITY moved.string "move"> + <!ENTITY string.1 "value"> + """, + """\ + <!ENTITY string.1 "value"> + <!ENTITY moved.string "move"> + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ +  + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + """, + """\ +  + <!ENTITY string.1 "First"> +  + <!ENTITY removed "REMOVED"> + <!ENTITY string.2 "Second"> + """, + """\ +  + <!ENTITY string.1 "First"> + <!ENTITY string.2 "Second"> + +  +  + <!ENTITY removed "REMOVED"> + """, + ) + + # With multiple lines of comments. + + assert_result( + """\ +  + +  +  + <!ENTITY string.1 "First"> + +  + + <!ENTITY string.2 "Second"> + """, + """\ +  + +  + <!ENTITY string.1 "First"> + <!ENTITY removed.1 "First removed"> +  + <!ENTITY removed.2 "Second removed"> + +  + +  +  + <!ENTITY removed.3 "Third removed"> + +  + + <!ENTITY removed.4 "Fourth removed"> + <!ENTITY string.2 "Second"> + """, + """\ +  + +  +  + <!ENTITY string.1 "First"> + +  + + <!ENTITY string.2 "Second"> + +  + <!ENTITY removed.1 "First removed"> +  +  + <!ENTITY removed.2 "Second removed"> +  +  +  + <!ENTITY removed.3 "Third removed"> +  + <!ENTITY removed.4 "Fourth removed"> + """, + ) ===================================== tools/base-browser/l10n/combine/tests/test_fluent.py ===================================== @@ -0,0 +1,344 @@ +import textwrap + +from combine import combine_files + + +def assert_result(new_content, old_content, expect): + # Allow for indents to make the tests more readable. + if new_content is not None: + new_content = textwrap.dedent(new_content) + if old_content is not None: + old_content = textwrap.dedent(old_content) + if expect is not None: + expect = textwrap.dedent(expect) + assert expect == combine_files( + "test.ftl", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + string-1 = First + string-2 = Second + """, + None, + """\ + string-1 = First + string-2 = Second + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + string-1 = First + string-2 = Second + """, + """\ + + + ## REMOVED STRING + + string-1 = First + string-2 = Second + """, + ) + + +def test_no_change(): + content = """\ + string-1 = First + string-2 = Second + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + string-1 = First + string-new = NEW + string-2 = Second + """, + """\ + string-1 = First + string-2 = Second + """, + """\ + string-1 = First + string-new = NEW + string-2 = Second + """, + ) + + +def test_removed_string(): + assert_result( + """\ + string-1 = First + string-2 = Second + """, + """\ + string-1 = First + removed = REMOVED + string-2 = Second + """, + """\ + string-1 = First + string-2 = Second + + + ## REMOVED STRING + + removed = REMOVED + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + new-1 = New string + string-1 = + .attr = First + string-2 = Second + new-2 = + .title = New string 2 + """, + """\ + string-1 = + .attr = First + removed-1 = First removed + removed-2 = + .attr = Second removed + string-2 = Second + removed-3 = Third removed + """, + """\ + new-1 = New string + string-1 = + .attr = First + string-2 = Second + new-2 = + .title = New string 2 + + + ## REMOVED STRING + + removed-1 = First removed + removed-2 = + .attr = Second removed + removed-3 = Third removed + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + changed-string = NEW + """, + """\ + changed-string = OLD + """, + """\ + changed-string = NEW + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ + # NEW + changed-string = string + """, + """\ + # OLD + changed-string = string + """, + """\ + # NEW + changed-string = string + """, + ) + # Comment added. + assert_result( + """\ + # NEW + changed-string = string + """, + """\ + changed-string = string + """, + """\ + # NEW + changed-string = string + """, + ) + # Comment removed. + assert_result( + """\ + changed-string = string + """, + """\ + # OLD + changed-string = string + """, + """\ + changed-string = string + """, + ) + + # With group comments. + assert_result( + """\ + ## GROUP NEW + + # NEW + changed-string = string + """, + """\ + ## GROUP OLD + + # OLD + changed-string = string + """, + """\ + ## GROUP NEW + + # NEW + changed-string = string + """, + ) + + +def test_reordered(): + # String was re-ordered. + assert_result( + """\ + string-1 = value + moved-string = move + """, + """\ + moved-string = move + string-1 = value + """, + """\ + string-1 = value + moved-string = move + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ + # Comment for first. + string-1 = First + string-2 = Second + """, + """\ + # Comment for first. + string-1 = First + # Comment for removed. + removed = REMOVED + string-2 = Second + """, + """\ + # Comment for first. + string-1 = First + string-2 = Second + + + ## REMOVED STRING + + # Comment for removed. + removed = REMOVED + """, + ) + + # Group comments are combined with the "REMOVED STRING" comments. + # If strings have no group comment, then a single "REMOVED STRING" is + # included for them. + assert_result( + """\ + ## First Group comment + + # Comment for first. + string-1 = First + + ## + + no-group = No group comment + + ## Second + ## Group comment + + string-2 = Second + """, + """\ + ## First Group comment + + # Comment for first. + string-1 = First + removed-1 = First removed + # Comment for second removed. + removed-2 = Second removed + + ## + + no-group = No group comment + removed-3 = Third removed + + ## Second + ## Group comment + + removed-4 = Fourth removed + string-2 = Second + """, + """\ + ## First Group comment + + # Comment for first. + string-1 = First + + ## + + no-group = No group comment + + ## Second + ## Group comment + + string-2 = Second + + + ## REMOVED STRING + ## First Group comment + + removed-1 = First removed + # Comment for second removed. + removed-2 = Second removed + + ## REMOVED STRING + + removed-3 = Third removed + + ## REMOVED STRING + ## Second + ## Group comment + + removed-4 = Fourth removed + """, + ) ===================================== tools/base-browser/l10n/combine/tests/test_properties.py ===================================== @@ -0,0 +1,322 @@ +import textwrap + +from combine import combine_files + + +def assert_result(new_content, old_content, expect): + # Allow for indents to make the tests more readable. + if new_content is not None: + new_content = textwrap.dedent(new_content) + if old_content is not None: + old_content = textwrap.dedent(old_content) + if expect is not None: + expect = textwrap.dedent(expect) + assert expect == combine_files( + "test.properties", new_content, old_content, "REMOVED STRING" + ) + + +def test_combine_empty(): + assert_result(None, None, None) + + +def test_combine_new_file(): + # New file with no old content. + assert_result( + """\ + string.1 = First + string.2 = Second + """, + None, + """\ + string.1 = First + string.2 = Second + """, + ) + + +def test_combine_removed_file(): + # Entire file was removed. + assert_result( + None, + """\ + string.1 = First + string.2 = Second + """, + """\ + + # REMOVED STRING + string.1 = First + # REMOVED STRING + string.2 = Second + """, + ) + + +def test_no_change(): + content = """\ + string.1 = First + string.2 = Second + """ + assert_result(content, content, content) + + +def test_added_string(): + assert_result( + """\ + string.1 = First + string.new = NEW + string.2 = Second + """, + """\ + string.1 = First + string.2 = Second + """, + """\ + string.1 = First + string.new = NEW + string.2 = Second + """, + ) + + +def test_removed_string(): + assert_result( + """\ + string.1 = First + string.2 = Second + """, + """\ + string.1 = First + removed = REMOVED + string.2 = Second + """, + """\ + string.1 = First + string.2 = Second + + # REMOVED STRING + removed = REMOVED + """, + ) + + +def test_removed_and_added(): + assert_result( + """\ + new.1 = New string + string.1 = First + string.2 = Second + new.2 = New string 2 + """, + """\ + string.1 = First + removed.1 = First removed + removed.2 = Second removed + string.2 = Second + removed.3 = Third removed + """, + """\ + new.1 = New string + string.1 = First + string.2 = Second + new.2 = New string 2 + + # REMOVED STRING + removed.1 = First removed + # REMOVED STRING + removed.2 = Second removed + # REMOVED STRING + removed.3 = Third removed + """, + ) + + +def test_updated(): + # String content was updated. + assert_result( + """\ + changed.string = NEW + """, + """\ + changed.string = OLD + """, + """\ + changed.string = NEW + """, + ) + + +def test_updated_comment(): + # String comment was updated. + assert_result( + """\ + # NEW + changed.string = string + """, + """\ + # OLD + changed.string = string + """, + """\ + # NEW + changed.string = string + """, + ) + # Comment added. + assert_result( + """\ + # NEW + changed.string = string + """, + """\ + changed.string = string + """, + """\ + # NEW + changed.string = string + """, + ) + # Comment removed. + assert_result( + """\ + changed.string = string + """, + """\ + # OLD + changed.string = string + """, + """\ + changed.string = string + """, + ) + + # With file comments + assert_result( + """\ + # NEW file comment + + # NEW + changed.string = string + """, + """\ + # OLD file comment + + # OLD + changed.string = string + """, + """\ + # NEW file comment + + # NEW + changed.string = string + """, + ) + + +def test_reordered(): + # String was re.ordered. + assert_result( + """\ + string.1 = value + moved.string = move + """, + """\ + moved.string = move + string.1 = value + """, + """\ + string.1 = value + moved.string = move + """, + ) + + +def test_removed_string_with_comment(): + assert_result( + """\ + # Comment for first. + string.1 = First + string.2 = Second + """, + """\ + # Comment for first. + string.1 = First + # Comment for removed. + removed = REMOVED + string.2 = Second + """, + """\ + # Comment for first. + string.1 = First + string.2 = Second + + # REMOVED STRING + # Comment for removed. + removed = REMOVED + """, + ) + + # With file comments and multi-line. + # All comments prior to a removed string are moved with it, until another + # entity or blank line is reached. + assert_result( + """\ + # First File comment + + # Comment for first. + # Comment 2 for first. + string.1 = First + + # Second + # File comment + + string.2 = Second + """, + """\ + # First File comment + + # Comment for first. + # Comment 2 for first. + string.1 = First + removed.1 = First removed + # Comment for second removed. + removed.2 = Second removed + + # Removed file comment + + # Comment 1 for third removed + # Comment 2 for third removed + removed.3 = Third removed + + # Second + # File comment + + removed.4 = Fourth removed + string.2 = Second + """, + """\ + # First File comment + + # Comment for first. + # Comment 2 for first. + string.1 = First + + # Second + # File comment + + string.2 = Second + + # REMOVED STRING + removed.1 = First removed + # REMOVED STRING + # Comment for second removed. + removed.2 = Second removed + # REMOVED STRING + # Comment 1 for third removed + # Comment 2 for third removed + removed.3 = Third removed + # REMOVED STRING + removed.4 = Fourth removed + """, + ) View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/186910… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/186910… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 5 commits: fixup! Add CI for Tor Browser
by Pier Angelo Vendrame (＠pierov) 22 Jan '25

22 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 4fd00231 by Henry Wilkes at 2025-01-22T11:06:59+00:00 fixup! Add CI for Tor Browser MB 324: Revert tor-browser changes that add update-translations CI. - - - - - f467154f by Henry Wilkes at 2025-01-22T11:06:59+00:00 Revert "TB 42305: Add script to combine translation files across versions." This reverts commit 3247633d3f6bf60be7feaf2e3515d9b7c2b1dad0. - - - - - 675287aa by Henry Wilkes at 2025-01-22T11:06:59+00:00 fixup! Add CI for Base Browser MB 324: Move update-translations CI to base-browser. - - - - - 4c57d159 by Henry Wilkes at 2025-01-22T11:06:59+00:00 BB 42305: Add script to combine translation files across versions. - - - - - e0847a5c by Henry Wilkes at 2025-01-22T11:06:59+00:00 fixup! Add CI for Tor Browser MB 324: Re-introduce tor-browser changes to update-translations - - - - - 10 changed files: - .gitlab/ci/jobs/update-translations.yml - tools/torbrowser/l10n/combine-translation-versions.py → tools/base-browser/l10n/combine-translation-versions.py - tools/torbrowser/l10n/combine/__init__.py → tools/base-browser/l10n/combine/__init__.py - tools/torbrowser/l10n/combine/combine.py → tools/base-browser/l10n/combine/combine.py - tools/torbrowser/l10n/combine/tests/README → tools/base-browser/l10n/combine/tests/README - tools/torbrowser/l10n/combine/tests/__init__.py → tools/base-browser/l10n/combine/tests/__init__.py - tools/torbrowser/l10n/combine/tests/test_android.py → tools/base-browser/l10n/combine/tests/test_android.py - tools/torbrowser/l10n/combine/tests/test_dtd.py → tools/base-browser/l10n/combine/tests/test_dtd.py - tools/torbrowser/l10n/combine/tests/test_fluent.py → tools/base-browser/l10n/combine/tests/test_fluent.py - tools/torbrowser/l10n/combine/tests/test_properties.py → tools/base-browser/l10n/combine/tests/test_properties.py Changes: ===================================== .gitlab/ci/jobs/update-translations.yml ===================================== @@ -1,7 +1,7 @@ .update-translation-base: stage: update-translations rules: - - if: ($CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push") + - if: ($TRANSLATION_FILES != "" && $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push") changes: - "**/*.ftl" - "**/*.properties" @@ -10,17 +10,9 @@ - "**/update-translations.yml" - "**/l10n/combine/combine.py" - "**/l10n/combine-translation-versions.py" - - if: $FORCE_UPDATE_TRANSLATIONS == "true" + - if: ($TRANSLATION_FILES != "" && $FORCE_UPDATE_TRANSLATIONS == "true") variables: - TOR_BROWSER_COMBINED_FILES_JSON: "combined-translation-files.json" - - -combine-en-US-translations: - extends: .update-translation-base - needs: [] - image: python - variables: - PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip" + COMBINED_FILES_JSON: "combined-translation-files.json" TRANSLATION_FILES: '[ { "name": "brand.ftl", @@ -49,13 +41,21 @@ combine-en-US-translations: } ]' TRANSLATION_INCLUDE_LEGACY: "true" + + +combine-en-US-translations: + extends: .update-translation-base + needs: [] + image: python + variables: + PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip" cache: paths: - .cache/pip # Artifact is for translation project job artifacts: paths: - - "$TOR_BROWSER_COMBINED_FILES_JSON" + - "$COMBINED_FILES_JSON" expire_in: "60 min" reports: dotenv: job_id.env @@ -66,14 +66,14 @@ combine-en-US-translations: # push-en-US-translations job. - echo 'COMBINE_TRANSLATIONS_JOB_ID='"$CI_JOB_ID" >job_id.env - pip install compare_locales - - python ./tools/torbrowser/l10n/combine-translation-versions.py "$CI_COMMIT_BRANCH" "$TRANSLATION_FILES" "$TOR_BROWSER_COMBINED_FILES_JSON" + - python ./tools/base-browser/l10n/combine-translation-versions.py "$CI_COMMIT_BRANCH" "$TRANSLATION_FILES" "$COMBINED_FILES_JSON" push-en-US-translations: extends: .update-translation-base needs: - job: combine-en-US-translations variables: - TOR_BROWSER_COMBINED_FILES_JSON_URL: "${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/jobs/${COMBINE_TRANSLATIONS_JOB_ID}/artifacts/${TOR_BROWSER_COMBINED_FILES_JSON}" + COMBINED_FILES_JSON_URL: "${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/jobs/${COMBINE_TRANSLATIONS_JOB_ID}/artifacts/${COMBINED_FILES_JSON}" trigger: strategy: depend project: tor-browser-translation-bot/translation ===================================== tools/torbrowser/l10n/combine-translation-versions.py → tools/base-browser/l10n/combine-translation-versions.py ===================================== ===================================== tools/torbrowser/l10n/combine/__init__.py → tools/base-browser/l10n/combine/__init__.py ===================================== ===================================== tools/torbrowser/l10n/combine/combine.py → tools/base-browser/l10n/combine/combine.py ===================================== ===================================== tools/torbrowser/l10n/combine/tests/README → tools/base-browser/l10n/combine/tests/README ===================================== ===================================== tools/torbrowser/l10n/combine/tests/__init__.py → tools/base-browser/l10n/combine/tests/__init__.py ===================================== ===================================== tools/torbrowser/l10n/combine/tests/test_android.py → tools/base-browser/l10n/combine/tests/test_android.py ===================================== ===================================== tools/torbrowser/l10n/combine/tests/test_dtd.py → tools/base-browser/l10n/combine/tests/test_dtd.py ===================================== ===================================== tools/torbrowser/l10n/combine/tests/test_fluent.py → tools/base-browser/l10n/combine/tests/test_fluent.py ===================================== ===================================== tools/torbrowser/l10n/combine/tests/test_properties.py → tools/base-browser/l10n/combine/tests/test_properties.py ===================================== View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/ecdccd… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/ecdccd… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 2 commits: fixup! TB 40597: Implement TorSettings module
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 266cf821 by Henry Wilkes at 2025-01-21T17:10:38+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Reset bootstrap stage when bridge settings change. In particular, this will cancel an autobootstrap when the user manually changes the bridge settings. However we apply this to all bootstrap attempts for consistency. By resetting to the Start stage, we also ensure that about:torconnect UI no longer shows the "Try bridges" pages when the user manually changes their bridges themselves. - - - - - ecdccd3f by Henry Wilkes at 2025-01-21T17:10:38+00:00 fixup! TB 31286: Implementation of bridge, proxy, and firewall settings in about:preferences#connection TB 41921: Also show the "Connect" button for bridge dialogs when we have an ongoing bootstrap attempt. With the new behaviour, changing the settings while not bootstrapped will cancel any ongoing bootstrap. Therefore, we can start a new bootstrap using the new settings. - - - - - 6 changed files: - browser/components/torpreferences/content/builtinBridgeDialog.js - browser/components/torpreferences/content/connectionPane.js - browser/components/torpreferences/content/provideBridgeDialog.js - browser/components/torpreferences/content/requestBridgeDialog.js - toolkit/modules/TorConnect.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== browser/components/torpreferences/content/builtinBridgeDialog.js ===================================== @@ -4,9 +4,8 @@ const { TorSettings, TorBridgeSource } = ChromeUtils.importESModule( "resource://gre/modules/TorSettings.sys.mjs" ); -const { TorConnect, TorConnectTopics } = ChromeUtils.importESModule( - "resource://gre/modules/TorConnect.sys.mjs" -); +const { TorConnect, TorConnectStage, TorConnectTopics } = + ChromeUtils.importESModule("resource://gre/modules/TorConnect.sys.mjs"); const gBuiltinBridgeDialog = { init() { @@ -96,7 +95,7 @@ const gBuiltinBridgeDialog = { }, onAcceptStateChange() { - const connect = TorConnect.canBeginBootstrap; + const connect = TorConnect.stageName !== TorConnectStage.Bootstrapped; this._result.connect = connect; this._acceptButton.setAttribute( "data-l10n-id", ===================================== browser/components/torpreferences/content/connectionPane.js ===================================== @@ -2257,6 +2257,8 @@ const gBridgeSettings = { } // Wait until the settings are applied before bootstrapping. + // NOTE: Saving the settings should also cancel any existing bootstrap + // attempt first. See tor-browser#41921. savedSettings.then(() => { // The bridge dialog button is "connect" when Tor is not // bootstrapped, so do the connect. ===================================== browser/components/torpreferences/content/provideBridgeDialog.js ===================================== @@ -3,9 +3,8 @@ const { TorSettings, TorBridgeSource, validateBridgeLines } = ChromeUtils.importESModule("resource://gre/modules/TorSettings.sys.mjs"); -const { TorConnect, TorConnectTopics } = ChromeUtils.importESModule( - "resource://gre/modules/TorConnect.sys.mjs" -); +const { TorConnect, TorConnectStage, TorConnectTopics } = + ChromeUtils.importESModule("resource://gre/modules/TorConnect.sys.mjs"); const { TorParsers } = ChromeUtils.importESModule( "resource://gre/modules/TorParsers.sys.mjs" @@ -190,7 +189,7 @@ const gProvideBridgeDialog = { "user-provide-bridge-dialog-next-button" ); } else { - connect = TorConnect.canBeginBootstrap; + connect = TorConnect.stageName !== TorConnectStage.Bootstrapped; this._acceptButton.setAttribute( "data-l10n-id", connect ? "bridge-dialog-button-connect" : "bridge-dialog-button-accept" ===================================== browser/components/torpreferences/content/requestBridgeDialog.js ===================================== @@ -4,9 +4,8 @@ const { BridgeDB } = ChromeUtils.importESModule( "resource://gre/modules/BridgeDB.sys.mjs" ); -const { TorConnect, TorConnectTopics } = ChromeUtils.importESModule( - "resource://gre/modules/TorConnect.sys.mjs" -); +const { TorConnect, TorConnectStage, TorConnectTopics } = + ChromeUtils.importESModule("resource://gre/modules/TorConnect.sys.mjs"); const log = console.createInstance({ maxLogLevel: "Warn", @@ -102,7 +101,7 @@ const gRequestBridgeDialog = { }, onAcceptStateChange() { - const connect = TorConnect.canBeginBootstrap; + const connect = TorConnect.stageName !== TorConnectStage.Bootstrapped; this._result.connect = connect; this._submitButton.setAttribute( "data-l10n-id", ===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -13,6 +13,7 @@ ChromeUtils.defineESModuleGetters(lazy, { TorProviderTopics: "resource://gre/modules/TorProviderBuilder.sys.mjs", TorLauncherUtil: "resource://gre/modules/TorLauncherUtil.sys.mjs", TorSettings: "resource://gre/modules/TorSettings.sys.mjs", + TorSettingsTopics: "resource://gre/modules/TorSettings.sys.mjs", }); const TorConnectPrefs = Object.freeze({ @@ -629,16 +630,6 @@ class AutoBootstrapAttempt { // Send the new settings directly to the provider. We will save them only // if the bootstrap succeeds. - // FIXME: We should somehow signal TorSettings users that we have set - // custom settings, and they should not apply theirs until we are done - // with trying ours. - // Otherwise, the new settings provided by the user while we were - // bootstrapping could be the ones that cause the bootstrap to succeed, - // but we overwrite them (unless we backup the original settings, and then - // save our new settings only if they have not changed). - // Another idea (maybe easier to implement) is to disable the settings - // UI while *any* bootstrap is going on. - // This is also documented in tor-browser#41921. await lazy.TorSettings.applyTemporaryBridges(bridges); if (this.#cancelled || this.#resolved) { @@ -1036,6 +1027,7 @@ export const TorConnect = { // register the Tor topics we always care about observeTopic(lazy.TorProviderTopics.ProcessExited); observeTopic(lazy.TorProviderTopics.HasWarnOrErr); + observeTopic(lazy.TorSettingsTopics.SettingsChanged); // NOTE: At this point, _requestedStage should still be `null`. this._setStage(TorConnectStage.Start); @@ -1070,8 +1062,32 @@ export const TorConnect = { Services.prefs.setBoolPref(TorConnectPrefs.prompt_at_startup, true); this._makeStageRequest(TorConnectStage.Start, true); break; - default: - // ignore + case lazy.TorSettingsTopics.SettingsChanged: + if ( + this._stageName !== TorConnectStage.Bootstrapped && + this._stageName !== TorConnectStage.Loading && + this._stageName !== TorConnectStage.Start && + subject.wrappedJSObject.changes.some(propertyName => + propertyName.startsWith("bridges.") + ) + ) { + // A change in bridge settings before we are bootstrapped, we reset + // the stage to Start to: + // + Cancel any ongoing bootstrap attempt. In particular, we + // definitely do not want to continue with an auto-bootstrap's + // temporary bridges if the settings have changed. + // + Reset the UI to be ready for normal bootstrapping in case the + // user returns to about:torconnect. + // See tor-browser#41921. + // NOTE: We do not reset in response to a change in the quickstart, + // firewall or proxy settings. + lazy.logger.warn( + "Resetting to Start stage since bridge settings changed" + ); + // Rather than cancel and return to the pre-bootstrap stage, we + // explicitly cancel and return to the start stage. + this._makeStageRequest(TorConnectStage.Start); + } break; } }, ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -953,6 +953,7 @@ class TorSettingsImpl { }; if ("bridges" in newValues) { + const changesLength = changes.length; if ("source" in newValues.bridges) { this.#fixupBridgeSettings(newValues.bridges); changeSetting("bridges", "source", newValues.bridges.source); @@ -982,6 +983,19 @@ class TorSettingsImpl { if ("enabled" in newValues.bridges) { changeSetting("bridges", "enabled", newValues.bridges.enabled); } + + if (this.#temporaryBridgeSettings && changes.length !== changesLength) { + // A change in the bridges settings. + // We want to clear the temporary bridge settings to ensure that they + // cannot be used to overwrite these user-provided settings. + // See tor-browser#41921. + // NOTE: This should also trigger TorConnect to cancel any ongoing + // AutoBootstrap that would have otherwise used these settings. + this.#temporaryBridgeSettings = null; + lazy.logger.warn( + "Cleared temporary bridges since bridge settings were changed" + ); + } } if ("proxy" in newValues) { View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/0bb78c… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/0bb78c… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] fixup! [android] Add standalone Tor Bootstrap
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 0bb78caa by clairehurst at 2025-01-21T16:51:39+00:00 fixup! [android] Add standalone Tor Bootstrap Bug 43368: Add @Suppress for linting error "Overriding method should call super. onNewIntent" - - - - - 1 changed file: - mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt Changes: ===================================== mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt ===================================== @@ -4,6 +4,7 @@ package org.mozilla.fenix +import android.annotation.SuppressLint import android.app.assist.AssistContent import android.app.PendingIntent import android.content.Context @@ -710,6 +711,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity, TorIn /** * Handles intents received when the activity is open. */ + @SuppressLint("MissingSuperCall") // super.onNewIntent is called in [onNewIntentInternal(intent)] final override fun onNewIntent(intent: Intent?) { if (intent?.action == ACTION_MAIN || components.torController.isConnected) { onNewIntentInternal(intent) View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/0bb78ca… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/0bb78ca… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] fixup! [android] Add standalone Tor Bootstrap
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: eabeaa5b by clairehurst at 2025-01-21T16:50:59+00:00 fixup! [android] Add standalone Tor Bootstrap Bug 43360: Replace custom variable isBeingRecreated with built-in isFinishing function - - - - - 1 changed file: - mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt Changes: ===================================== mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt ===================================== @@ -176,8 +176,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity, TorIn private var isToolbarInflated = false - private var isBeingRecreated = false - private val webExtensionPopupObserver by lazy { WebExtensionPopupObserver(components.core.store, ::openPopup) } @@ -652,7 +650,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity, TorIn stopMediaSession() } - if (!isBeingRecreated && !(application as FenixApplication).isTerminating()) { + if (isFinishing && !(application as FenixApplication).isTerminating()) { // We assume the Activity is being destroyed because the user // swiped away the app on the Recent screen. When this happens, // we assume the user expects the entire Application is destroyed @@ -686,8 +684,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity, TorIn message = "recreate()", ) - isBeingRecreated = true - super.recreate() } View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/eabeaa5… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/eabeaa5… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] fixup! [android] Implement Android-native Connection Assist UI
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 8e5e2d5c by clairehurst at 2025-01-21T16:49:29+00:00 fixup! [android] Implement Android-native Connection Assist UI Bug 43359: Improper handling of TorBootstrapChangeListener with respect to system onDestroy() calls for HomeActivity - - - - - 1 changed file: - mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt Changes: ===================================== mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt ===================================== @@ -659,12 +659,13 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity, TorIn // and not only the top Activity/Task. Therefore we kill the // underlying Application, as well. (application as FenixApplication).terminate() - } - - val engine = components.core.engine - if (engine is GeckoEngine) { - val torIntegration = engine.getTorIntegrationController() - torIntegration.unregisterBootstrapStateChangeListener(this) + if (settings().useHtmlConnectionUi) { + val engine = components.core.engine + if (engine is GeckoEngine) { + val torIntegration = engine.getTorIntegrationController() + torIntegration.unregisterBootstrapStateChangeListener(this) + } + } } } View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/8e5e2d5… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/8e5e2d5… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 4 commits: fixup! TB 40597: Implement TorSettings module
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 4b7dc6cc by Henry Wilkes at 2025-01-21T15:54:08+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Do not wait for TorSettings to initialise before allowing a bootstrap. Instead, we wait for TorSettings as required: for preparing AutoBootstrap attempts, and TorBootstrapRequest should already wait for TorSettings before attempting to connect. - - - - - 34cfa54a by Henry Wilkes at 2025-01-21T15:54:13+00:00 fixup! TB 40933: Add tor-launcher functionality TB 41921: Ensure TorProviderBuilder.build can be called immediately after TorProviderBuilder.init returns (without await). We make sure that the `#provider` instance is created before any break in execution caused by an `await`. - - - - - 1dc47dd9 by Henry Wilkes at 2025-01-21T15:54:14+00:00 fixup! TB 42247: Android helpers for the TorProvider TB 41921: Do not wait for TorProviderBuilder.init since it is no longer async. - - - - - fff7daea by Henry Wilkes at 2025-01-21T15:54:14+00:00 fixup! TB 40933: Add tor-launcher functionality TB 41921: Add a note to TorBootstrapRequest explaining the expectation that the TorProvider has already read from TorSettings before it connects. - - - - - 4 changed files: - toolkit/components/tor-launcher/TorBootstrapRequest.sys.mjs - toolkit/components/tor-launcher/TorProviderBuilder.sys.mjs - toolkit/modules/TorAndroidIntegration.sys.mjs - toolkit/modules/TorConnect.sys.mjs Changes: ===================================== toolkit/components/tor-launcher/TorBootstrapRequest.sys.mjs ===================================== @@ -90,6 +90,11 @@ export class TorBootstrapRequest { // Wait for bootstrapping to begin and maybe handle error. // Notice that we do not resolve the promise here in case of success, but // we do it from the BootstrapStatus observer. + // NOTE: After TorProviderBuilder.build resolves, TorProvider.init will + // have completed. In particular, assuming no errors, the TorSettings will + // have been initialised and passed on to the provider via + // TorProvider.writeSettings. Therefore we should be safe to immediately + // call `connect` using the latest user settings. lazy.TorProviderBuilder.build() .then(provider => provider.connect()) .catch(err => { ===================================== toolkit/components/tor-launcher/TorProviderBuilder.sys.mjs ===================================== @@ -54,14 +54,15 @@ export class TorProviderBuilder { /** * Initialize the provider of choice. - * Even though initialization is asynchronous, we do not expect the caller to - * await this method. The reason is that any call to build() will wait the - * initialization anyway (and re-throw any initialization error). */ - static async init() { + static init() { switch (this.providerType) { case TorProviders.tor: - await this.#initTorProvider(); + // Even though initialization of the initial TorProvider is + // asynchronous, we do not expect the caller to await it. The reason is + // that any call to build() will wait the initialization anyway (and + // re-throw any initialization error). + this.#initTorProvider(); break; case TorProviders.none: lazy.TorLauncherUtil.setProxyConfiguration( @@ -74,7 +75,12 @@ export class TorProviderBuilder { } } - static async #initTorProvider() { + /** + * Replace #provider with a new instance. + * + * @returns {Promise<TorProvider>} The new instance. + */ + static #initTorProvider() { if (!this.#exitObserver) { this.#exitObserver = this.#torExited.bind(this); Services.obs.addObserver( @@ -83,18 +89,39 @@ export class TorProviderBuilder { ); } + // NOTE: We need to ensure that the #provider is set as soon + // TorProviderBuilder.init is called. + // I.e. it should be safe to call + // TorProviderBuilder.init(); + // TorProviderBuilder.build(); + // without any await. + // + // Therefore, we await the oldProvider within the Promise rather than make + // #initTorProvider async. + // + // In particular, this is needed by TorConnect when the user has selected + // quickstart, in which case `TorConnect.init` will immediately request the + // provider. See tor-browser#41921. + this.#provider = this.#replaceTorProvider(this.#provider); + return this.#provider; + } + + /** + * Replace a TorProvider instance. Resolves once the TorProvider is + * initialised. + * + * @param {Promise<TorProvider>?} oldProvider - The previous's provider's + * promise, if any. + * @returns {TorProvider} The new TorProvider instance. + */ + static async #replaceTorProvider(oldProvider) { try { - const old = await this.#provider; - old?.uninit(); + // Uninitialise the old TorProvider, if there is any. + (await oldProvider)?.uninit(); } catch {} - this.#provider = new Promise((resolve, reject) => { - const provider = new lazy.TorProvider(); - provider - .init() - .then(() => resolve(provider)) - .catch(reject); - }); - await this.#provider; + const provider = new lazy.TorProvider(); + await provider.init(); + return provider; } static uninit() { ===================================== toolkit/modules/TorAndroidIntegration.sys.mjs ===================================== @@ -68,9 +68,12 @@ class TorAndroidIntegrationImpl { Services.obs.addObserver(this, lazy.TorSettingsTopics[topic]); } - lazy.TorProviderBuilder.init().finally(() => { - lazy.TorProviderBuilder.firstWindowLoaded(); - }); + lazy.TorProviderBuilder.init(); + // On Android immediately call firstWindowLoaded. This should be safe to + // call since it will await the initialisation of the TorProvider set up + // by TorProviderBuilder.init. + lazy.TorProviderBuilder.firstWindowLoaded(); + try { await lazy.TorSettings.init(); await lazy.TorConnect.init(); ===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -10,7 +10,6 @@ ChromeUtils.defineESModuleGetters(lazy, { BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.sys.mjs", MoatRPC: "resource://gre/modules/Moat.sys.mjs", TorBootstrapRequest: "resource://gre/modules/TorBootstrapRequest.sys.mjs", - TorProviderBuilder: "resource://gre/modules/TorProviderBuilder.sys.mjs", TorProviderTopics: "resource://gre/modules/TorProviderBuilder.sys.mjs", TorLauncherUtil: "resource://gre/modules/TorLauncherUtil.sys.mjs", TorSettings: "resource://gre/modules/TorSettings.sys.mjs", @@ -436,15 +435,23 @@ class AutoBootstrapAttempt { return; } this.#resolved = true; - try { - // Run cleanup before we resolve the promise to ensure two instances - // of AutoBootstrapAttempt are not trying to change the settings at - // the same time. - if (arg.result !== "complete") { - await lazy.TorSettings.clearTemporaryBridges(); + + if (lazy.TorSettings.initialized) { + // If not initialized, then we won't have applied any settings. + try { + // Run cleanup before we resolve the promise to ensure two instances + // of AutoBootstrapAttempt are not trying to change the settings at + // the same time. + if (arg.result !== "complete") { + // Should do nothing if we never called applyTemporaryBridges. + await lazy.TorSettings.clearTemporaryBridges(); + } + } catch (error) { + lazy.logger.error( + "Unexpected error in auto-bootstrap cleanup", + error + ); } - } catch (error) { - lazy.logger.error("Unexpected error in auto-bootstrap cleanup", error); } if (arg.error) { reject(arg.error); @@ -483,6 +490,16 @@ class AutoBootstrapAttempt { * @param {BootstrapOptions} options - Options to apply to the bootstrap. */ async #runInternal(progressCallback, options) { + // Wait for TorSettings to be initialised, which is used for the + // AutoBootstrapping set up. + await Promise.race([ + lazy.TorSettings.initializedPromise, + this.#cancelledPromise, + ]); + if (this.#cancelled || this.#resolved) { + return; + } + await this.#fetchBridges(options); if (this.#cancelled || this.#resolved) { return; @@ -1016,19 +1033,20 @@ export const TorConnect = { lazy.logger.debug(`Observing topic '${addTopic}'`); }; - // Wait for TorSettings, as we will need it. - // We will wait for a TorProvider only after TorSettings is ready, - // because the TorProviderBuilder initialization might not have finished - // at this point, and TorSettings initialization is a prerequisite for - // having a provider. - // So, we prefer initializing TorConnect as soon as possible, so that - // the UI will be able to detect it is in the Initializing state and act - // consequently. - lazy.TorSettings.initializedPromise.then(() => this._settingsInitialized()); - // register the Tor topics we always care about observeTopic(lazy.TorProviderTopics.ProcessExited); observeTopic(lazy.TorProviderTopics.HasWarnOrErr); + + // NOTE: At this point, _requestedStage should still be `null`. + this._setStage(TorConnectStage.Start); + if ( + // Quickstart setting is enabled. + this.quickstart && + // And the previous bootstrap attempt must have succeeded. + !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) + ) { + this.beginBootstrapping(); + } }, async observe(subject, topic) { @@ -1058,26 +1076,6 @@ export const TorConnect = { } }, - async _settingsInitialized() { - // TODO: Handle failures here, instead of the prompt to restart the - // daemon when it exits (tor-browser#21053, tor-browser#41921). - await lazy.TorProviderBuilder.build(); - - lazy.logger.debug("The TorProvider is ready, changing state."); - // NOTE: If the tor process exits before this point, then - // shouldQuickStart would be `false`. - // NOTE: At this point, _requestedStage should still be `null`. - this._setStage(TorConnectStage.Start); - if ( - // Quickstart setting is enabled. - this.quickstart && - // And the previous bootstrap attempt must have succeeded. - !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) - ) { - this.beginBootstrapping(); - } - }, - /** * Set the user stage. * View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/b5cd60… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/b5cd60… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 4 commits: fixup! TB 40597: Implement TorSettings module
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 21b2be4c by Henry Wilkes at 2025-01-21T15:25:28+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Move quickstart setting from TorSettings to TorConnect. - - - - - cfd39fb0 by Henry Wilkes at 2025-01-21T15:25:29+00:00 fixup! TB 31286: Implementation of bridge, proxy, and firewall settings in about:preferences#connection TB 41921: Move quickstart setting from TorSettings to TorConnect. - - - - - 62e7a81f by Henry Wilkes at 2025-01-21T15:25:30+00:00 fixup! TB 27476: Implement about:torconnect captive portal within Tor Browser TB 41921: Move quickstart setting from TorSettings to TorConnect. This also means we no longer need to listen for TorSettingsTopics.Ready for the initial value. Also fix typo in signal name: "quickstart-changed" to "quickstart-change". - - - - - b5cd602f by Henry Wilkes at 2025-01-21T15:25:31+00:00 fixup! TB 42247: Android helpers for the TorProvider TB 41921: Move quickstart setting from TorSettings to TorConnect. - - - - - 5 changed files: - browser/components/torpreferences/content/connectionPane.js - toolkit/components/torconnect/TorConnectParent.sys.mjs - toolkit/modules/TorAndroidIntegration.sys.mjs - toolkit/modules/TorConnect.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== browser/components/torpreferences/content/connectionPane.js ===================================== @@ -2550,12 +2550,10 @@ const gConnectionPane = (function () { "torPreferences-quickstart-toggle" ); this._enableQuickstartCheckbox.addEventListener("command", () => { - TorSettings.changeSettings({ - quickstart: { enabled: this._enableQuickstartCheckbox.checked }, - }); + TorConnect.quickstart = this._enableQuickstartCheckbox.checked; }); - this._enableQuickstartCheckbox.checked = TorSettings.quickstart.enabled; - Services.obs.addObserver(this, TorSettingsTopics.SettingsChanged); + this._enableQuickstartCheckbox.checked = TorConnect.quickstart; + Services.obs.addObserver(this, TorConnectTopics.QuickstartChange); // Location { @@ -2667,7 +2665,7 @@ const gConnectionPane = (function () { gBridgeSettings.init(); gNetworkStatus.init(); - TorSettings.initializedPromise.then(() => this._populateXUL()); + this._populateXUL(); const onUnload = () => { window.removeEventListener("unload", onUnload); @@ -2681,7 +2679,7 @@ const gConnectionPane = (function () { gNetworkStatus.uninit(); // unregister our observer topics - Services.obs.removeObserver(this, TorSettingsTopics.SettingsChanged); + Services.obs.removeObserver(this, TorConnectTopics.QuickstartChange); Services.obs.removeObserver(this, TorConnectTopics.StageChange); }, @@ -2696,12 +2694,8 @@ const gConnectionPane = (function () { observe(subject, topic) { switch (topic) { - // triggered when a TorSettings param has changed - case TorSettingsTopics.SettingsChanged: { - if (subject.wrappedJSObject.changes.includes("quickstart.enabled")) { - this._enableQuickstartCheckbox.checked = - TorSettings.quickstart.enabled; - } + case TorConnectTopics.QuickstartChange: { + this._enableQuickstartCheckbox.checked = TorConnect.quickstart; break; } // triggered when tor connect state changes and we may @@ -2713,7 +2707,10 @@ const gConnectionPane = (function () { } }, - onAdvancedSettings() { + async onAdvancedSettings() { + // Ensure TorSettings is complete before loading the dialog, which reads + // from TorSettings. + await TorSettings.initializedPromise; gSubDialog.open( "chrome://browser/content/torpreferences/connectionSettingsDialog.xhtml", { features: "resizable=yes" } ===================================== toolkit/components/torconnect/TorConnectParent.sys.mjs ===================================== @@ -5,10 +5,6 @@ import { TorConnect, TorConnectTopics, } from "resource://gre/modules/TorConnect.sys.mjs"; -import { - TorSettings, - TorSettingsTopics, -} from "resource://gre/modules/TorSettings.sys.mjs"; const lazy = {}; @@ -47,15 +43,10 @@ export class TorConnectParent extends JSWindowActorParent { case TorConnectTopics.BootstrapProgress: self.sendAsyncMessage("torconnect:bootstrap-progress", obj); break; - case TorSettingsTopics.SettingsChanged: - if (!obj.changes.includes("quickstart.enabled")) { - break; - } - // eslint-disable-next-lined no-fallthrough - case TorSettingsTopics.Ready: + case TorConnectTopics.QuickstartChange: self.sendAsyncMessage( - "torconnect:quickstart-changed", - TorSettings.quickstart.enabled + "torconnect:quickstart-change", + TorConnect.quickstart ); break; } @@ -70,10 +61,9 @@ export class TorConnectParent extends JSWindowActorParent { this.torConnectObserver, TorConnectTopics.BootstrapProgress ); - Services.obs.addObserver(this.torConnectObserver, TorSettingsTopics.Ready); Services.obs.addObserver( this.torConnectObserver, - TorSettingsTopics.SettingsChanged + TorConnectTopics.QuickstartChange ); } @@ -88,11 +78,7 @@ export class TorConnectParent extends JSWindowActorParent { ); Services.obs.removeObserver( this.torConnectObserver, - TorSettingsTopics.Ready - ); - Services.obs.removeObserver( - this.torConnectObserver, - TorSettingsTopics.SettingsChanged + TorConnectTopics.QuickstartChange ); } @@ -104,7 +90,7 @@ export class TorConnectParent extends JSWindowActorParent { // If there are multiple home pages, just load the first one. return Promise.resolve(TorConnect.fixupURIs(lazy.HomePage.get())[0]); case "torconnect:set-quickstart": - TorSettings.changeSettings({ quickstart: { enabled: message.data } }); + TorConnect.quickstart = message.data; break; case "torconnect:open-tor-preferences": this.browsingContext.top.embedderElement.ownerGlobal.openPreferences( @@ -133,31 +119,16 @@ export class TorConnectParent extends JSWindowActorParent { case "torconnect:cancel-bootstrapping": TorConnect.cancelBootstrapping(); break; - case "torconnect:get-init-args": { + case "torconnect:get-init-args": // Called on AboutTorConnect.init(), pass down all state data it needs // to init. - - let quickstartEnabled = false; - - // Workaround for a race condition, but we should fix it asap. - // about:torconnect is loaded before TorSettings is actually initialized. - // The getter might throw and the page not loaded correctly as a result. - // Silence any warning for now, but we should really fix it. - // See also tor-browser#41921. - try { - quickstartEnabled = TorSettings.quickstart.enabled; - } catch (e) { - // Do not throw. - } - return { TorStrings, Direction: Services.locale.isAppLocaleRTL ? "rtl" : "ltr", CountryNames: TorConnect.countryNames, stage: TorConnect.stage, - quickstartEnabled, + quickstartEnabled: TorConnect.quickstart, }; - } case "torconnect:get-country-codes": return TorConnect.getCountryCodes(); } ===================================== toolkit/modules/TorAndroidIntegration.sys.mjs ===================================== @@ -79,6 +79,23 @@ class TorAndroidIntegrationImpl { } } + /** + * Combine the current TorSettings settings with the TorConnect settings. + * + * @returns {object} The TorSettings in an object, which also has a + * `quickstart.enabled` property. + */ + // This is added for backward compatibility with TorSettings.getSettings prior + // to tor-browser#41921, when it used to control the quickstart setting. + // TODO: Have android separate out the request for TorConnect.quickstart. In + // principle, this would allow android tor connect UI to be loaded before + // TorSettings has initialized (the SettingsReady signal), similar to desktop. + // See tor-browser#43408. + #getAllSettings() { + const settings = lazy.TorSettings.getSettings(); + settings.quickstart = { enabled: lazy.TorConnect.quickstart }; + } + observe(subj, topic) { switch (topic) { // TODO: Replace with StageChange. @@ -120,7 +137,7 @@ class TorAndroidIntegrationImpl { case lazy.TorSettingsTopics.Ready: lazy.EventDispatcher.instance.sendRequest({ type: EmittedEvents.settingsReady, - settings: lazy.TorSettings.getSettings(), + settings: this.#getAllSettings(), }); break; case lazy.TorSettingsTopics.SettingsChanged: @@ -129,7 +146,20 @@ class TorAndroidIntegrationImpl { lazy.EventDispatcher.instance.sendRequest({ type: EmittedEvents.settingsChanged, changes: subj.wrappedJSObject.changes ?? [], - settings: lazy.TorSettings.getSettings(), + settings: this.#getAllSettings(), + }); + break; + case lazy.TorConnectTopics.QuickstartChange: + // We also include the TorSettings, and a `changes` Array similar to + // SettingsChanged signal. This is for backward compatibility with + // TorSettings.getSettings prior to tor-browser#41921, when it used to + // control the quickstart setting. + // TODO: Have android separate out the request for TorConnect.quickstart. + // See tor-browser#43408. + lazy.EventDispatcher.instance.sendRequest({ + type: EmittedEvents.settingsChanged, + changes: ["quickstart.enabled"], + settings: this.#getAllSettings(), }); break; } @@ -140,9 +170,19 @@ class TorAndroidIntegrationImpl { try { switch (event) { case ListenedEvents.settingsGet: - callback?.onSuccess(lazy.TorSettings.getSettings()); + callback?.onSuccess(this.#getAllSettings()); return; case ListenedEvents.settingsSet: + // TODO: Set quickstart via a separate event. See tor-browser#43408. + // NOTE: Currently this may trigger GeckoView:Tor:SettingsChanged + // twice: once for quickstart.enabled, and again for the other + // settings. + if ( + "quickstart" in data.settings && + "enabled" in data.settings.quickstart + ) { + lazy.TorConnect.quickstart = data.settings.quickstart.enabled; + } // TODO: Handle setting throw? This can throw if data.settings is // incorrectly formatted, but more like it can throw when the settings // fail to be passed onto the TorProvider. tor-browser#43405. ===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -22,6 +22,7 @@ const TorConnectPrefs = Object.freeze({ log_level: "torbrowser.bootstrap.log_level", /* prompt_at_startup now controls whether the quickstart can trigger. */ prompt_at_startup: "extensions.torlauncher.prompt_at_startup", + quickstart: "torbrowser.settings.quickstart.enabled", }); export const TorConnectState = Object.freeze({ @@ -80,6 +81,7 @@ export const TorConnectTopics = Object.freeze({ StageChange: "torconnect:stage-change", // TODO: Remove torconnect:state-change when pages have switched to stage. StateChange: "torconnect:state-change", + QuickstartChange: "torconnect:quickstart-change", BootstrapProgress: "torconnect:bootstrap-progress", BootstrapComplete: "torconnect:bootstrap-complete", // TODO: Remove torconnect:error when pages have switched to stage. @@ -1066,8 +1068,12 @@ export const TorConnect = { // shouldQuickStart would be `false`. // NOTE: At this point, _requestedStage should still be `null`. this._setStage(TorConnectStage.Start); - if (this.shouldQuickStart) { - // Quickstart + if ( + // Quickstart setting is enabled. + this.quickstart && + // And the previous bootstrap attempt must have succeeded. + !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) + ) { this.beginBootstrapping(); } }, @@ -1120,6 +1126,25 @@ export const TorConnect = { return lazy.TorLauncherUtil.shouldStartAndOwnTor; }, + /** + * Whether bootstrapping can begin immediately once Tor Browser has been + * opened. + * + * @type {boolean} + */ + get quickstart() { + return Services.prefs.getBoolPref(TorConnectPrefs.quickstart, false); + }, + + set quickstart(enabled) { + enabled = Boolean(enabled); + if (enabled === this.quickstart) { + return; + } + Services.prefs.setBoolPref(TorConnectPrefs.quickstart, enabled); + Services.obs.notifyObservers(null, TorConnectTopics.QuickstartChange); + }, + get shouldShowTorConnect() { // TorBrowser must control the daemon return ( @@ -1163,15 +1188,6 @@ export const TorConnect = { ); }, - get shouldQuickStart() { - // quickstart must be enabled - return ( - lazy.TorSettings.quickstart.enabled && - // and the previous bootstrap attempt must have succeeded - !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) - ); - }, - // TODO: Remove when all pages have switched to "stage". get state() { // There is no "Error" stage, but about:torconnect relies on receiving the ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -27,10 +27,8 @@ export const TorSettingsTopics = Object.freeze({ /* Prefs used to store settings in TorBrowser prefs */ const TorSettingsPrefs = Object.freeze({ - quickstart: { - /* bool: does tor connect automatically on launch */ - enabled: "torbrowser.settings.quickstart.enabled", - }, + // NOTE: torbrowser.settings.quickstart.enabled used to be managed by + // TorSettings but was moved to TorConnect.quickstart in tor-browser#41921. bridges: { /* bool: does tor use bridges */ enabled: "torbrowser.settings.bridges.enabled", @@ -173,9 +171,6 @@ class TorSettingsImpl { * @type {object} */ #settings = { - quickstart: { - enabled: false, - }, bridges: { /** * Whether the bridges are enabled or not. @@ -579,11 +574,6 @@ class TorSettingsImpl { #loadFromPrefs() { lazy.logger.debug("loadFromPrefs()"); - /* Quickstart */ - this.#settings.quickstart.enabled = Services.prefs.getBoolPref( - TorSettingsPrefs.quickstart.enabled, - false - ); /* Bridges */ const bridges = {}; bridges.enabled = Services.prefs.getBoolPref( @@ -691,11 +681,6 @@ class TorSettingsImpl { this.#checkIfInitialized(); - /* Quickstart */ - Services.prefs.setBoolPref( - TorSettingsPrefs.quickstart.enabled, - this.#settings.quickstart.enabled - ); /* Bridges */ Services.prefs.setBoolPref( TorSettingsPrefs.bridges.enabled, @@ -928,7 +913,6 @@ class TorSettingsImpl { * * It is possible to set all settings, or only some sections: * - * + quickstart.enabled can be set individually. * + bridges.enabled can be set individually. * + bridges.source can be set with a corresponding bridge specification for * the source (bridge_strings, lox_id, builtin_type). @@ -968,14 +952,6 @@ class TorSettingsImpl { changes.push(`${group}.${prop}`); }; - if ("quickstart" in newValues && "enabled" in newValues.quickstart) { - changeSetting( - "quickstart", - "enabled", - Boolean(newValues.quickstart.enabled) - ); - } - if ("bridges" in newValues) { if ("source" in newValues.bridges) { this.#fixupBridgeSettings(newValues.bridges); @@ -1048,11 +1024,7 @@ class TorSettingsImpl { // saved the preferences we send the new settings to TorProvider. // Some properties are unread by TorProvider. So if only these values change // there is no need to re-apply the settings. - const unreadProps = [ - "quickstart.enabled", - "bridges.builtin_type", - "bridges.lox_id", - ]; + const unreadProps = ["bridges.builtin_type", "bridges.lox_id"]; const shouldApply = changes.some(prop => !unreadProps.includes(prop)); if (shouldApply) { await this.#applySettings(true); View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/7a9894… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/7a9894… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 4 commits: fixup! TB 40933: Add tor-launcher functionality
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 1ad717fe by Henry Wilkes at 2025-01-21T14:56:40+00:00 fixup! TB 40933: Add tor-launcher functionality TB 41921: Ensure shouldStartAndOwnTor is constant per-session. TorConnect and TorSettings `enabled` properties depend on this value, and shouldn't change per session. - - - - - 8ae53510 by Henry Wilkes at 2025-01-21T14:56:41+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Tighten up TorSettings. We ensure TorSettings is enabled before certain operations. Also tidy up uninit to account for early calls (before init completes) or repeat calls. Also stop using "torbrowser.settings.enabled" preference, and read from the preferences for the first run. Also make TorSettings.initailizedPromise read-only. - - - - - a0c13700 by Henry Wilkes at 2025-01-21T14:56:41+00:00 fixup! TB 40562: Added Tor Browser preferences to 000-tor-browser.js TB 41921: Set default TorSetting setting values in 000-tor-browser.js. - - - - - 7a98943f by Henry Wilkes at 2025-01-21T15:00:22+00:00 fixup! TB 41435: Add a Tor Browser migration function TB 41921: Clear out unused "torbrowser.settings.enabled" preference. - - - - - 4 changed files: - browser/app/profile/000-tor-browser.js - browser/components/BrowserGlue.sys.mjs - toolkit/components/tor-launcher/TorLauncherUtil.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== browser/app/profile/000-tor-browser.js ===================================== @@ -61,6 +61,31 @@ pref("browser.startup.homepage", "about:tor"); // tor-browser#40701: Add new download warning pref("browser.download.showTorWarning", true); + +// Tor connection setting preferences. + +pref("torbrowser.settings.quickstart.enabled", false); +pref("torbrowser.settings.bridges.enabled", false); +// TorBridgeSource. Initially TorBridgeSource.Invalid = -1. +pref("torbrowser.settings.bridges.source", -1); +pref("torbrowser.settings.bridges.lox_id", ""); +// obfs4|meek-azure|snowflake|etc. +pref("torbrowser.settings.bridges.builtin_type", ""); +// torbrowser.settings.bridges.bridge_strings.0 +// torbrowser.settings.bridges.bridge_strings.1 +// etc hold the bridge lines. +pref("torbrowser.settings.proxy.enabled", false); +// TorProxyType. Initially TorProxyType.Invalid = -1. +pref("torbrowser.settings.proxy.type", -1); +pref("torbrowser.settings.proxy.address", ""); +pref("torbrowser.settings.proxy.port", 0); +pref("torbrowser.settings.proxy.username", ""); +pref("torbrowser.settings.proxy.password", ""); +pref("torbrowser.settings.firewall.enabled", false); +// comma-delimited list of port numbers. +pref("torbrowser.settings.firewall.allowed_ports", ""); + + // This pref specifies an ad-hoc "version" for various pref update hacks we need to do pref("extensions.torbutton.pref_fixup_version", 0); ===================================== browser/components/BrowserGlue.sys.mjs ===================================== @@ -4822,7 +4822,9 @@ BrowserGlue.prototype = { // YouTube search engines (tor-browser#41835). // Version 5: Tor Browser 14.0a5: Clear user preference for CFR settings // since we hid the UI (tor-browser#43118). - const TBB_MIGRATION_VERSION = 5; + // Version 6: Tor Browser 14.5a3: Clear preference for TorSettings that is + // no longer used (tor-browser#41921). + const TBB_MIGRATION_VERSION = 6; const MIGRATION_PREF = "torbrowser.migration.version"; // If we decide to force updating users to pass through any version @@ -4904,6 +4906,10 @@ BrowserGlue.prototype = { } } + if (currentVersion < 6) { + Services.prefs.clearUserPref("torbrowser.settings.enabled"); + } + Services.prefs.setIntPref(MIGRATION_PREF, TBB_MIGRATION_VERSION); }, ===================================== toolkit/components/tor-launcher/TorLauncherUtil.sys.mjs ===================================== @@ -17,8 +17,6 @@ const kPropBundleURI = "chrome://torbutton/locale/torlauncher.properties"; const kPropNamePrefix = "torlauncher."; const kIPCDirPrefName = "extensions.torlauncher.tmp_ipc_dir"; -let gStringBundle = null; - /** * This class allows to lookup for the paths of the various files that are * needed or can be used with the tor daemon, such as its configuration, the @@ -332,7 +330,7 @@ class TorFile { } } -export const TorLauncherUtil = Object.freeze({ +export const TorLauncherUtil = { get isAndroid() { return Services.appinfo.OS === "Android"; }, @@ -417,6 +415,8 @@ export const TorLauncherUtil = Object.freeze({ return this.showConfirm(null, s, defaultBtnLabel, cancelBtnLabel); }, + _stringBundle: null, + // Localized Strings // TODO: Switch to fluent also these ones. @@ -425,6 +425,9 @@ export const TorLauncherUtil = Object.freeze({ if (!aStringName) { return aStringName; } + if (!this._stringBundle) { + this._stringBundle = Services.strings.createBundle(kPropBundleURI); + } try { const key = kPropNamePrefix + aStringName; return this._stringBundle.GetStringFromName(key); @@ -587,7 +590,12 @@ export const TorLauncherUtil = Object.freeze({ Services.prefs.savePrefFile(null); }, - get shouldStartAndOwnTor() { + /** + * Determine the current value for whether we should start and own Tor. + * + * @returns {boolean} Whether we should start and own Tor. + */ + _getShouldStartAndOwnTor() { const kPrefStartTor = "extensions.torlauncher.start_tor"; try { const kBrowserToolboxPort = "MOZ_BROWSER_TOOLBOX_PORT"; @@ -610,6 +618,29 @@ export const TorLauncherUtil = Object.freeze({ return Services.prefs.getBoolPref(kPrefStartTor, true); }, + /** + * Cached value for shouldStartAndOwnTor, or `null` if not yet initialised. + * + * @type {boolean} + */ + _shouldStartAndOwnTor: null, + + /** + * Whether we should start and own Tor. + * + * The value should be constant per-session. + * + * @type {boolean} + */ + get shouldStartAndOwnTor() { + // Do not want this value to change within the same session, so always used + // the cached valued if it is available. + if (this._shouldStartAndOwnTor === null) { + this._shouldStartAndOwnTor = this._getShouldStartAndOwnTor(); + } + return this._shouldStartAndOwnTor; + }, + get shouldShowNetworkSettings() { try { const kEnvForceShowNetConfig = "TOR_FORCE_NET_CONFIG"; @@ -668,11 +699,4 @@ export const TorLauncherUtil = Object.freeze({ console.warn("Could not remove the IPC directory", e); } }, - - get _stringBundle() { - if (!gStringBundle) { - gStringBundle = Services.strings.createBundle(kPropBundleURI); - } - return gStringBundle; - }, -}); +}; ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -27,8 +27,6 @@ export const TorSettingsTopics = Object.freeze({ /* Prefs used to store settings in TorBrowser prefs */ const TorSettingsPrefs = Object.freeze({ - /* bool: are we pulling tor settings from the preferences */ - enabled: "torbrowser.settings.enabled", quickstart: { /* bool: does tor connect automatically on launch */ enabled: "torbrowser.settings.quickstart.enabled", @@ -245,6 +243,13 @@ class TorSettingsImpl { */ #builtinBridges = {}; + /** + * A promise that resolves once we are initialized, or throws if there was an + * initialization error. + * + * @type {Promise} + */ + #initializedPromise; /** * Resolve callback of the initializedPromise. */ @@ -261,8 +266,29 @@ class TorSettingsImpl { */ #initialized = false; + /** + * Whether uninit cleanup has been called. + * + * @type {boolean} + */ + #uninitCalled = false; + + /** + * Whether Lox was initialized. + * + * @type {boolean} + */ + #initializedLox = false; + + /** + * Whether observers were initialized. + * + * @type {boolean} + */ + #initializedObservers = false; + constructor() { - this.initializedPromise = new Promise((resolve, reject) => { + this.#initializedPromise = new Promise((resolve, reject) => { this.#initComplete = resolve; this.#initFailed = reject; }); @@ -378,12 +404,22 @@ class TorSettingsImpl { return this.#builtinBridges[pt] ?? []; } + /** + * Whether this module is enabled. + * + * @type {boolean} + */ + get enabled() { + return lazy.TorLauncherUtil.shouldStartAndOwnTor; + } + /** * Load or init our settings. */ async init() { if (this.#initialized) { lazy.logger.warn("Called init twice."); + await this.#initializedPromise; return; } try { @@ -402,6 +438,11 @@ class TorSettingsImpl { * it easier to update initializatedPromise. */ async #initInternal() { + if (!this.enabled || this.#uninitCalled) { + // Nothing to do. + return; + } + try { const req = await fetch("chrome://global/content/pt_config.json"); const config = await req.json(); @@ -419,26 +460,37 @@ class TorSettingsImpl { lazy.logger.error("Could not load the built-in PT config.", e); } + // `uninit` may have been called whilst we awaited pt_config. + if (this.#uninitCalled) { + lazy.logger.warn("unint was called before init completed."); + return; + } + // Initialize this before loading from prefs because we need Lox initialized // before any calls to Lox.getBridges(). if (!lazy.TorLauncherUtil.isAndroid) { try { + // Set as initialized before calling to ensure it is cleaned up by our + // `uninit` method. + this.#initializedLox = true; await lazy.Lox.init(); } catch (e) { lazy.logger.error("Could not initialize Lox.", e); } } - if ( - lazy.TorLauncherUtil.shouldStartAndOwnTor && - Services.prefs.getBoolPref(TorSettingsPrefs.enabled, false) - ) { - this.#loadFromPrefs(); - // We do not pass on the loaded settings to the TorProvider yet. Instead - // TorProvider will ask for these once it has initialised. + // `uninit` may have been called whilst we awaited Lox.init. + if (this.#uninitCalled) { + lazy.logger.warn("unint was called before init completed."); + return; } + this.#loadFromPrefs(); + // We do not pass on the loaded settings to the TorProvider yet. Instead + // TorProvider will ask for these once it has initialised. + Services.obs.addObserver(this, lazy.LoxTopics.UpdateBridges); + this.#initializedObservers = true; lazy.logger.info("Ready"); } @@ -447,8 +499,22 @@ class TorSettingsImpl { * Unload or uninit our settings. */ async uninit() { - Services.obs.removeObserver(this, lazy.LoxTopics.UpdateBridges); - await lazy.Lox.uninit(); + if (this.#uninitCalled) { + lazy.logger.warn("Called uninit twice"); + return; + } + + this.#uninitCalled = true; + // NOTE: We do not reset #initialized to false because we want it to remain + // in place for external callers, and we do not want `#initInternal` to be + // re-entered. + + if (this.#initializedObservers) { + Services.obs.removeObserver(this, lazy.LoxTopics.UpdateBridges); + } + if (this.#initializedLox) { + await lazy.Lox.uninit(); + } } observe(subject, topic) { @@ -473,10 +539,13 @@ class TorSettingsImpl { } /** - * Check whether the object has been successfully initialized, and throw if - * it has not. + * Check whether the module is enabled and successfully initialized, and throw + * if it is not. */ #checkIfInitialized() { + if (!this.enabled) { + throw new Error("TorSettings is not enabled"); + } if (!this.#initialized) { lazy.logger.trace("Not initialized code path."); throw new Error( @@ -494,6 +563,16 @@ class TorSettingsImpl { return this.#initialized; } + /** + * A promise that resolves once we are initialized, or throws if there was an + * initialization error. + * + * @type {Promise} + */ + get initializedPromise() { + return this.#initializedPromise; + } + /** * Load our settings from prefs. */ @@ -701,9 +780,6 @@ class TorSettingsImpl { } else { Services.prefs.clearUserPref(TorSettingsPrefs.firewall.allowed_ports); } - - // all tor settings now stored in prefs :) - Services.prefs.setBoolPref(TorSettingsPrefs.enabled, true); } /** View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/06cbcd… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/06cbcd… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 5 commits: fixup! TB 40933: Add tor-launcher functionality
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 6571ac7b by Henry Wilkes at 2025-01-21T14:15:40+00:00 fixup! TB 40933: Add tor-launcher functionality TB 41921: Move prompt_at_startup preference from TorProvider to TorConnect only. - - - - - fa81e56c by Henry Wilkes at 2025-01-21T14:15:40+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Move prompt_at_startup pref from TorProvider to TorConnect only. This simplifies the logic, where we set the preference any time we have a bootstrap error and clear it when we are bootstrapped. - - - - - 093e9483 by Henry Wilkes at 2025-01-21T14:15:41+00:00 fixup! TB 31286: Implementation of bridge, proxy, and firewall settings in about:preferences#connection TB 41921: Move flushSettings call to TorSettings. The call in connectionPane will not necessarily run when the preference tab is closed, nor will it trigger for bridges added through auto-bootstrapping. - - - - - b50c1c9c by Henry Wilkes at 2025-01-21T14:15:42+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Flush TorSettings settings every time they are saved. - - - - - 06cbcd5d by Henry Wilkes at 2025-01-21T14:15:43+00:00 fixup! TB 40933: Add tor-launcher functionality TB 41921: Add a NOTE for future development ensuring that writeSettings is not subject to any races between sequential calls. - - - - - 5 changed files: - browser/components/torpreferences/content/connectionPane.js - toolkit/components/tor-launcher/TorControlPort.sys.mjs - toolkit/components/tor-launcher/TorProvider.sys.mjs - toolkit/modules/TorConnect.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== browser/components/torpreferences/content/connectionPane.js ===================================== @@ -2545,19 +2545,6 @@ const gConnectionPane = (function () { // populate xul with strings and cache the relevant elements _populateXUL() { - // saves tor settings to disk when navigate away from about:preferences - window.addEventListener("blur", async () => { - try { - // Build a new provider each time because this might be called also - // when closing the browser (if about:preferences was open), maybe - // when the provider was already uninitialized. - const provider = await TorProviderBuilder.build(); - provider.flushSettings(); - } catch (e) { - console.warn("Could not save the tor settings.", e); - } - }); - // Quickstart this._enableQuickstartCheckbox = document.getElementById( "torPreferences-quickstart-toggle" ===================================== toolkit/components/tor-launcher/TorControlPort.sys.mjs ===================================== @@ -364,6 +364,9 @@ export class TorController { /** * The commands that need to be run or receive a response. * + * NOTE: This must be in the order with the last requested command at the end + * of the queue. + * * @type {Command[]} */ #commandQueue = []; @@ -947,6 +950,10 @@ export class TorController { * values will be automatically unrolled. */ async setConf(values) { + // NOTE: This is an async method. It must ensure that sequential calls to + // this method do not race against each other. I.e. the last call to this + // method must always be the last in #commandQueue. Otherwise a delayed + // earlier call could overwrite the configuration of a later call. const args = values .flatMap(([key, value]) => { if (value === undefined || value === null) { ===================================== toolkit/components/tor-launcher/TorProvider.sys.mjs ===================================== @@ -70,7 +70,6 @@ const Preferences = Object.freeze({ ControlHost: "extensions.torlauncher.control_host", ControlPort: "extensions.torlauncher.control_port", MaxLogEntries: "extensions.torlauncher.max_tor_log_entries", - PromptAtStartup: "extensions.torlauncher.prompt_at_startup", }); /* Config Keys used to configure tor daemon */ @@ -335,6 +334,15 @@ export class TorProvider { } logger.debug("Mapped settings object", settings, torSettings); + + // Send settings to the tor process. + // NOTE: Since everything up to this point has been non-async, the order in + // which TorProvider.writeSettings is called should match the order in which + // the configuration is passed onto setConf. In turn, TorControlPort.setConf + // should similarly ensure that the configuration reaches the tor process in + // the same order. + // In particular, we do not want a race where an earlier call to + // TorProvider.writeSettings can be delayed and override a later call. await this.#controller.setConf(Array.from(torSettings)); } @@ -962,11 +970,6 @@ export class TorProvider { if (statusObj.PROGRESS === 100) { this.#isBootstrapDone = true; - try { - Services.prefs.setBoolPref(Preferences.PromptAtStartup, false); - } catch (e) { - logger.warn(`Cannot set ${Preferences.PromptAtStartup}`, e); - } return; } @@ -988,11 +991,6 @@ export class TorProvider { * @param {object} statusObj The bootstrap status object with the error */ #notifyBootstrapError(statusObj) { - try { - Services.prefs.setBoolPref(Preferences.PromptAtStartup, true); - } catch (e) { - logger.warn(`Cannot set ${Preferences.PromptAtStartup}`, e); - } logger.error("Tor bootstrap error", statusObj); if ( ===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -16,15 +16,12 @@ ChromeUtils.defineESModuleGetters(lazy, { TorSettings: "resource://gre/modules/TorSettings.sys.mjs", }); -/* Relevant prefs used by tor-launcher */ -const TorLauncherPrefs = Object.freeze({ - prompt_at_startup: "extensions.torlauncher.prompt_at_startup", -}); - const TorConnectPrefs = Object.freeze({ censorship_level: "torbrowser.debug.censorship_level", allow_internet_test: "torbrowser.bootstrap.allow_internet_test", log_level: "torbrowser.bootstrap.log_level", + /* prompt_at_startup now controls whether the quickstart can trigger. */ + prompt_at_startup: "extensions.torlauncher.prompt_at_startup", }); export const TorConnectState = Object.freeze({ @@ -1050,7 +1047,7 @@ export const TorConnect = { lazy.logger.info("Starting again since the tor process exited"); // Treat a failure as a possibly broken configuration. // So, prevent quickstart at the next start. - Services.prefs.setBoolPref(TorLauncherPrefs.prompt_at_startup, true); + Services.prefs.setBoolPref(TorConnectPrefs.prompt_at_startup, true); this._makeStageRequest(TorConnectStage.Start, true); break; default: @@ -1171,7 +1168,7 @@ export const TorConnect = { return ( lazy.TorSettings.quickstart.enabled && // and the previous bootstrap attempt must have succeeded - !Services.prefs.getBoolPref(TorLauncherPrefs.prompt_at_startup, true) + !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) ); }, @@ -1451,6 +1448,8 @@ export const TorConnect = { this._tryAgain = false; this._potentiallyBlocked = false; this._errorDetails = null; + // Re-enable quickstart for future sessions. + Services.prefs.setBoolPref(TorConnectPrefs.prompt_at_startup, false); if (requestedStage) { lazy.logger.warn( @@ -1491,6 +1490,8 @@ export const TorConnect = { this._tryAgain = true; this._potentiallyBlocked = true; + // Disable quickstart until we have a successful bootstrap. + Services.prefs.setBoolPref(TorConnectPrefs.prompt_at_startup, true); this._signalError(error); ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -711,10 +711,15 @@ class TorSettingsImpl { * * Even though this introduces a circular depdency, it makes the API nicer for * frontend consumers. + * + * @param {boolean} flush - Whether to also flush the settings to disk. */ - async #applySettings() { + async #applySettings(flush) { const provider = await lazy.TorProviderBuilder.build(); await provider.writeSettings(); + if (flush) { + provider.flushSettings(); + } } /** @@ -974,7 +979,7 @@ class TorSettingsImpl { ]; const shouldApply = changes.some(prop => !unreadProps.includes(prop)); if (shouldApply) { - await this.#applySettings(); + await this.#applySettings(true); } } @@ -1042,7 +1047,8 @@ class TorSettingsImpl { // After checks are complete, we commit them. this.#temporaryBridgeSettings = bridgeSettings; - await this.#applySettings(); + // Do not flush the temporary bridge settings until they are saved. + await this.#applySettings(false); } /** View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/909f72… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/909f72… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 2 commits: fixup! TB 40597: Implement TorSettings module
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: b64f4aad by Henry Wilkes at 2025-01-21T11:50:20+01:00 fixup! TB 40597: Implement TorSettings module TB 41921: Make the TorSettings properties read-only outside of the TorSettings modules. Within TorSettings we make all post initialisation changes in changeSettings. This means we can simplify the class and move all the setter logic into one place. - - - - - 909f72ff by Henry Wilkes at 2025-01-21T11:50:24+01:00 fixup! TB 31286: Implementation of bridge, proxy, and firewall settings in about:preferences#connection TB 41921: Pass in the BridgeDB bridge_strings as an Array to TorSettings. - - - - - 3 changed files: - browser/components/torpreferences/content/connectionPane.js - toolkit/modules/DomainFrontedRequests.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== browser/components/torpreferences/content/connectionPane.js ===================================== @@ -2319,7 +2319,7 @@ const gBridgeSettings = { bridges: { enabled: true, source: TorBridgeSource.BridgeDB, - bridge_strings: result.bridges.join("\n"), + bridge_strings: result.bridges, }, }); } ===================================== toolkit/modules/DomainFrontedRequests.sys.mjs ===================================== @@ -130,7 +130,7 @@ class MeekTransport { TOR_PT_CLIENT_TRANSPORTS: meekTransport, }; if (lazy.TorSettings.proxy.enabled) { - envAdditions.TOR_PT_PROXY = lazy.TorSettings.proxy.uri; + envAdditions.TOR_PT_PROXY = lazy.TorSettings.proxyUri; } const opts = { ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -179,10 +179,34 @@ class TorSettingsImpl { enabled: false, }, bridges: { + /** + * Whether the bridges are enabled or not. + * + * @type {boolean} + */ enabled: false, source: TorBridgeSource.Invalid, + /** + * The lox id is used with the Lox "source", and remains set with the + * stored value when other sources are used. + * + * @type {string} + */ lox_id: "", + /** + * The built-in type to use when using the BuiltIn "source", or empty when + * using any other source. + * + * @type {string} + */ builtin_type: "", + /** + * The current bridge strings. + * + * Can only be non-empty if the "source" is not Invalid. + * + * @type {Array<string>} + */ bridge_strings: [], }, proxy: { @@ -206,15 +230,6 @@ class TorSettingsImpl { */ #temporaryBridgeSettings = null; - /** - * Accumulated errors from trying to set settings. - * - * Only added to if not null. - * - * @type {Array<Error>?} - */ - #settingErrors = null; - /** * The recommended pluggable transport. * @@ -245,13 +260,6 @@ class TorSettingsImpl { * @type {boolean} */ #initialized = false; - /** - * During some phases of the initialization, allow calling setters and - * getters without throwing errors. - * - * @type {boolean} - */ - #allowUninitialized = false; constructor() { this.initializedPromise = new Promise((resolve, reject) => { @@ -259,347 +267,57 @@ class TorSettingsImpl { this.#initFailed = reject; }); - this.#addProperties("quickstart", { - enabled: {}, - }); - this.#addProperties("bridges", { - /** - * Whether the bridges are enabled or not. - * - * @type {boolean} - */ - enabled: {}, - /** - * The current bridge source. - * - * @type {integer} - */ - source: { - transform: (val, addError) => { - if (Object.values(TorBridgeSource).includes(val)) { - return val; - } - addError(`Not a valid bridge source: "${val}"`); - return TorBridgeSource.Invalid; - }, - }, - /** - * The current bridge strings. - * - * Can only be non-empty if the "source" is not Invalid. - * - * @type {Array<string>} - */ - bridge_strings: { - transform: val => { - if (Array.isArray(val)) { - return [...val]; - } - // Split the bridge strings, discarding empty. - return splitBridgeLines(val).filter(val => val); - }, - copy: val => [...val], - equal: (val1, val2) => this.#arrayEqual(val1, val2), - }, - /** - * The built-in type to use when using the BuiltIn "source", or empty when - * using any other source. - * - * @type {string} - */ - builtin_type: { - callback: (val, addError) => { - if (!val) { - return; - } - const bridgeStrings = this.getBuiltinBridges(val); - if (bridgeStrings.length) { - this.bridges.bridge_strings = bridgeStrings; - return; - } - - addError(`No built-in ${val} bridges found`); - // Set as invalid, which will make the builtin_type "" and set the - // bridge_strings to be empty at the next #cleanupSettings. - this.bridges.source = TorBridgeSource.Invalid; - }, - }, - /** - * The lox id is used with the Lox "source", and remains set with the stored value when - * other sources are used. - * - * @type {string} - */ - lox_id: { - callback: (val, addError) => { - if (!val) { - return; - } - let bridgeStrings; - try { - bridgeStrings = lazy.Lox.getBridges(val); - } catch (error) { - addError(`No bridges for lox_id ${val}: ${error?.message}`); - // Set as invalid, which will make the builtin_type "" and set the - // bridge_strings to be empty at the next #cleanupSettings. - this.bridges.source = TorBridgeSource.Invalid; - return; - } - this.bridges.bridge_strings = bridgeStrings; - }, - }, - }); - this.#addProperties("proxy", { - enabled: {}, - type: { - transform: (val, addError) => { - if (Object.values(TorProxyType).includes(val)) { - return val; - } - addError(`Not a valid proxy type: "${val}"`); - return TorProxyType.Invalid; - }, - }, - address: {}, - port: { - transform: (val, addError) => { - if (val === 0) { - // This is a valid value that "unsets" the port. - // Keep this value without giving a warning. - // NOTE: In contrast, "0" is not valid. - return 0; - } - // Unset to 0 if invalid null is returned. - return this.#parsePort(val, false, addError) ?? 0; - }, - }, - username: {}, - password: {}, - uri: { - getter: () => { - const { type, address, port, username, password } = this.proxy; - switch (type) { - case TorProxyType.Socks4: - return `socks4a://${address}:${port}`; - case TorProxyType.Socks5: - if (username) { - return `socks5://${username}:${password}@${address}:${port}`; - } - return `socks5://${address}:${port}`; - case TorProxyType.HTTPS: - if (username) { - return `http://${username}:${password}@${address}:${port}`; - } - return `http://${address}:${port}`; - } - return null; - }, - }, - }); - this.#addProperties("firewall", { - enabled: {}, - allowed_ports: { - transform: (val, addError) => { - if (!Array.isArray(val)) { - val = val === "" ? [] : val.split(","); - } - // parse and remove duplicates - const portSet = new Set( - val.map(p => this.#parsePort(p, true, addError)) - ); - // parsePort returns null for failed parses, so remove it. - portSet.delete(null); - return [...portSet]; - }, - copy: val => [...val], - equal: (val1, val2) => this.#arrayEqual(val1, val2), - }, - }); - } - - /** - * Clean the setting values after making some changes, so that the values do - * not contradict each other. - */ - #cleanupSettings() { - this.#freezeNotifications(); - try { - if (this.bridges.source === TorBridgeSource.Invalid) { - this.bridges.enabled = false; - this.bridges.bridge_strings = []; + // Add some read-only getters for the #settings object. + // E.g. TorSetting.#settings.bridges.source is exposed publicly as + // TorSettings.bridges.source. + for (const groupname in this.#settings) { + const publicGroup = {}; + for (const name in this.#settings[groupname]) { + // Public group only has a getter for the property. + Object.defineProperty(publicGroup, name, { + get: () => { + this.#checkIfInitialized(); + return structuredClone(this.#settings[groupname][name]); + }, + set: () => { + throw new Error( + `TorSettings.${groupname}.${name} cannot be set directly` + ); + }, + }); } - if (!this.bridges.bridge_strings.length) { - this.bridges.enabled = false; - this.bridges.source = TorBridgeSource.Invalid; - } - if (this.bridges.source !== TorBridgeSource.BuiltIn) { - this.bridges.builtin_type = ""; - } - if (this.bridges.source !== TorBridgeSource.Lox) { - this.bridges.lox_id = ""; - } - if (!this.proxy.enabled) { - this.proxy.type = TorProxyType.Invalid; - this.proxy.address = ""; - this.proxy.port = 0; - this.proxy.username = ""; - this.proxy.password = ""; - } - if (!this.firewall.enabled) { - this.firewall.allowed_ports = []; - } - } finally { - this.#thawNotifications(); + // The group object itself should not be writable. + Object.preventExtensions(publicGroup); + Object.defineProperty(this, groupname, { + writable: false, + value: publicGroup, + }); } } /** - * The current number of freezes applied to the notifications. - * - * @type {integer} - */ - #freezeNotificationsCount = 0; - /** - * The queue for settings that have changed. To be broadcast in the - * notification when not frozen. - * - * @type {Set<string>} - */ - #notificationQueue = new Set(); - /** - * Send a notification if we have any queued and we are not frozen. - */ - #tryNotification() { - if (this.#freezeNotificationsCount || !this.#notificationQueue.size) { - return; - } - Services.obs.notifyObservers( - { changes: [...this.#notificationQueue] }, - TorSettingsTopics.SettingsChanged - ); - this.#notificationQueue.clear(); - } - /** - * Pause notifications for changes in setting values. This is useful if you - * need to make batch changes to settings. - * - * This should always be paired with a call to thawNotifications once - * notifications should be released. Usually you should wrap whatever - * changes you make with a `try` block and call thawNotifications in the - * `finally` block. - */ - #freezeNotifications() { - this.#freezeNotificationsCount++; - } - /** - * Release the hold on notifications so they may be sent out. - * - * Note, if some other method has also frozen the notifications, this will - * only release them once it has also called this method. - */ - #thawNotifications() { - this.#freezeNotificationsCount--; - this.#tryNotification(); - } - /** - * @typedef {object} TorSettingProperty + * The proxy URI for the current settings, or `null` if no proxy is + * configured. * - * @property {function} [getter] - A getter for the property. If this is - * given, the property cannot be set. - * @property {function} [transform] - Called in the setter for the property, - * with the new value given. Should transform the given value into the - * right type. - * @property {function} [equal] - Test whether two values for the property - * are considered equal. Otherwise uses `===`. - * @property {function} [callback] - Called whenever the property value - * changes, with the new value given. Should be used to trigger any other - * required changes for the new value. - * @property {function} [copy] - Called whenever the property is read, with - * the stored value given. Should return a copy of the value. Otherwise - * returns the stored value. + * @type {?string} */ - /** - * Add properties to the TorSettings instance, to be read or set. - * - * @param {string} groupname - The name of the setting group. The given - * settings will be accessible from the TorSettings property of the same - * name. - * @param {object.<string, TorSettingProperty>} propParams - An object that - * defines the settings to add to this group. The object property names - * will be mapped to properties of TorSettings under the given groupname - * property. Details about the setting should be described in the - * TorSettingProperty property value. - */ - #addProperties(groupname, propParams) { - // Create a new object to hold all these settings. - const group = {}; - for (const name in propParams) { - const { getter, transform, callback, copy, equal } = propParams[name]; - // Method for adding setting errors. - const addError = message => { - message = `TorSettings.${groupname}.${name}: ${message}`; - lazy.logger.error(message); - // Only add to #settingErrors if it is not null. - this.#settingErrors?.push(message); - }; - Object.defineProperty(group, name, { - get: getter - ? () => { - // Allow getting in loadFromPrefs before we are initialized. - if (!this.#allowUninitialized) { - this.#checkIfInitialized(); - } - return getter(); - } - : () => { - // Allow getting in loadFromPrefs before we are initialized. - if (!this.#allowUninitialized) { - this.#checkIfInitialized(); - } - let val = this.#settings[groupname][name]; - if (copy) { - val = copy(val); - } - // Assume string or number value. - return val; - }, - set: getter - ? undefined - : val => { - // Allow setting in loadFromPrefs before we are initialized. - if (!this.#allowUninitialized) { - this.#checkIfInitialized(); - } - const prevVal = this.#settings[groupname][name]; - this.#freezeNotifications(); - try { - if (transform) { - val = transform(val, addError); - } - const isEqual = equal ? equal(val, prevVal) : val === prevVal; - if (!isEqual) { - // Set before the callback. - this.#settings[groupname][name] = val; - this.#notificationQueue.add(`${groupname}.${name}`); - - if (callback) { - callback(val, addError); - } - } - } catch (e) { - addError(e.message); - } finally { - this.#thawNotifications(); - } - }, - }); + get proxyUri() { + const { type, address, port, username, password } = this.#settings.proxy; + switch (type) { + case TorProxyType.Socks4: + return `socks4a://${address}:${port}`; + case TorProxyType.Socks5: + if (username) { + return `socks5://${username}:${password}@${address}:${port}`; + } + return `socks5://${address}:${port}`; + case TorProxyType.HTTPS: + if (username) { + return `http://${username}:${password}@${address}:${port}`; + } + return `http://${address}:${port}`; } - // The group object itself should not be writable. - Object.preventExtensions(group); - Object.defineProperty(this, groupname, { - writable: false, - value: group, - }); + return null; } /** @@ -614,12 +332,11 @@ class TorSettingsImpl { * @param {string|integer} val - The value to parse. * @param {boolean} trim - Whether a string value can be stripped of * whitespace before parsing. - * @param {function} addError - Callback to add error messages to. * * @return {integer?} - The port number, or null if the given value was not * valid. */ - #parsePort(val, trim, addError) { + #parsePort(val, trim) { if (typeof val === "string") { if (trim) { val = val.trim(); @@ -628,13 +345,11 @@ class TorSettingsImpl { if (this.#portRegex.test(val)) { val = Number.parseInt(val, 10); } else { - addError(`Invalid port string "${val}"`); - return null; + throw new Error(`Invalid port string "${val}"`); } } if (!Number.isInteger(val) || val < 1 || val > 65535) { - addError(`Port out of range: ${val}`); - return null; + throw new Error(`Port out of range: ${val}`); } return val; } @@ -659,13 +374,8 @@ class TorSettingsImpl { * @param {string} pt The pluggable transport to return the lines for * @returns {string[]} The bridge lines in random order */ - getBuiltinBridges(pt) { - if (!this.#allowUninitialized) { - this.#checkIfInitialized(); - } - // Shuffle so that Tor Browser users do not all try the built-in bridges in - // the same order. - return arrayShuffle(this.#builtinBridges[pt] ?? []); + #getBuiltinBridges(pt) { + return this.#builtinBridges[pt] ?? []; } /** @@ -698,6 +408,13 @@ class TorSettingsImpl { lazy.logger.debug("Loaded pt_config.json", config); this.#recommendedPT = config.recommendedDefault; this.#builtinBridges = config.bridges; + for (const type in this.#builtinBridges) { + // Shuffle so that Tor Browser users do not all try the built-in bridges + // in the same order. + // Only do this once per session. In particular, we don't re-shuffle if + // changeSettings is called with the same bridges.builtin_type value. + this.#builtinBridges[type] = arrayShuffle(this.#builtinBridges[type]); + } } catch (e) { lazy.logger.error("Could not load the built-in PT config.", e); } @@ -716,18 +433,9 @@ class TorSettingsImpl { lazy.TorLauncherUtil.shouldStartAndOwnTor && Services.prefs.getBoolPref(TorSettingsPrefs.enabled, false) ) { - // Do not want notifications for initially loaded prefs. - this.#freezeNotifications(); - try { - this.#allowUninitialized = true; - this.#loadFromPrefs(); - // We do not pass on the loaded settings to the TorProvider yet. Instead - // TorProvider will ask for these once it has initialised. - } finally { - this.#allowUninitialized = false; - this.#notificationQueue.clear(); - this.#thawNotifications(); - } + this.#loadFromPrefs(); + // We do not pass on the loaded settings to the TorProvider yet. Instead + // TorProvider will ask for these once it has initialised. } Services.obs.addObserver(this, lazy.LoxTopics.UpdateBridges); @@ -746,16 +454,19 @@ class TorSettingsImpl { observe(subject, topic) { switch (topic) { case lazy.LoxTopics.UpdateBridges: - if (this.bridges.lox_id) { - // Fetch the newest bridges. - this.bridges.bridge_strings = lazy.Lox.getBridges( - this.bridges.lox_id - ); - // No need to save to prefs since bridge_strings is not stored for Lox - // source. But we do pass on the changes to TorProvider. + if ( + this.#settings.bridges.lox_id && + this.#settings.bridges.source === TorBridgeSource.Lox + ) { + // Re-trigger the call to lazy.Lox.getBridges. // FIXME: This can compete with TorConnect to reach TorProvider. // tor-browser#42316 - this.#applySettings(); + this.changeSettings({ + bridges: { + source: TorBridgeSource.Lox, + lox_id: this.#settings.bridges.lox_id, + }, + }); } break; } @@ -790,23 +501,24 @@ class TorSettingsImpl { lazy.logger.debug("loadFromPrefs()"); /* Quickstart */ - this.quickstart.enabled = Services.prefs.getBoolPref( + this.#settings.quickstart.enabled = Services.prefs.getBoolPref( TorSettingsPrefs.quickstart.enabled, false ); /* Bridges */ - this.bridges.enabled = Services.prefs.getBoolPref( + const bridges = {}; + bridges.enabled = Services.prefs.getBoolPref( TorSettingsPrefs.bridges.enabled, false ); - this.bridges.source = Services.prefs.getIntPref( + bridges.source = Services.prefs.getIntPref( TorSettingsPrefs.bridges.source, TorBridgeSource.Invalid ); - switch (this.bridges.source) { + switch (bridges.source) { case TorBridgeSource.BridgeDB: case TorBridgeSource.UserProvided: - this.bridges.bridge_strings = Services.prefs + bridges.bridge_strings = Services.prefs .getBranch(TorSettingsPrefs.bridges.bridge_strings) .getChildList("") .map(pref => @@ -817,60 +529,79 @@ class TorSettingsImpl { break; case TorBridgeSource.BuiltIn: // bridge_strings is set via builtin_type. - this.bridges.builtin_type = Services.prefs.getStringPref( + bridges.builtin_type = Services.prefs.getStringPref( TorSettingsPrefs.bridges.builtin_type, "" ); break; case TorBridgeSource.Lox: // bridge_strings is set via lox id. - this.bridges.lox_id = Services.prefs.getStringPref( + bridges.lox_id = Services.prefs.getStringPref( TorSettingsPrefs.bridges.lox_id, "" ); break; } + try { + this.#fixupBridgeSettings(bridges); + this.#settings.bridges = bridges; + } catch (error) { + lazy.logger.error("Loaded bridge preferences failed", error); + // Keep the default #settings.bridges. + } + /* Proxy */ - this.proxy.enabled = Services.prefs.getBoolPref( + const proxy = {}; + proxy.enabled = Services.prefs.getBoolPref( TorSettingsPrefs.proxy.enabled, false ); - if (this.proxy.enabled) { - this.proxy.type = Services.prefs.getIntPref( + if (proxy.enabled) { + proxy.type = Services.prefs.getIntPref( TorSettingsPrefs.proxy.type, TorProxyType.Invalid ); - this.proxy.address = Services.prefs.getStringPref( + proxy.address = Services.prefs.getStringPref( TorSettingsPrefs.proxy.address, "" ); - this.proxy.port = Services.prefs.getIntPref( - TorSettingsPrefs.proxy.port, - 0 - ); - this.proxy.username = Services.prefs.getStringPref( + proxy.port = Services.prefs.getIntPref(TorSettingsPrefs.proxy.port, 0); + proxy.username = Services.prefs.getStringPref( TorSettingsPrefs.proxy.username, "" ); - this.proxy.password = Services.prefs.getStringPref( + proxy.password = Services.prefs.getStringPref( TorSettingsPrefs.proxy.password, "" ); } + try { + this.#fixupProxySettings(proxy); + this.#settings.proxy = proxy; + } catch (error) { + lazy.logger.error("Loaded proxy preferences failed", error); + // Keep the default #settings.proxy. + } /* Firewall */ - this.firewall.enabled = Services.prefs.getBoolPref( + const firewall = {}; + firewall.enabled = Services.prefs.getBoolPref( TorSettingsPrefs.firewall.enabled, false ); - if (this.firewall.enabled) { - this.firewall.allowed_ports = Services.prefs.getStringPref( + if (firewall.enabled) { + firewall.allowed_ports = Services.prefs.getStringPref( TorSettingsPrefs.firewall.allowed_ports, "" ); } - - this.#cleanupSettings(); + try { + this.#fixupFirewallSettings(firewall); + this.#settings.firewall = firewall; + } catch (error) { + lazy.logger.error("Loaded firewall preferences failed", error); + // Keep the default #settings.firewall. + } } /** @@ -880,29 +611,28 @@ class TorSettingsImpl { lazy.logger.debug("saveToPrefs()"); this.#checkIfInitialized(); - this.#cleanupSettings(); /* Quickstart */ Services.prefs.setBoolPref( TorSettingsPrefs.quickstart.enabled, - this.quickstart.enabled + this.#settings.quickstart.enabled ); /* Bridges */ Services.prefs.setBoolPref( TorSettingsPrefs.bridges.enabled, - this.bridges.enabled + this.#settings.bridges.enabled ); Services.prefs.setIntPref( TorSettingsPrefs.bridges.source, - this.bridges.source + this.#settings.bridges.source ); Services.prefs.setStringPref( TorSettingsPrefs.bridges.builtin_type, - this.bridges.builtin_type + this.#settings.bridges.builtin_type ); Services.prefs.setStringPref( TorSettingsPrefs.bridges.lox_id, - this.bridges.lox_id + this.#settings.bridges.lox_id ); // erase existing bridge strings const bridgeBranchPrefs = Services.prefs @@ -915,10 +645,10 @@ class TorSettingsImpl { }); // write new ones if ( - this.bridges.source !== TorBridgeSource.Lox && - this.bridges.source !== TorBridgeSource.BuiltIn + this.#settings.bridges.source !== TorBridgeSource.Lox && + this.#settings.bridges.source !== TorBridgeSource.BuiltIn ) { - this.bridges.bridge_strings.forEach((string, index) => { + this.#settings.bridges.bridge_strings.forEach((string, index) => { Services.prefs.setStringPref( `${TorSettingsPrefs.bridges.bridge_strings}.${index}`, string @@ -928,22 +658,28 @@ class TorSettingsImpl { /* Proxy */ Services.prefs.setBoolPref( TorSettingsPrefs.proxy.enabled, - this.proxy.enabled + this.#settings.proxy.enabled ); - if (this.proxy.enabled) { - Services.prefs.setIntPref(TorSettingsPrefs.proxy.type, this.proxy.type); + if (this.#settings.proxy.enabled) { + Services.prefs.setIntPref( + TorSettingsPrefs.proxy.type, + this.#settings.proxy.type + ); Services.prefs.setStringPref( TorSettingsPrefs.proxy.address, - this.proxy.address + this.#settings.proxy.address + ); + Services.prefs.setIntPref( + TorSettingsPrefs.proxy.port, + this.#settings.proxy.port ); - Services.prefs.setIntPref(TorSettingsPrefs.proxy.port, this.proxy.port); Services.prefs.setStringPref( TorSettingsPrefs.proxy.username, - this.proxy.username + this.#settings.proxy.username ); Services.prefs.setStringPref( TorSettingsPrefs.proxy.password, - this.proxy.password + this.#settings.proxy.password ); } else { Services.prefs.clearUserPref(TorSettingsPrefs.proxy.type); @@ -955,12 +691,12 @@ class TorSettingsImpl { /* Firewall */ Services.prefs.setBoolPref( TorSettingsPrefs.firewall.enabled, - this.firewall.enabled + this.#settings.firewall.enabled ); - if (this.firewall.enabled) { + if (this.#settings.firewall.enabled) { Services.prefs.setStringPref( TorSettingsPrefs.firewall.allowed_ports, - this.firewall.allowed_ports.join(",") + this.#settings.firewall.allowed_ports.join(",") ); } else { Services.prefs.clearUserPref(TorSettingsPrefs.firewall.allowed_ports); @@ -977,11 +713,135 @@ class TorSettingsImpl { * frontend consumers. */ async #applySettings() { - this.#checkIfInitialized(); const provider = await lazy.TorProviderBuilder.build(); await provider.writeSettings(); } + /** + * Fixup the given bridges settings to fill in details, establish the correct + * types and clean up. + * + * May throw if there is an error in the given values. + * + * @param {Object} bridges - The bridges settings to fix up. + */ + #fixupBridgeSettings(bridges) { + if (!Object.values(TorBridgeSource).includes(bridges.source)) { + throw new Error(`Not a valid bridge source: "${bridges.source}"`); + } + + if ("enabled" in bridges) { + bridges.enabled = Boolean(bridges.enabled); + } + + // Set bridge_strings + switch (bridges.source) { + case TorBridgeSource.UserProvided: + case TorBridgeSource.BridgeDB: + // Only accept an Array for UserProvided and BridgeDB bridge_strings. + break; + case TorBridgeSource.BuiltIn: + bridges.builtin_type = String(bridges.builtin_type); + bridges.bridge_strings = this.#getBuiltinBridges(bridges.builtin_type); + break; + case TorBridgeSource.Lox: + bridges.lox_id = String(bridges.lox_id); + bridges.bridge_strings = lazy.Lox.getBridges(bridges.lox_id); + break; + case TorBridgeSource.Invalid: + bridges.bridge_strings = []; + break; + } + + if ( + !Array.isArray(bridges.bridge_strings) || + bridges.bridge_strings.some(str => typeof str !== "string") + ) { + throw new Error("bridge_strings should be an Array of strings"); + } + + if ( + bridges.source !== TorBridgeSource.Invalid && + !bridges.bridge_strings?.length + ) { + throw new Error( + `Missing bridge_strings for bridge source ${bridges.source}` + ); + } + + if (bridges.source !== TorBridgeSource.BuiltIn) { + bridges.builtin_type = ""; + } + if (bridges.source !== TorBridgeSource.Lox) { + bridges.lox_id = ""; + } + + if (bridges.source === TorBridgeSource.Invalid) { + bridges.enabled = false; + } + } + + /** + * Fixup the given proxy settings to fill in details, establish the correct + * types and clean up. + * + * May throw if there is an error in the given values. + * + * @param {Object} proxy - The proxy settings to fix up. + */ + #fixupProxySettings(proxy) { + proxy.enabled = Boolean(proxy.enabled); + if (!proxy.enabled) { + proxy.type = TorProxyType.Invalid; + proxy.address = ""; + proxy.port = 0; + proxy.username = ""; + proxy.password = ""; + return; + } + + if (!Object.values(TorProxyType).includes(proxy.type)) { + throw new Error(`Invalid proxy type: ${proxy.type}`); + } + proxy.port = this.#parsePort(proxy.port, false); + proxy.address = String(proxy.address); + proxy.username = String(proxy.username); + proxy.password = String(proxy.password); + } + + /** + * Fixup the given firewall settings to fill in details, establish the correct + * types and clean up. + * + * May throw if there is an error in the given values. + * + * @param {Object} firewall - The proxy settings to fix up. + */ + #fixupFirewallSettings(firewall) { + firewall.enabled = Boolean(firewall.enabled); + if (!firewall.enabled) { + firewall.allowed_ports = []; + return; + } + + let allowed_ports = firewall.allowed_ports; + if (!Array.isArray(allowed_ports)) { + allowed_ports = allowed_ports === "" ? [] : allowed_ports.split(","); + } + // parse and remove duplicates + const portSet = new Set(); + + for (const port of allowed_ports) { + try { + portSet.add(this.#parsePort(port, true)); + } catch (e) { + // Do not throw for individual ports. + lazy.logger.error(`Failed to parse the port ${port}. Ignoring.`, e); + } + } + firewall.allowed_ports = [...portSet]; + } + /** * Change the Tor settings in use. * @@ -994,101 +854,128 @@ class TorSettingsImpl { * + proxy settings can be set as a group. * + firewall settings can be set a group. * - * @param {object} settings - The settings object to set. + * @param {object} newValues - The new setting values, a subset of the + * complete settings that should be changed. */ - async changeSettings(settings) { - lazy.logger.debug("changeSettings()", settings); + async changeSettings(newValues) { + lazy.logger.debug("changeSettings()", newValues); this.#checkIfInitialized(); - const backup = this.getSettings(); - const backupNotifications = [...this.#notificationQueue]; - // Start collecting errors. - this.#settingErrors = []; - - // Hold off on lots of notifications until all settings are changed. - this.#freezeNotifications(); - try { - if ("quickstart" in settings && "enabled" in settings.quickstart) { - this.quickstart.enabled = !!settings.quickstart.enabled; + // Make a structured clone since we change the object and may adopt some of + // the Array values. + newValues = structuredClone(newValues); + + const completeSettings = structuredClone(this.#settings); + const changes = []; + + /** + * Change the given setting to a new value. Does nothing if the new value + * equals the old one, otherwise the change will be recorded in `changes`. + * + * @param {string} group - The group name for the property. + * @param {string} prop - The property name within the group. + * @param {any} value - The value to set. + * @param [Function?] equal - A method to test equality between the old and + * new value. Otherwise uses `===` to check equality. + */ + const changeSetting = (group, prop, value, equal = null) => { + const currentValue = this.#settings[group][prop]; + if (equal ? equal(currentValue, value) : currentValue === value) { + return; } + completeSettings[group][prop] = value; + changes.push(`${group}.${prop}`); + }; - if ("bridges" in settings) { - if ("enabled" in settings.bridges) { - this.bridges.enabled = !!settings.bridges.enabled; - } - if ("source" in settings.bridges) { - this.bridges.source = settings.bridges.source; - switch (settings.bridges.source) { - case TorBridgeSource.BridgeDB: - case TorBridgeSource.UserProvided: - this.bridges.bridge_strings = settings.bridges.bridge_strings; - break; - case TorBridgeSource.BuiltIn: - this.bridges.builtin_type = settings.bridges.builtin_type; - break; - case TorBridgeSource.Lox: - this.bridges.lox_id = settings.bridges.lox_id; - break; - case TorBridgeSource.Invalid: - break; - case undefined: - break; - } - } - } + if ("quickstart" in newValues && "enabled" in newValues.quickstart) { + changeSetting( + "quickstart", + "enabled", + Boolean(newValues.quickstart.enabled) + ); + } - if ("proxy" in settings) { - // proxy settings have to be set as a group. - this.proxy.enabled = !!settings.proxy.enabled; - if (this.proxy.enabled) { - this.proxy.type = settings.proxy.type; - this.proxy.address = settings.proxy.address; - this.proxy.port = settings.proxy.port; - this.proxy.username = settings.proxy.username; - this.proxy.password = settings.proxy.password; + if ("bridges" in newValues) { + if ("source" in newValues.bridges) { + this.#fixupBridgeSettings(newValues.bridges); + changeSetting("bridges", "source", newValues.bridges.source); + changeSetting( + "bridges", + "bridge_strings", + newValues.bridges.bridge_strings, + this.#arrayEqual + ); + changeSetting("bridges", "lox_id", newValues.bridges.lox_id); + changeSetting( + "bridges", + "builtin_type", + newValues.bridges.builtin_type + ); + } else if ("enabled" in newValues.bridges) { + // Don't need to fixup all the settings, just need to ensure that the + // enabled value is compatible with the current source. + newValues.bridges.enabled = Boolean(newValues.bridges.enabled); + if ( + newValues.bridges.enabled && + completeSettings.bridges.source === TorBridgeSource.Invalid + ) { + throw new Error("Cannot enable bridges without a bridge source."); } } - - if ("firewall" in settings) { - // firewall settings have to be set as a group. - this.firewall.enabled = !!settings.firewall.enabled; - if (this.firewall.enabled) { - this.firewall.allowed_ports = settings.firewall.allowed_ports; - } + if ("enabled" in newValues.bridges) { + changeSetting("bridges", "enabled", newValues.bridges.enabled); } + } - this.#cleanupSettings(); + if ("proxy" in newValues) { + // proxy settings have to be set as a group. + this.#fixupProxySettings(newValues.proxy); + changeSetting("proxy", "enabled", Boolean(newValues.proxy.enabled)); + changeSetting("proxy", "type", newValues.proxy.type); + changeSetting("proxy", "address", newValues.proxy.address); + changeSetting("proxy", "port", newValues.proxy.port); + changeSetting("proxy", "username", newValues.proxy.username); + changeSetting("proxy", "password", newValues.proxy.password); + } - if (this.#settingErrors.length) { - throw Error(this.#settingErrors.join("; ")); - } - this.#saveToPrefs(); - } catch (ex) { - // Restore the old settings without any new notifications generated from - // the above code. - // NOTE: Since the code that changes #settings is not async, it should not - // be possible for some other call to TorSettings to change anything - // whilst we are in this context (other than lower down in this call - // stack), so it is safe to discard all changes to settings and - // notifications. - this.#settings = backup; - this.#notificationQueue.clear(); - for (const notification of backupNotifications) { - this.#notificationQueue.add(notification); - } + if ("firewall" in newValues) { + // firewall settings have to be set as a group. + this.#fixupFirewallSettings(newValues.firewall); + changeSetting("firewall", "enabled", Boolean(newValues.firewall.enabled)); + changeSetting( + "firewall", + "allowed_ports", + newValues.firewall.allowed_ports, + this.#arrayEqual + ); + } + + // No errors so far, so save and commit. + this.#settings = completeSettings; + this.#saveToPrefs(); - throw ex; - } finally { - this.#thawNotifications(); - // Stop collecting errors. - this.#settingErrors = null; + if (changes.length) { + Services.obs.notifyObservers( + { changes }, + TorSettingsTopics.SettingsChanged + ); } - lazy.logger.debug("setSettings result", this.#settings); + lazy.logger.debug("setSettings result", this.#settings, changes); // After we have sent out the notifications for the changed settings and // saved the preferences we send the new settings to TorProvider. - await this.#applySettings(); + // Some properties are unread by TorProvider. So if only these values change + // there is no need to re-apply the settings. + const unreadProps = [ + "quickstart.enabled", + "bridges.builtin_type", + "bridges.lox_id", + ]; + const shouldApply = changes.some(prop => !unreadProps.includes(prop)); + if (shouldApply) { + await this.#applySettings(); + } } /** @@ -1147,29 +1034,11 @@ class TorSettingsImpl { const bridgeSettings = { enabled: true, source: bridges.source, + builtin_type: String(bridges.builtin_type), + bridge_strings: structuredClone(bridges.bridge_strings), }; - if (bridges.source === TorBridgeSource.BuiltIn) { - if (!bridges.builtin_type) { - throw Error("Missing a built-in type"); - } - bridgeSettings.builtin_type = String(bridges.builtin_type); - const bridgeStrings = this.getBuiltinBridges(bridgeSettings.builtin_type); - if (!bridgeStrings.length) { - throw new Error(`No builtin bridges for type ${bridges.builtin_type}`); - } - bridgeSettings.bridge_strings = bridgeStrings; - } else { - // BridgeDB. - if (!bridges.bridge_strings?.length) { - throw new Error("Missing bridges strings"); - } - // TODO: Can we safely verify the format of the bridge addresses sent from - // Moat? - bridgeSettings.bridge_strings = Array.from(bridges.bridge_strings, item => - String(item) - ); - } + this.#fixupBridgeSettings(bridgeSettings); // After checks are complete, we commit them. this.#temporaryBridgeSettings = bridgeSettings; View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/3dd180… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/3dd180… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 5 commits: fixup! TB 40597: Implement TorSettings module
by Pier Angelo Vendrame (＠pierov) 21 Jan '25

21 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: bd7602fe by Henry Wilkes at 2025-01-20T18:25:01+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Only use TorSettings.changeSettings to change the Tor settings and apply them in one method. The granular methods have been made private. - - - - - e43836ca by Henry Wilkes at 2025-01-20T18:26:14+00:00 fixup! TB 42247: Android helpers for the TorProvider TB 41921: Only use TorSettings.changeSettings to change the Tor settings and apply them in one method. We drop redundant arguments and unused methods from android integration. - - - - - 6864f8ab by Henry Wilkes at 2025-01-20T18:26:15+00:00 fixup! TB 41878: [android] Add standalone Tor Bootstrap TB 41921: Drop redundant arguments to getTorIntegration().setSettings. All calls to this method passed true for the last two arguments. - - - - - d6536e87 by Henry Wilkes at 2025-01-20T18:26:16+00:00 fixup! TB 31286: Implementation of bridge, proxy, and firewall settings in about:preferences#connection TB 41921: Only use TorSettings.changeSettings to change the Tor settings and apply them in one method. - - - - - 3dd18040 by Henry Wilkes at 2025-01-20T18:26:16+00:00 fixup! TB 27476: Implement about:torconnect captive portal within Tor Browser TB 41921: Only use TorSettings.changeSettings to change the Tor settings and apply them in one method. - - - - - 7 changed files: - browser/components/torpreferences/content/connectionPane.js - browser/components/torpreferences/content/connectionSettingsDialog.js - mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tor/TorControllerGV.kt - mobile/android/geckoview/src/main/java/org/mozilla/geckoview/TorIntegrationAndroid.java - toolkit/components/torconnect/TorConnectParent.sys.mjs - toolkit/modules/TorAndroidIntegration.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== browser/components/torpreferences/content/connectionPane.js ===================================== @@ -96,33 +96,6 @@ const Lox = { }; */ -/** - * Make changes to TorSettings and save them. - * - * Bulk changes will be frozen together. - * - * @param {Function} changes - Method to apply changes to TorSettings. - */ -async function setTorSettings(changes) { - if (!TorSettings.initialized) { - log.warning("Ignoring changes to uninitialized TorSettings"); - return; - } - TorSettings.freezeNotifications(); - try { - changes(); - // This will trigger TorSettings.#cleanupSettings() - TorSettings.saveToPrefs(); - try { - await TorSettings.applySettings(); - } catch (e) { - console.error("Failed to apply Tor settings", e); - } - } finally { - TorSettings.thawNotifications(); - } -} - /** * Get the ID/fingerprint of the bridge used in the most recent Tor circuit. * @@ -757,6 +730,7 @@ const gBridgeGrid = { }); removeItem.addEventListener("click", () => { const bridgeLine = row.bridgeLine; + const source = TorSettings.bridges.source; const strings = TorSettings.bridges.bridge_strings; const index = strings.indexOf(bridgeLine); if (index === -1) { @@ -764,8 +738,8 @@ const gBridgeGrid = { } strings.splice(index, 1); - setTorSettings(() => { - TorSettings.bridges.bridge_strings = strings; + TorSettings.changeSettings({ + bridges: { source, bridge_strings: strings }, }); }); }, @@ -1829,8 +1803,8 @@ const gBridgeSettings = { if (!this._haveBridges) { return; } - setTorSettings(() => { - TorSettings.bridges.enabled = this._toggleButton.pressed; + TorSettings.changeSettings({ + bridges: { enabled: this._toggleButton.pressed }, }); }); @@ -2215,10 +2189,10 @@ const gBridgeSettings = { return; } - setTorSettings(() => { + TorSettings.changeSettings({ // This should always have the side effect of disabling bridges as // well. - TorSettings.bridges.source = TorBridgeSource.Invalid; + bridges: { source: TorBridgeSource.Invalid }, }); }); @@ -2319,10 +2293,12 @@ const gBridgeSettings = { if (!result.type) { return null; } - return setTorSettings(() => { - TorSettings.bridges.enabled = true; - TorSettings.bridges.source = TorBridgeSource.BuiltIn; - TorSettings.bridges.builtin_type = result.type; + return TorSettings.changeSettings({ + bridges: { + enabled: true, + source: TorBridgeSource.BuiltIn, + builtin_type: result.type, + }, }); } ); @@ -2339,10 +2315,12 @@ const gBridgeSettings = { if (!result.bridges?.length) { return null; } - return setTorSettings(() => { - TorSettings.bridges.enabled = true; - TorSettings.bridges.source = TorBridgeSource.BridgeDB; - TorSettings.bridges.bridge_strings = result.bridges.join("\n"); + return TorSettings.changeSettings({ + bridges: { + enabled: true, + source: TorBridgeSource.BridgeDB, + bridge_strings: result.bridges.join("\n"), + }, }); } ); @@ -2363,16 +2341,15 @@ const gBridgeSettings = { if (!loxId && !result.addresses?.length) { return null; } - return setTorSettings(() => { - TorSettings.bridges.enabled = true; - if (loxId) { - TorSettings.bridges.source = TorBridgeSource.Lox; - TorSettings.bridges.lox_id = loxId; - } else { - TorSettings.bridges.source = TorBridgeSource.UserProvided; - TorSettings.bridges.bridge_strings = result.addresses; - } - }); + const bridges = { enabled: true }; + if (loxId) { + bridges.source = TorBridgeSource.Lox; + bridges.lox_id = loxId; + } else { + bridges.source = TorBridgeSource.UserProvided; + bridges.bridge_strings = result.addresses; + } + return TorSettings.changeSettings({ bridges }); } ); }, @@ -2586,9 +2563,9 @@ const gConnectionPane = (function () { "torPreferences-quickstart-toggle" ); this._enableQuickstartCheckbox.addEventListener("command", () => { - const checked = this._enableQuickstartCheckbox.checked; - TorSettings.quickstart.enabled = checked; - TorSettings.saveToPrefs().applySettings(); + TorSettings.changeSettings({ + quickstart: { enabled: this._enableQuickstartCheckbox.checked }, + }); }); this._enableQuickstartCheckbox.checked = TorSettings.quickstart.enabled; Services.obs.addObserver(this, TorSettingsTopics.SettingsChanged); ===================================== browser/components/torpreferences/content/connectionSettingsDialog.js ===================================== @@ -270,33 +270,34 @@ const gConnectionSettingsDialog = { const port = this._proxyPortTextbox.value; const username = this._proxyUsernameTextbox.value; const password = this._proxyPasswordTextbox.value; + const settings = { proxy: {}, firewall: {} }; switch (type) { case TorProxyType.Invalid: - TorSettings.proxy.enabled = false; + settings.proxy.enabled = false; break; case TorProxyType.Socks4: - TorSettings.proxy.enabled = true; - TorSettings.proxy.type = type; - TorSettings.proxy.address = address; - TorSettings.proxy.port = port; - TorSettings.proxy.username = ""; - TorSettings.proxy.password = ""; + settings.proxy.enabled = true; + settings.proxy.type = type; + settings.proxy.address = address; + settings.proxy.port = port; + settings.proxy.username = ""; + settings.proxy.password = ""; break; case TorProxyType.Socks5: - TorSettings.proxy.enabled = true; - TorSettings.proxy.type = type; - TorSettings.proxy.address = address; - TorSettings.proxy.port = port; - TorSettings.proxy.username = username; - TorSettings.proxy.password = password; + settings.proxy.enabled = true; + settings.proxy.type = type; + settings.proxy.address = address; + settings.proxy.port = port; + settings.proxy.username = username; + settings.proxy.password = password; break; case TorProxyType.HTTPS: - TorSettings.proxy.enabled = true; - TorSettings.proxy.type = type; - TorSettings.proxy.address = address; - TorSettings.proxy.port = port; - TorSettings.proxy.username = username; - TorSettings.proxy.password = password; + settings.proxy.enabled = true; + settings.proxy.type = type; + settings.proxy.address = address; + settings.proxy.port = port; + settings.proxy.username = username; + settings.proxy.password = password; break; } @@ -304,16 +305,15 @@ const gConnectionSettingsDialog = { ? this._allowedPortsTextbox.value : ""; if (portListString) { - TorSettings.firewall.enabled = true; - TorSettings.firewall.allowed_ports = portListString; + settings.firewall.enabled = true; + settings.firewall.allowed_ports = portListString; } else { - TorSettings.firewall.enabled = false; + settings.firewall.enabled = false; } - TorSettings.saveToPrefs(); // FIXME: What if this fails? Should we prevent the dialog to close and show // an error? - TorSettings.applySettings(); + TorSettings.changeSettings(settings); }, }; ===================================== mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tor/TorControllerGV.kt ===================================== @@ -80,7 +80,7 @@ class TorControllerGV( set(value) { getTorSettings()?.let { it.quickstart = value - getTorIntegration().setSettings(it, true, true) + getTorIntegration().setSettings(it) } } @@ -110,7 +110,7 @@ class TorControllerGV( getTorSettings()?.let { if (!value || it.bridgesSource != BridgeSource.Invalid) { it.bridgesEnabled = value - getTorIntegration().setSettings(it, true, true) + getTorIntegration().setSettings(it) } } } @@ -150,7 +150,7 @@ class TorControllerGV( } it.bridgesBuiltinType = bbt } - getTorIntegration().setSettings(it, true, true) + getTorIntegration().setSettings(it) } } @@ -175,7 +175,7 @@ class TorControllerGV( val userProvidedLines: Array<String> = value?.split("\n")?.filter { it.length > 4 }?.toTypedArray() ?: arrayOf<String>() it.bridgesSource = BridgeSource.UserProvided it.bridgeBridgeStrings = userProvidedLines - getTorIntegration().setSettings(it, true, true) + getTorIntegration().setSettings(it) } } ===================================== mobile/android/geckoview/src/main/java/org/mozilla/geckoview/TorIntegrationAndroid.java ===================================== @@ -48,8 +48,6 @@ public class TorIntegrationAndroid implements BundleEventListener { // Events we emit private static final String EVENT_SETTINGS_GET = "GeckoView:Tor:SettingsGet"; private static final String EVENT_SETTINGS_SET = "GeckoView:Tor:SettingsSet"; - private static final String EVENT_SETTINGS_APPLY = "GeckoView:Tor:SettingsApply"; - private static final String EVENT_SETTINGS_SAVE = "GeckoView:Tor:SettingsSave"; private static final String EVENT_BOOTSTRAP_BEGIN = "GeckoView:Tor:BootstrapBegin"; private static final String EVENT_BOOTSTRAP_BEGIN_AUTO = "GeckoView:Tor:BootstrapBeginAuto"; private static final String EVENT_BOOTSTRAP_CANCEL = "GeckoView:Tor:BootstrapCancel"; @@ -194,7 +192,7 @@ public class TorIntegrationAndroid implements BundleEventListener { protected void onPostExecute(TorSettings torSettings) { mSettings = torSettings; if (TorLegacyAndroidSettings.unmigrated()) { - setSettings(mSettings, true, true); + setSettings(mSettings); TorLegacyAndroidSettings.setMigrated(); } } @@ -657,10 +655,10 @@ public class TorIntegrationAndroid implements BundleEventListener { return mSettings; } - public void setSettings(final TorSettings settings, boolean save, boolean apply) { + public void setSettings(final TorSettings settings) { mSettings = settings; - emitSetSettings(settings, save, apply) + emitSetSettings(settings) .then( new GeckoResult.OnValueListener<Void, Void>() { public GeckoResult<Void> onValue(Void v) { @@ -677,22 +675,12 @@ public class TorIntegrationAndroid implements BundleEventListener { } private @NonNull GeckoResult<Void> emitSetSettings( - final TorSettings settings, boolean save, boolean apply) { - GeckoBundle bundle = new GeckoBundle(3); - bundle.putBoolean("save", save); - bundle.putBoolean("apply", apply); + final TorSettings settings) { + GeckoBundle bundle = new GeckoBundle(1); bundle.putBundle("settings", settings.asGeckoBundle()); return EventDispatcher.getInstance().queryVoid(EVENT_SETTINGS_SET, bundle); } - public @NonNull GeckoResult<Void> applySettings() { - return EventDispatcher.getInstance().queryVoid(EVENT_SETTINGS_APPLY); - } - - public @NonNull GeckoResult<Void> saveSettings() { - return EventDispatcher.getInstance().queryVoid(EVENT_SETTINGS_SAVE); - } - public @NonNull GeckoResult<Void> beginBootstrap() { return EventDispatcher.getInstance().queryVoid(EVENT_BOOTSTRAP_BEGIN); } ===================================== toolkit/components/torconnect/TorConnectParent.sys.mjs ===================================== @@ -104,8 +104,7 @@ export class TorConnectParent extends JSWindowActorParent { // If there are multiple home pages, just load the first one. return Promise.resolve(TorConnect.fixupURIs(lazy.HomePage.get())[0]); case "torconnect:set-quickstart": - TorSettings.quickstart.enabled = message.data; - TorSettings.saveToPrefs().applySettings(); + TorSettings.changeSettings({ quickstart: { enabled: message.data } }); break; case "torconnect:open-tor-preferences": this.browsingContext.top.embedderElement.ownerGlobal.openPreferences( ===================================== toolkit/modules/TorAndroidIntegration.sys.mjs ===================================== @@ -36,8 +36,6 @@ const ListenedEvents = Object.freeze({ settingsGet: "GeckoView:Tor:SettingsGet", // The data is passed directly to TorSettings. settingsSet: "GeckoView:Tor:SettingsSet", - settingsApply: "GeckoView:Tor:SettingsApply", - settingsSave: "GeckoView:Tor:SettingsSave", bootstrapBegin: "GeckoView:Tor:BootstrapBegin", // Optionally takes a countryCode, as data.countryCode. bootstrapBeginAuto: "GeckoView:Tor:BootstrapBeginAuto", @@ -145,20 +143,10 @@ class TorAndroidIntegrationImpl { callback?.onSuccess(lazy.TorSettings.getSettings()); return; case ListenedEvents.settingsSet: - // This does not throw, so we do not have any way to report the error! - lazy.TorSettings.setSettings(data.settings); - if (data.save) { - lazy.TorSettings.saveToPrefs(); - } - if (data.apply) { - await lazy.TorSettings.applySettings(); - } - break; - case ListenedEvents.settingsApply: - await lazy.TorSettings.applySettings(); - break; - case ListenedEvents.settingsSave: - await lazy.TorSettings.saveToPrefs(); + // TODO: Handle setting throw? This can throw if data.settings is + // incorrectly formatted, but more like it can throw when the settings + // fail to be passed onto the TorProvider. tor-browser#43405. + await lazy.TorSettings.changeSettings(data.settings); break; case ListenedEvents.bootstrapBegin: lazy.TorConnect.beginBootstrapping(); ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -422,7 +422,7 @@ class TorSettingsImpl { * not contradict each other. */ #cleanupSettings() { - this.freezeNotifications(); + this.#freezeNotifications(); try { if (this.bridges.source === TorBridgeSource.Invalid) { this.bridges.enabled = false; @@ -449,7 +449,7 @@ class TorSettingsImpl { this.firewall.allowed_ports = []; } } finally { - this.thawNotifications(); + this.#thawNotifications(); } } @@ -488,7 +488,7 @@ class TorSettingsImpl { * changes you make with a `try` block and call thawNotifications in the * `finally` block. */ - freezeNotifications() { + #freezeNotifications() { this.#freezeNotificationsCount++; } /** @@ -497,7 +497,7 @@ class TorSettingsImpl { * Note, if some other method has also frozen the notifications, this will * only release them once it has also called this method. */ - thawNotifications() { + #thawNotifications() { this.#freezeNotificationsCount--; this.#tryNotification(); } @@ -571,7 +571,7 @@ class TorSettingsImpl { this.#checkIfInitialized(); } const prevVal = this.#settings[groupname][name]; - this.freezeNotifications(); + this.#freezeNotifications(); try { if (transform) { val = transform(val, addError); @@ -589,7 +589,7 @@ class TorSettingsImpl { } catch (e) { addError(e.message); } finally { - this.thawNotifications(); + this.#thawNotifications(); } }, }); @@ -717,7 +717,7 @@ class TorSettingsImpl { Services.prefs.getBoolPref(TorSettingsPrefs.enabled, false) ) { // Do not want notifications for initially loaded prefs. - this.freezeNotifications(); + this.#freezeNotifications(); try { this.#allowUninitialized = true; this.#loadFromPrefs(); @@ -726,7 +726,7 @@ class TorSettingsImpl { } finally { this.#allowUninitialized = false; this.#notificationQueue.clear(); - this.thawNotifications(); + this.#thawNotifications(); } } @@ -755,7 +755,7 @@ class TorSettingsImpl { // source. But we do pass on the changes to TorProvider. // FIXME: This can compete with TorConnect to reach TorProvider. // tor-browser#42316 - this.applySettings(); + this.#applySettings(); } break; } @@ -876,7 +876,7 @@ class TorSettingsImpl { /** * Save our settings to prefs. */ - saveToPrefs() { + #saveToPrefs() { lazy.logger.debug("saveToPrefs()"); this.#checkIfInitialized(); @@ -968,8 +968,6 @@ class TorSettingsImpl { // all tor settings now stored in prefs :) Services.prefs.setBoolPref(TorSettingsPrefs.enabled, true); - - return this; } /** @@ -978,23 +976,28 @@ class TorSettingsImpl { * Even though this introduces a circular depdency, it makes the API nicer for * frontend consumers. */ - async applySettings() { + async #applySettings() { this.#checkIfInitialized(); const provider = await lazy.TorProviderBuilder.build(); await provider.writeSettings(); } /** - * Set blocks of settings at once from an object. + * Change the Tor settings in use. + * + * It is possible to set all settings, or only some sections: * - * It is possible to set all settings, or only some sections (e.g., only - * bridges), but if a key is present, its settings must make sense (e.g., if - * bridges are enabled, a valid source must be provided). + * + quickstart.enabled can be set individually. + * + bridges.enabled can be set individually. + * + bridges.source can be set with a corresponding bridge specification for + * the source (bridge_strings, lox_id, builtin_type). + * + proxy settings can be set as a group. + * + firewall settings can be set a group. * - * @param {object} settings The settings object to set + * @param {object} settings - The settings object to set. */ - setSettings(settings) { - lazy.logger.debug("setSettings()"); + async changeSettings(settings) { + lazy.logger.debug("changeSettings()", settings); this.#checkIfInitialized(); const backup = this.getSettings(); @@ -1003,38 +1006,39 @@ class TorSettingsImpl { this.#settingErrors = []; // Hold off on lots of notifications until all settings are changed. - this.freezeNotifications(); + this.#freezeNotifications(); try { - if ("quickstart" in settings) { + if ("quickstart" in settings && "enabled" in settings.quickstart) { this.quickstart.enabled = !!settings.quickstart.enabled; } if ("bridges" in settings) { - this.bridges.enabled = !!settings.bridges.enabled; - // Currently, disabling bridges in the UI does not remove the lines, - // because we call only the `enabled` setter. - // So, if the bridge source is undefined but bridges are disabled, - // do not force Invalid. Instead, keep the current source. - if (this.bridges.enabled || settings.bridges.source !== undefined) { - this.bridges.source = settings.bridges.source; + if ("enabled" in settings.bridges) { + this.bridges.enabled = !!settings.bridges.enabled; } - switch (settings.bridges.source) { - case TorBridgeSource.BridgeDB: - case TorBridgeSource.UserProvided: - this.bridges.bridge_strings = settings.bridges.bridge_strings; - break; - case TorBridgeSource.BuiltIn: - this.bridges.builtin_type = settings.bridges.builtin_type; - break; - case TorBridgeSource.Lox: - this.bridges.lox_id = settings.bridges.lox_id; - break; - case TorBridgeSource.Invalid: - break; + if ("source" in settings.bridges) { + this.bridges.source = settings.bridges.source; + switch (settings.bridges.source) { + case TorBridgeSource.BridgeDB: + case TorBridgeSource.UserProvided: + this.bridges.bridge_strings = settings.bridges.bridge_strings; + break; + case TorBridgeSource.BuiltIn: + this.bridges.builtin_type = settings.bridges.builtin_type; + break; + case TorBridgeSource.Lox: + this.bridges.lox_id = settings.bridges.lox_id; + break; + case TorBridgeSource.Invalid: + break; + case undefined: + break; + } } } if ("proxy" in settings) { + // proxy settings have to be set as a group. this.proxy.enabled = !!settings.proxy.enabled; if (this.proxy.enabled) { this.proxy.type = settings.proxy.type; @@ -1046,6 +1050,7 @@ class TorSettingsImpl { } if ("firewall" in settings) { + // firewall settings have to be set as a group. this.firewall.enabled = !!settings.firewall.enabled; if (this.firewall.enabled) { this.firewall.allowed_ports = settings.firewall.allowed_ports; @@ -1057,27 +1062,33 @@ class TorSettingsImpl { if (this.#settingErrors.length) { throw Error(this.#settingErrors.join("; ")); } + this.#saveToPrefs(); } catch (ex) { // Restore the old settings without any new notifications generated from // the above code. - // NOTE: Since this code is not async, it should not be possible for - // some other call to TorSettings to change anything whilst we are - // in this context (other than lower down in this call stack), so it is - // safe to discard all changes to settings and notifications. + // NOTE: Since the code that changes #settings is not async, it should not + // be possible for some other call to TorSettings to change anything + // whilst we are in this context (other than lower down in this call + // stack), so it is safe to discard all changes to settings and + // notifications. this.#settings = backup; this.#notificationQueue.clear(); for (const notification of backupNotifications) { this.#notificationQueue.add(notification); } - lazy.logger.error("setSettings failed", ex); + throw ex; } finally { - this.thawNotifications(); + this.#thawNotifications(); // Stop collecting errors. this.#settingErrors = null; } lazy.logger.debug("setSettings result", this.#settings); + + // After we have sent out the notifications for the changed settings and + // saved the preferences we send the new settings to TorProvider. + await this.#applySettings(); } /** @@ -1162,7 +1173,7 @@ class TorSettingsImpl { // After checks are complete, we commit them. this.#temporaryBridgeSettings = bridgeSettings; - await this.applySettings(); + await this.#applySettings(); } /** @@ -1174,10 +1185,9 @@ class TorSettingsImpl { lazy.logger.warn("No temporary bridges to save"); return; } - this.setSettings({ bridges: this.#temporaryBridgeSettings }); + const bridgeSettings = this.#temporaryBridgeSettings; this.#temporaryBridgeSettings = null; - this.saveToPrefs(); - await this.applySettings(); + await this.changeSettings({ bridges: bridgeSettings }); } /** @@ -1190,7 +1200,7 @@ class TorSettingsImpl { return; } this.#temporaryBridgeSettings = null; - await this.applySettings(); + await this.#applySettings(); } } View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/6ea6bd… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/6ea6bd… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser-build][maint-13.5] Bug 41324: Improve build signing ergonomics
by ma1 (＠ma1) 21 Jan '25

21 Jan '25

ma1 pushed to branch maint-13.5 at The Tor Project / Applications / tor-browser-build Commits: 311a9672 by hackademix at 2025-01-21T09:48:21+01:00 Bug 41324: Improve build signing ergonomics - - - - - 2 changed files: - tools/signing/do-all-signing - + tools/signing/set-config.passwords Changes: ===================================== tools/signing/do-all-signing ===================================== @@ -3,29 +3,59 @@ set -e script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) source "$script_dir/functions" source "$script_dir/set-config.update-responses" - NON_INTERACTIVE=1 steps_dir="$signed_version_dir.steps" test -d "$steps_dir" || mkdir -p "$steps_dir" -test -f "$steps_dir/linux-signer-rcodesign-sign.done" || +function get_sekrit { + echo "$SEKRITS" | grep -A1 "$1:" | tail -n1 +} + +[ -f "$script_dir/set-config.passwords" ] && . "$script_dir/set-config.passwords" 2>/dev/null + +if [[ $1 = "-p" ]]; then + shift + passwords_gpg_file="$1" + shift +fi + +is_project torbrowser && nssdb=torbrowser-nssdb7 +is_project mullvadbrowser && nssdb=mullvadbrowser-nssdb1 + +if [ -f "$passwords_gpg_file" ]; then + echo "Reading passwords from $passwords_gpg_file" + SEKRITS=$(gpg --decrypt "$passwords_gpg_file") + RCODESIGN_PW=$(get_sekrit 'rcodesign') + NSSPASS=$(get_sekrit "$nssdb (mar signing)") + KSPASS=$(get_sekrit "android apk ($tbb_version_type)") + YUBIPASS=$(get_sekrit "windows authenticode") + GPG_PASS=$(get_sekrit "gpg") +else + echo "Rather than entering all the password manually, you may want to provide a gpg-encrypted file either on the command line (-p <filepath>) or in set-config.passwords." +fi + +test -f "$steps_dir/linux-signer-rcodesign-sign.done" || [ -n "$RCODESIGN_PW" ] || read -sp "Enter rcodesign passphrase for key-1: " RCODESIGN_PW echo -is_project torbrowser && nssdb=torbrowser-nssdb7 -is_project mullvadbrowser && nssdb=mullvadbrowser-nssdb-1 -test -f "$steps_dir/linux-signer-signmars.done" || + +test -f "$steps_dir/linux-signer-signmars.done" || [ -n "$NSSPASS" ] || read -sp "Enter $nssdb (mar signing) passphrase: " NSSPASS echo -test -f "$steps_dir/linux-signer-authenticode-signing.done" || +if is_project torbrowser; then + test -f "$steps_dir/linux-signer-sign-android-apks.done" || [ -n "$KSPASS" ] || + read -sp "Enter android apk signing password ($tbb_version_type): " KSPASS + echo +fi +test -f "$steps_dir/linux-signer-authenticode-signing.done" || [ -n "$YUBIPASS" ] || read -sp "Enter windows authenticode passphrase: " YUBIPASS echo -test -f "$steps_dir/linux-signer-gpg-sign.done" || +test -f "$steps_dir/linux-signer-gpg-sign.done" || [ -n "$GPG_PASS" ] || read -sp "Enter gpg passphrase: " GPG_PASS echo function set-time-on-signing-machine { - local current_time=$(date -u) + local current_time=$(date -u -Iseconds) ssh "$ssh_host_linux_signer" sudo /usr/bin/date -s "'$current_time'" } @@ -169,6 +199,10 @@ function do_step { echo "$(date -Iseconds) - Finished step: $1" } +function is_legacy { + [[ "$tbb_version" = 13.* ]] +} + export SIGNING_PROJECTNAME do_step set-time-on-signing-machine @@ -185,6 +219,10 @@ do_step sync-scripts-to-linux-signer do_step sync-before-linux-signer-signmars do_step linux-signer-signmars do_step sync-after-signmars +is_project torbrowser && ! is_legacy && \ + do_step linux-signer-sign-android-apks +is_project torbrowser && ! is_legacy && \ + do_step sync-after-sign-android-apks do_step linux-signer-authenticode-signing do_step sync-after-authenticode-signing do_step authenticode-timestamping @@ -197,6 +235,6 @@ do_step download-unsigned-sha256sums-gpg-signatures-from-people-tpo do_step sync-local-to-staticiforme do_step sync-scripts-to-staticiforme do_step staticiforme-prepare-cdn-dist-upload -is_project mullvadbrowser && \ +! is_legacy && do_step upload-update_responses-to-staticiforme do_step finished-signing-clean-linux-signer ===================================== tools/signing/set-config.passwords ===================================== @@ -0,0 +1,2 @@ +# Path to a gpg-encrypted cache of passwords not to be asked on each run +passwords_gpg_file=~/.tor-browser-signing/tor-browser-passwords.txt.gpg View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/3… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/3… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser-build][maint-14.0] Bug 41324: Improve build signing ergonomics
by ma1 (＠ma1) 21 Jan '25

21 Jan '25

ma1 pushed to branch maint-14.0 at The Tor Project / Applications / tor-browser-build Commits: 27887a68 by hackademix at 2025-01-21T09:36:51+01:00 Bug 41324: Improve build signing ergonomics - - - - - 2 changed files: - tools/signing/do-all-signing - + tools/signing/set-config.passwords Changes: ===================================== tools/signing/do-all-signing ===================================== @@ -3,34 +3,59 @@ set -e script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) source "$script_dir/functions" source "$script_dir/set-config.update-responses" - NON_INTERACTIVE=1 steps_dir="$signed_version_dir.steps" test -d "$steps_dir" || mkdir -p "$steps_dir" -test -f "$steps_dir/linux-signer-rcodesign-sign.done" || +function get_sekrit { + echo "$SEKRITS" | grep -A1 "$1:" | tail -n1 +} + +[ -f "$script_dir/set-config.passwords" ] && . "$script_dir/set-config.passwords" 2>/dev/null + +if [[ $1 = "-p" ]]; then + shift + passwords_gpg_file="$1" + shift +fi + +is_project torbrowser && nssdb=torbrowser-nssdb7 +is_project mullvadbrowser && nssdb=mullvadbrowser-nssdb1 + +if [ -f "$passwords_gpg_file" ]; then + echo "Reading passwords from $passwords_gpg_file" + SEKRITS=$(gpg --decrypt "$passwords_gpg_file") + RCODESIGN_PW=$(get_sekrit 'rcodesign') + NSSPASS=$(get_sekrit "$nssdb (mar signing)") + KSPASS=$(get_sekrit "android apk ($tbb_version_type)") + YUBIPASS=$(get_sekrit "windows authenticode") + GPG_PASS=$(get_sekrit "gpg") +else + echo "Rather than entering all the password manually, you may want to provide a gpg-encrypted file either on the command line (-p <filepath>) or in set-config.passwords." +fi + +test -f "$steps_dir/linux-signer-rcodesign-sign.done" || [ -n "$RCODESIGN_PW" ] || read -sp "Enter rcodesign passphrase for key-1: " RCODESIGN_PW echo -is_project torbrowser && nssdb=torbrowser-nssdb7 -is_project mullvadbrowser && nssdb=mullvadbrowser-nssdb-1 -test -f "$steps_dir/linux-signer-signmars.done" || + +test -f "$steps_dir/linux-signer-signmars.done" || [ -n "$NSSPASS" ] || read -sp "Enter $nssdb (mar signing) passphrase: " NSSPASS echo if is_project torbrowser; then - test -f "$steps_dir/linux-signer-sign-android-apks.done" || + test -f "$steps_dir/linux-signer-sign-android-apks.done" || [ -n "$KSPASS" ] || read -sp "Enter android apk signing password ($tbb_version_type): " KSPASS echo fi -test -f "$steps_dir/linux-signer-authenticode-signing.done" || +test -f "$steps_dir/linux-signer-authenticode-signing.done" || [ -n "$YUBIPASS" ] || read -sp "Enter windows authenticode passphrase: " YUBIPASS echo -test -f "$steps_dir/linux-signer-gpg-sign.done" || +test -f "$steps_dir/linux-signer-gpg-sign.done" || [ -n "$GPG_PASS" ] || read -sp "Enter gpg passphrase: " GPG_PASS echo function set-time-on-signing-machine { - local current_time=$(date -u) + local current_time=$(date -u -Iseconds) ssh "$ssh_host_linux_signer" sudo /usr/bin/date -s "'$current_time'" } @@ -178,6 +203,10 @@ function do_step { echo "$(date -Iseconds) - Finished step: $1" } +function is_legacy { + [[ "$tbb_version" = 13.* ]] +} + export SIGNING_PROJECTNAME do_step set-time-on-signing-machine @@ -195,9 +224,9 @@ do_step sync-scripts-to-linux-signer do_step sync-before-linux-signer-signmars do_step linux-signer-signmars do_step sync-after-signmars -is_project torbrowser && \ +is_project torbrowser && ! is_legacy && \ do_step linux-signer-sign-android-apks -is_project torbrowser && \ +is_project torbrowser && ! is_legacy && \ do_step sync-after-sign-android-apks do_step linux-signer-authenticode-signing do_step sync-after-authenticode-signing @@ -211,5 +240,6 @@ do_step download-unsigned-sha256sums-gpg-signatures-from-people-tpo do_step sync-local-to-staticiforme do_step sync-scripts-to-staticiforme do_step staticiforme-prepare-cdn-dist-upload -do_step upload-update_responses-to-staticiforme +! is_legacy && + do_step upload-update_responses-to-staticiforme do_step finished-signing-clean-linux-signer ===================================== tools/signing/set-config.passwords ===================================== @@ -0,0 +1,2 @@ +# Path to a gpg-encrypted cache of passwords not to be asked on each run +passwords_gpg_file=~/.tor-browser-signing/tor-browser-passwords.txt.gpg View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/2… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/2… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser-build][main] Bug 41324: Improve build signing ergonomics
by ma1 (＠ma1) 20 Jan '25

20 Jan '25

ma1 pushed to branch main at The Tor Project / Applications / tor-browser-build Commits: 3af0e5b3 by hackademix at 2025-01-20T18:16:36+01:00 Bug 41324: Improve build signing ergonomics - - - - - 7 changed files: - .gitlab/issue_templates/Release Prep - Mullvad Browser Alpha.md - .gitlab/issue_templates/Release Prep - Mullvad Browser Stable.md - .gitlab/issue_templates/Release Prep - Tor Browser Alpha.md - .gitlab/issue_templates/Release Prep - Tor Browser Legacy.md - .gitlab/issue_templates/Release Prep - Tor Browser Stable.md - tools/signing/do-all-signing - + tools/signing/set-config.passwords Changes: ===================================== .gitlab/issue_templates/Release Prep - Mullvad Browser Alpha.md ===================================== @@ -208,7 +208,7 @@ Mullvad Browser Alpha (and Nightly) are on the `main` branch ``` - **Subject** ``` - New build: Mullvad Browser ${MULLVAD_BROWSER_VERION} (signed) + New build: Mullvad Browser ${MULLVAD_BROWSER_VERSION} (signed) ``` - **Body** ``` @@ -219,6 +219,8 @@ Mullvad Browser Alpha (and Nightly) are on the `main` branch - signed builds: https://dist.torproject.org/mullvadbrowser/${MULLVAD_BROWSER_VERSION} - update_response hashes: ${MULLVAD_UPDATE_RESPONSES_HASH} + * https://gitlab.torproject.org/tpo/applications/mullvad-browser-update-respo… + changelog: # paste changelog as quote here ... ===================================== .gitlab/issue_templates/Release Prep - Mullvad Browser Stable.md ===================================== @@ -208,7 +208,7 @@ Mullvad Browser Stable is on the `maint-${MULLVAD_BROWSER_MAJOR}.${MULLVAD_BROWS ``` - **Subject** ``` - New build: Mullvad Browser ${MULLVAD_BROWSER_VERION} (signed) + New build: Mullvad Browser ${MULLVAD_BROWSER_VERSION} (signed) ``` - **Body** ``` @@ -217,7 +217,9 @@ Mullvad Browser Stable is on the `maint-${MULLVAD_BROWSER_MAJOR}.${MULLVAD_BROWS Branch+Tags have been pushed to Mullvad's GitHub repo. - signed builds: https://dist.torproject.org/mullvadbrowser/${MULLVAD_BROWSER_VERSION} - - update_response hashes: ${MULLVAD_UPDATE_RESPONSES_HASH} + - update_response hashes: ${MULLVAD_UPDATE_RESPONSES_HASH}* + + * https://gitlab.torproject.org/tpo/applications/mullvad-browser-update-respo… changelog: # paste changelog as quote here ===================================== .gitlab/issue_templates/Release Prep - Tor Browser Alpha.md ===================================== @@ -223,9 +223,9 @@ Tor Browser Alpha (and Nightly) are on the `main` branch ```bash # Point OSSLSIGNCODE to your osslsigncode binary -pushd tor-browser-build/${channel}/signed/$TORBROWSER_VERSION +pushd tor-browser-build/torbrowser/${channel}/signed/$TORBROWSER_VERSION OSSLSIGNCODE=/path/to/osslsigncode -../../../tools/authenticode_check.sh +../../../../tools/authenticode_check.sh popd ``` @@ -234,14 +234,15 @@ popd <summary>Check whether the MAR files got properly signed</summary> ```bash -# Point NSSDB to your nssdb containing the mar signing certificate +# Point NSS_DB_DIR to your nssdb dir containing the mar signing certificate +# (check tools/marsigning_check.sh source code for details) # Point SIGNMAR to your signmar binary # Point LD_LIBRARY_PATH to your mar-tools directory -pushd tor-browser-build/${channel}/signed/$TORBROWSER_VERSION +pushd tor-browser-build/torbrowser/${channel}/signed/$TORBROWSER_VERSION NSSDB=/path/to/nssdb SIGNMAR=/path/to/mar-tools/signmar LD_LIBRARY_PATH=/path/to/mar-tools/ -../../../tools/marsigning_check.sh +../../../../tools/marsigning_check.sh popd ``` ===================================== .gitlab/issue_templates/Release Prep - Tor Browser Legacy.md ===================================== @@ -203,9 +203,9 @@ Tor Browser Legacy is on the `maint-13.5` branch ```bash # Point OSSLSIGNCODE to your osslsigncode binary -pushd tor-browser-build/${channel}/signed/$TORBROWSER_VERSION +pushd tor-browser-build/torbrowser/${channel}/signed/$TORBROWSER_VERSION OSSLSIGNCODE=/path/to/osslsigncode -../../../tools/authenticode_check.sh +../../../../tools/authenticode_check.sh popd ``` @@ -217,11 +217,11 @@ popd # Point NSSDB to your nssdb containing the mar signing certificate # Point SIGNMAR to your signmar binary # Point LD_LIBRARY_PATH to your mar-tools directory -pushd tor-browser-build/${channel}/signed/$TORBROWSER_VERSION +pushd tor-browser-build/torbrowser/${channel}/signed/$TORBROWSER_VERSION NSSDB=/path/to/nssdb SIGNMAR=/path/to/mar-tools/signmar LD_LIBRARY_PATH=/path/to/mar-tools/ -../../../tools/marsigning_check.sh +../../../../tools/marsigning_check.sh popd ``` ===================================== .gitlab/issue_templates/Release Prep - Tor Browser Stable.md ===================================== @@ -228,9 +228,9 @@ Tor Browser Stable is on the `maint-${TOR_BROWSER_MAJOR}.${TOR_BROWSER_MINOR}` b ```bash # Point OSSLSIGNCODE to your osslsigncode binary -pushd tor-browser-build/${channel}/signed/$TORBROWSER_VERSION +pushd tor-browser-build/torbrowser/${channel}/signed/$TORBROWSER_VERSION OSSLSIGNCODE=/path/to/osslsigncode -../../../tools/authenticode_check.sh +../../../../tools/authenticode_check.sh popd ``` @@ -242,11 +242,11 @@ popd # Point NSSDB to your nssdb containing the mar signing certificate # Point SIGNMAR to your signmar binary # Point LD_LIBRARY_PATH to your mar-tools directory -pushd tor-browser-build/${channel}/signed/$TORBROWSER_VERSION +pushd tor-browser-build/torbrowser/${channel}/signed/$TORBROWSER_VERSION NSSDB=/path/to/nssdb SIGNMAR=/path/to/mar-tools/signmar LD_LIBRARY_PATH=/path/to/mar-tools/ -../../../tools/marsigning_check.sh +../../../../tools/marsigning_check.sh popd ``` ===================================== tools/signing/do-all-signing ===================================== @@ -3,34 +3,59 @@ set -e script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) source "$script_dir/functions" source "$script_dir/set-config.update-responses" - NON_INTERACTIVE=1 steps_dir="$signed_version_dir.steps" test -d "$steps_dir" || mkdir -p "$steps_dir" -test -f "$steps_dir/linux-signer-rcodesign-sign.done" || +function get_sekrit { + echo "$SEKRITS" | grep -A1 "$1:" | tail -n1 +} + +[ -f "$script_dir/set-config.passwords" ] && . "$script_dir/set-config.passwords" 2>/dev/null + +if [[ $1 = "-p" ]]; then + shift + passwords_gpg_file="$1" + shift +fi + +is_project torbrowser && nssdb=torbrowser-nssdb7 +is_project mullvadbrowser && nssdb=mullvadbrowser-nssdb1 + +if [ -f "$passwords_gpg_file" ]; then + echo "Reading passwords from $passwords_gpg_file" + SEKRITS=$(gpg --decrypt "$passwords_gpg_file") + RCODESIGN_PW=$(get_sekrit 'rcodesign') + NSSPASS=$(get_sekrit "$nssdb (mar signing)") + KSPASS=$(get_sekrit "android apk ($tbb_version_type)") + YUBIPASS=$(get_sekrit "windows authenticode") + GPG_PASS=$(get_sekrit "gpg") +else + echo "Rather than entering all the password manually, you may want to provide a gpg-encrypted file either on the command line (-p <filepath>) or in set-config.passwords." +fi + +test -f "$steps_dir/linux-signer-rcodesign-sign.done" || [ -n "$RCODESIGN_PW" ] || read -sp "Enter rcodesign passphrase for key-1: " RCODESIGN_PW echo -is_project torbrowser && nssdb=torbrowser-nssdb7 -is_project mullvadbrowser && nssdb=mullvadbrowser-nssdb-1 -test -f "$steps_dir/linux-signer-signmars.done" || + +test -f "$steps_dir/linux-signer-signmars.done" || [ -n "$NSSPASS" ] || read -sp "Enter $nssdb (mar signing) passphrase: " NSSPASS echo if is_project torbrowser; then - test -f "$steps_dir/linux-signer-sign-android-apks.done" || + test -f "$steps_dir/linux-signer-sign-android-apks.done" || [ -n "$KSPASS" ] || read -sp "Enter android apk signing password ($tbb_version_type): " KSPASS echo fi -test -f "$steps_dir/linux-signer-authenticode-signing.done" || +test -f "$steps_dir/linux-signer-authenticode-signing.done" || [ -n "$YUBIPASS" ] || read -sp "Enter windows authenticode passphrase: " YUBIPASS echo -test -f "$steps_dir/linux-signer-gpg-sign.done" || +test -f "$steps_dir/linux-signer-gpg-sign.done" || [ -n "$GPG_PASS" ] || read -sp "Enter gpg passphrase: " GPG_PASS echo function set-time-on-signing-machine { - local current_time=$(date -u) + local current_time=$(date -u -Iseconds) ssh "$ssh_host_linux_signer" sudo /usr/bin/date -s "'$current_time'" } @@ -178,6 +203,10 @@ function do_step { echo "$(date -Iseconds) - Finished step: $1" } +function is_legacy { + [[ "$tbb_version" = 13.* ]] +} + export SIGNING_PROJECTNAME do_step set-time-on-signing-machine @@ -195,9 +224,9 @@ do_step sync-scripts-to-linux-signer do_step sync-before-linux-signer-signmars do_step linux-signer-signmars do_step sync-after-signmars -is_project torbrowser && \ +is_project torbrowser && ! is_legacy && \ do_step linux-signer-sign-android-apks -is_project torbrowser && \ +is_project torbrowser && ! is_legacy && \ do_step sync-after-sign-android-apks do_step linux-signer-authenticode-signing do_step sync-after-authenticode-signing @@ -211,5 +240,6 @@ do_step download-unsigned-sha256sums-gpg-signatures-from-people-tpo do_step sync-local-to-staticiforme do_step sync-scripts-to-staticiforme do_step staticiforme-prepare-cdn-dist-upload -do_step upload-update_responses-to-staticiforme +! is_legacy && + do_step upload-update_responses-to-staticiforme do_step finished-signing-clean-linux-signer ===================================== tools/signing/set-config.passwords ===================================== @@ -0,0 +1,2 @@ +# Path to a gpg-encrypted cache of passwords not to be asked on each run +passwords_gpg_file=~/.tor-browser-signing/tor-browser-passwords.txt.gpg View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/3… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/3… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] 3 commits: fixup! TB 40597: Implement TorSettings module
by Pier Angelo Vendrame (＠pierov) 20 Jan '25

20 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 0b42b720 by Henry Wilkes at 2025-01-20T17:57:59+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Return only the bridge settings from Moat. Rather than return { bridges: enabled: true, ... } from MoatRPC API, we just return the relevant parts. This should make it clearer that Moat can *only* change TorSettings.bridges, and nothing else. - - - - - 7129f408 by Henry Wilkes at 2025-01-20T17:57:59+00:00 fixup! TB 40597: Implement TorSettings module TB 41921: Have Moat bridges pass through TorSettings rather than using TorProvider directly. TorSettings should be the only caller to TorProvider.writeSettings, so we can establish more control over which bridges are in use at any given moment in one place. Also add some sanitation to the Moat response. - - - - - 6ea6bd74 by Henry Wilkes at 2025-01-20T17:57:59+00:00 fixup! TB 40933: Add tor-launcher functionality TB 41921: Use temporary bridges for TorProvider initialisation. It is unlikely that temporary bridges would be present when TorProvider is initialised, but it should be using whatever TorSettings.applySettings uses. - - - - - 4 changed files: - toolkit/components/tor-launcher/TorProvider.sys.mjs - toolkit/modules/Moat.sys.mjs - toolkit/modules/TorConnect.sys.mjs - toolkit/modules/TorSettings.sys.mjs Changes: ===================================== toolkit/components/tor-launcher/TorProvider.sys.mjs ===================================== @@ -227,7 +227,7 @@ export class TorProvider { if (this.ownsTorDaemon) { try { await lazy.TorSettings.initializedPromise; - await this.writeSettings(lazy.TorSettings.getSettings()); + await this.writeSettings(); } catch (e) { logger.warn( "Failed to initialize TorSettings or to write our initial settings. Continuing the initialization anyway.", @@ -269,11 +269,13 @@ export class TorProvider { /** * Send settings to the tor daemon. * - * @param {object} settings A settings object, as returned by - * TorSettings.getSettings(). This allow to try settings without passing - * through TorSettings. + * This should only be called internally or by the TorSettings module. */ - async writeSettings(settings) { + async writeSettings() { + // Fetch the current settings. + // We set the useTemporary parameter since we want to apply temporary + // bridges if they are available. + const settings = lazy.TorSettings.getSettings(true); logger.debug("TorProvider.writeSettings", settings); const torSettings = new Map(); ===================================== toolkit/modules/Moat.sys.mjs ===================================== @@ -13,7 +13,6 @@ ChromeUtils.defineESModuleGetters(lazy, { DomainFrontRequestBuilder: "resource://gre/modules/DomainFrontedRequests.sys.mjs", TorBridgeSource: "resource://gre/modules/TorSettings.sys.mjs", - TorSettings: "resource://gre/modules/TorSettings.sys.mjs", }); const TorLauncherPrefs = Object.freeze({ @@ -73,6 +72,25 @@ class InternetTestResponseListener { } } +/** + * @typedef {Object} MoatBridges + * + * Bridge settings that can be passed to TorSettings.bridges. + * + * @property {number} source - The `TorBridgeSource` type. + * @property {string} [builtin_type] - The built-in bridge type. + * @property {string[]} [bridge_strings] - The bridge lines. + */ + +/** + * @typedef {Object} MoatSettings + * + * The settings returned by Moat. + * + * @property {MoatBridges[]} bridgesList - The list of bridges found. + * @property {string} [country] - The detected country (region). + */ + /** * Constructs JSON objects and sends requests over Moat. * The documentation about the JSON schemas to use are available at @@ -213,37 +231,31 @@ export class MoatRPC { return { bridges, qrcode: qrcodeImg }; } - // Convert received settings object to format used by TorSettings module. - #fixupSettings(settings) { + /** + * Extract bridges from the received Moat settings object. + * + * @param {Object} settings - The received settings. + * @return {MoatBridge} The extracted bridges. + */ + #extractBridges(settings) { if (!("bridges" in settings)) { throw new Error("Expected to find `bridges` in the settings object."); } - const retval = { - bridges: { - enabled: true, - }, - }; + const bridges = {}; switch (settings.bridges.source) { case "builtin": - retval.bridges.source = lazy.TorBridgeSource.BuiltIn; - retval.bridges.builtin_type = settings.bridges.type; - // TorSettings will ignore strings for built-in bridges, and use the - // ones it already knows, instead. However, when we try these settings - // in the connect assist, we skip TorSettings. Therefore, we set the - // lines also here (the ones we already know, not the ones we receive - // from Moat). This needs TorSettings to be initialized, which by now - // should have already happened (this method is used only by TorConnect, - // that needs TorSettings to be initialized). - // In any case, getBuiltinBridges will throw if the data is not ready, - // yet. - retval.bridges.bridge_strings = lazy.TorSettings.getBuiltinBridges( - settings.bridges.type - ); + bridges.source = lazy.TorBridgeSource.BuiltIn; + bridges.builtin_type = String(settings.bridges.type); + // Ignore the bridge_strings argument since we will use our built-in + // bridge strings instead. break; case "bridgedb": - retval.bridges.source = lazy.TorBridgeSource.BridgeDB; - if (settings.bridges.bridge_strings) { - retval.bridges.bridge_strings = settings.bridges.bridge_strings; + bridges.source = lazy.TorBridgeSource.BridgeDB; + if (settings.bridges.bridge_strings?.length) { + bridges.bridge_strings = Array.from( + settings.bridges.bridge_strings, + item => String(item) + ); } else { throw new Error( "Received no bridge-strings for BridgeDB bridge source" @@ -255,37 +267,38 @@ export class MoatRPC { `Unexpected bridge source '${settings.bridges.source}'` ); } - return retval; + return bridges; } - // Converts a list of settings objects received from BridgeDB to a list of - // settings objects understood by the TorSettings module. - // In the event of error, returns an empty list. - #fixupSettingsList(settingsList) { - const retval = []; + /** + * Extract a list of bridges from the received Moat settings object. + * + * @param {Object} settings - The received settings. + * @return {MoatBridge[]} The list of extracted bridges. + */ + #extractBridgesList(settingsList) { + const bridgesList = []; for (const settings of settingsList) { try { - retval.push(this.#fixupSettings(settings)); + bridgesList.push(this.#extractBridges(settings)); } catch (ex) { log.error(ex); } } - return retval; + return bridgesList; } - // Request tor settings for the user optionally based on their location - // (derived from their IP). Takes the following parameters: - // - transports: optional, an array of transports available to the client; if - // empty (or not given) returns settings using all working transports known - // to the server - // - country: optional, an ISO 3166-1 alpha-2 country code to request settings - // for; if not provided the country is determined by the user's IP address - // - // Returns an object with the detected country code and an array of settings - // in a format that can be passed to the TorSettings module. This array might - // be empty if the country has no associated settings. - // If the server cannot determine the user's country (and no country code is - // provided), then null is returned instead of the object. + /** + * Request tor settings for the user optionally based on their location + * (derived from their IP). Takes the following parameters: + * + * @param {string[]} transports - A list of transports we support. + * @param {?string} country - The region to request bridges for, as an + * ISO 3166-1 alpha-2 region code, or `null` to have the server + * automatically determine the region. + * @returns {?MoatSettings} - The returned settings from the server, or `null` + * if the region could not be determined by the server. + */ async circumvention_settings(transports, country) { const args = { transports: transports ? transports : [], @@ -306,7 +319,7 @@ export class MoatRPC { throw new Error(`MoatRPC: ${detail} (${code})`); } else if ("settings" in response) { - settings.settings = this.#fixupSettingsList(response.settings); + settings.bridgesList = this.#extractBridgesList(response.settings); } if ("country" in response) { settings.country = response.country; @@ -349,14 +362,12 @@ export class MoatRPC { return map; } - // Request a copy of the defaul/fallback bridge settings, takes the following - // parameters: - // - transports: optional, an array of transports available to the client; if - // empty (or not given) returns settings using all working transports known - // to the server - // - // returns an array of settings objects in roughly the same format as the - // _settings object on the TorSettings module + /** + * Request a copy of the default/fallback bridge settings. + * + * @param {string[]} transports - A list of transports we support. + * @returns {MoatBridges[]} - The list of bridges found. + */ async circumvention_defaults(transports) { const args = { transports: transports ? transports : [], @@ -367,7 +378,7 @@ export class MoatRPC { const detail = response.errors[0].detail; throw new Error(`MoatRPC: ${detail} (${code})`); } else if ("settings" in response) { - return this.#fixupSettingsList(response.settings); + return this.#extractBridgesList(response.settings); } return []; } ===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -103,11 +103,12 @@ export const TorConnectTopics = Object.freeze({ * failing bootstrap. * @property {integer} [options.simulateDelay] - The delay in microseconds to * apply to simulated bootstraps. - * @property {object} [options.simulateMoatResponse] - Simulate a Moat response - * for circumvention settings. Should include a "settings" property, and - * optionally a "country" property. You may add a "simulateCensorship" - * property to some of the settings to make only their bootstrap attempts - * fail. + * @property {MoatSettings} [options.simulateMoatResponse] - Simulate a Moat + * response for circumvention settings. Should include a "bridgesList" + * property, and optionally a "country" property. The "bridgesList" property + * should be an Array of MoatBridges objects that match the bridge settings + * accepted by TorSettings.bridges, plus you may add a "simulateCensorship" + * property to make only their bootstrap attempts fail. * @property {boolean} [options.testInternet] - Whether to also test the * internet connection. * @property {boolean} [options.simulateOffline] - Whether to simulate an @@ -214,6 +215,14 @@ class BootstrapAttempt { return promise; } + /** + * Method to call just after the Bootstrapped stage is set in response to this + * bootstrap attempt. + */ + postBootstrapped() { + // Nothing to do. + } + /** * Run the attempt. * @@ -395,17 +404,11 @@ class AutoBootstrapAttempt { */ #cancelledPromise = null; /** - * The found settings from Moat. - * - * @type {?object[]} - */ - #settings = null; - /** - * The last settings that have been applied to the TorProvider, if any. + * The list of bridge configurations from Moat. * - * @type {?object} + * @type {?MoatBridges[]} */ - #changedSetting = null; + #bridgesList = null; /** * The detected region code returned by Moat, if any. * @@ -438,13 +441,8 @@ class AutoBootstrapAttempt { // Run cleanup before we resolve the promise to ensure two instances // of AutoBootstrapAttempt are not trying to change the settings at // the same time. - if (this.#changedSetting) { - if (arg.result === "complete") { - // Persist the current settings to preferences. - lazy.TorSettings.setSettings(this.#changedSetting); - lazy.TorSettings.saveToPrefs(); - } // else, applySettings will restore the current settings. - await lazy.TorSettings.applySettings(); + if (arg.result !== "complete") { + await lazy.TorSettings.clearTemporaryBridges(); } } catch (error) { lazy.logger.error("Unexpected error in auto-bootstrap cleanup", error); @@ -466,6 +464,15 @@ class AutoBootstrapAttempt { return promise; } + /** + * Method to call just after the Bootstrapped stage is set in response to this + * bootstrap attempt. + */ + postBootstrapped() { + // Persist the current settings to preferences. + lazy.TorSettings.saveTemporaryBridges(); + } + /** * Run the attempt. * @@ -477,12 +484,12 @@ class AutoBootstrapAttempt { * @param {BootstrapOptions} options - Options to apply to the bootstrap. */ async #runInternal(progressCallback, options) { - await this.#fetchSettings(options); + await this.#fetchBridges(options); if (this.#cancelled || this.#resolved) { return; } - if (!this.#settings?.length) { + if (!this.#bridgesList?.length) { this.#resolveRun({ error: new TorConnectError( options.regionCode === "automatic" && !this.detectedRegion @@ -493,14 +500,14 @@ class AutoBootstrapAttempt { } // Apply each of our settings and try to bootstrap with each. - for (const [index, currentSetting] of this.#settings.entries()) { + for (const [index, bridges] of this.#bridgesList.entries()) { lazy.logger.info( `Attempting Bootstrap with configuration ${index + 1}/${ - this.#settings.length + this.#bridgesList.length }` ); - await this.#trySetting(currentSetting, progressCallback, options); + await this.#tryBridges(bridges, progressCallback, options); if (this.#cancelled || this.#resolved) { return; @@ -518,7 +525,7 @@ class AutoBootstrapAttempt { * * @param {BootstrapOptions} options - Options to apply to the bootstrap. */ - async #fetchSettings(options) { + async #fetchBridges(options) { if (options.simulateMoatResponse) { await Promise.race([ new Promise(res => setTimeout(res, options.simulateDelay || 0)), @@ -530,7 +537,7 @@ class AutoBootstrapAttempt { } this.detectedRegion = options.simulateMoatResponse.country || null; - this.#settings = options.simulateMoatResponse.settings ?? null; + this.#bridgesList = options.simulateMoatResponse.bridgesList ?? null; return; } @@ -564,16 +571,16 @@ class AutoBootstrapAttempt { this.detectedRegion = maybeSettings?.country || null; - if (maybeSettings?.settings?.length) { - this.#settings = maybeSettings.settings; + if (maybeSettings?.bridgesList?.length) { + this.#bridgesList = maybeSettings.bridgesList; } else { // Keep consistency with the other call. - this.#settings = await Promise.race([ + this.#bridgesList = await Promise.race([ moat.circumvention_defaults([ ...lazy.TorSettings.builtinBridgeTypes, "vanilla", ]), - // This might set this.#settings to undefined. + // This might set this.#bridgesList to undefined. this.#cancelledPromise, ]); } @@ -586,21 +593,21 @@ class AutoBootstrapAttempt { /** * Try to apply the settings we fetched. * - * @param {object} setting - The setting to try. + * @param {MoatBridges} bridges - The bridges to try. * @param {ProgressCallback} progressCallback - The callback to invoke with * the bootstrap progress. * @param {BootstrapOptions} options - Options to apply to the bootstrap. */ - async #trySetting(setting, progressCallback, options) { + async #tryBridges(bridges, progressCallback, options) { if (this.#cancelled || this.#resolved) { return; } - if (options.simulateMoatResponse && setting.simulateCensorship) { + if (options.simulateMoatResponse && bridges.simulateCensorship) { // Move the simulateCensorship option to the options for the next // BootstrapAttempt. - setting = structuredClone(setting); - delete setting.simulateCensorship; + bridges = structuredClone(bridges); + delete bridges.simulateCensorship; options = { ...options, simulateCensorship: true }; } @@ -616,14 +623,7 @@ class AutoBootstrapAttempt { // Another idea (maybe easier to implement) is to disable the settings // UI while *any* bootstrap is going on. // This is also documented in tor-browser#41921. - const provider = await lazy.TorProviderBuilder.build(); - this.#changedSetting = setting; - // We need to merge with old settings, in case the user is using a proxy - // or is behind a firewall. - await provider.writeSettings({ - ...lazy.TorSettings.getSettings(), - ...setting, - }); + await lazy.TorSettings.applyTemporaryBridges(bridges); if (this.#cancelled || this.#resolved) { return; @@ -642,7 +642,7 @@ class AutoBootstrapAttempt { error instanceof TorConnectError && error.code === TorConnectError.BootstrapError ) { - lazy.logger.info("TorConnect setting failed", setting, error); + lazy.logger.info("TorConnect setting failed", bridges, error); // Try with the next settings. // NOTE: We do not restore the user settings in between these runs. // Instead we wait for #resolveRun callback to do so. @@ -1459,6 +1459,12 @@ export const TorConnect = { } this._setStage(TorConnectStage.Bootstrapped); Services.obs.notifyObservers(null, TorConnectTopics.BootstrapComplete); + + // Now call the postBootstrapped method. We do this after changing the + // stage to ensure that AutoBootstrapAttempt.postBootstrapped call to + // saveTemporaryBridges does not trigger SettingsChanged too early and + // cancel itself. + bootstrapAttempt.postBootstrapped(); return; } ===================================== toolkit/modules/TorSettings.sys.mjs ===================================== @@ -198,6 +198,14 @@ class TorSettingsImpl { allowed_ports: [], }, }; + + /** + * Temporary bridge settings to apply instead of #settings.bridges. + * + * @type {?Object} + */ + #temporaryBridgeSettings = null; + /** * Accumulated errors from trying to set settings. * @@ -713,6 +721,8 @@ class TorSettingsImpl { try { this.#allowUninitialized = true; this.#loadFromPrefs(); + // We do not pass on the loaded settings to the TorProvider yet. Instead + // TorProvider will ask for these once it has initialised. } finally { this.#allowUninitialized = false; this.#notificationQueue.clear(); @@ -971,7 +981,7 @@ class TorSettingsImpl { async applySettings() { this.#checkIfInitialized(); const provider = await lazy.TorProviderBuilder.build(); - await provider.writeSettings(this.getSettings()); + await provider.writeSettings(); } /** @@ -1073,12 +1083,20 @@ class TorSettingsImpl { /** * Get a copy of all our settings. * + * @param {boolean} [useTemporary=false] - Whether the returned settings + * should use the temporary bridge settings, if any, instead. + * * @returns {object} A copy of the settings object */ - getSettings() { + getSettings(useTemporary = false) { lazy.logger.debug("getSettings()"); this.#checkIfInitialized(); - return structuredClone(this.#settings); + const settings = structuredClone(this.#settings); + if (useTemporary && this.#temporaryBridgeSettings) { + // Override the bridge settings with our temporary ones. + settings.bridges = structuredClone(this.#temporaryBridgeSettings); + } + return settings; } /** @@ -1097,6 +1115,83 @@ class TorSettingsImpl { } return types; } + + /** + * Apply some Moat bridges temporarily. + * + * These bridges will not yet be saved to settings. + * + * @param {MoatBridges} bridges - The bridges to apply. + */ + async applyTemporaryBridges(bridges) { + this.#checkIfInitialized(); + + if ( + bridges.source !== TorBridgeSource.BuiltIn && + bridges.source !== TorBridgeSource.BridgeDB + ) { + throw new Error(`Invalid bridge source ${bridges.source}`); + } + + const bridgeSettings = { + enabled: true, + source: bridges.source, + }; + + if (bridges.source === TorBridgeSource.BuiltIn) { + if (!bridges.builtin_type) { + throw Error("Missing a built-in type"); + } + bridgeSettings.builtin_type = String(bridges.builtin_type); + const bridgeStrings = this.getBuiltinBridges(bridgeSettings.builtin_type); + if (!bridgeStrings.length) { + throw new Error(`No builtin bridges for type ${bridges.builtin_type}`); + } + bridgeSettings.bridge_strings = bridgeStrings; + } else { + // BridgeDB. + if (!bridges.bridge_strings?.length) { + throw new Error("Missing bridges strings"); + } + // TODO: Can we safely verify the format of the bridge addresses sent from + // Moat? + bridgeSettings.bridge_strings = Array.from(bridges.bridge_strings, item => + String(item) + ); + } + + // After checks are complete, we commit them. + this.#temporaryBridgeSettings = bridgeSettings; + await this.applySettings(); + } + + /** + * Save to current temporary bridges to be permanent instead. + */ + async saveTemporaryBridges() { + this.#checkIfInitialized(); + if (!this.#temporaryBridgeSettings) { + lazy.logger.warn("No temporary bridges to save"); + return; + } + this.setSettings({ bridges: this.#temporaryBridgeSettings }); + this.#temporaryBridgeSettings = null; + this.saveToPrefs(); + await this.applySettings(); + } + + /** + * Clear the current temporary bridges. + */ + async clearTemporaryBridges() { + this.#checkIfInitialized(); + if (!this.#temporaryBridgeSettings) { + lazy.logger.debug("No temporary bridges to clear"); + return; + } + this.#temporaryBridgeSettings = null; + await this.applySettings(); + } } export const TorSettings = new TorSettingsImpl(); View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/9e5f76… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/9e5f76… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/mullvad-browser][mullvad-browser-128.6.0esr-14.5-1] fixup! Firefox preference overrides.
by Pier Angelo Vendrame (＠pierov) 20 Jan '25

20 Jan '25

Pier Angelo Vendrame pushed to branch mullvad-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Mullvad Browser Commits: 76d1293f by Pier Angelo Vendrame at 2025-01-20T18:42:58+01:00 fixup! Firefox preference overrides. BB 43402: set browser.startup.blankWindow false. - - - - - 1 changed file: - browser/app/profile/001-base-profile.js Changes: ===================================== browser/app/profile/001-base-profile.js ===================================== @@ -454,6 +454,9 @@ pref("privacy.resistFingerprinting.letterboxing.gradient", true); pref("privacy.resistFingerprinting.letterboxing.rememberSize", false); // tor-browser#41695: How many warnings we show if user closes them without restoring the window size pref("privacy.resistFingerprinting.resizeWarnings", 3); +// tor-browser#43402: Avoid a resize from the skeleton to the newwin size. +// Should be fixed in ESR-140 with Bug 1448423. +pref("browser.startup.blankWindow", false); // Enforce Network Information API as disabled pref("dom.netinfo.enabled", false); pref("network.http.referer.defaultPolicy", 2); // Bug 32948: Make referer behavior consistent regardless of private browing mode status View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/commit/76d… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/mullvad-browser/-/commit/76d… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][base-browser-128.6.0esr-14.5-1] fixup! Firefox preference overrides.
by Pier Angelo Vendrame (＠pierov) 20 Jan '25

20 Jan '25

Pier Angelo Vendrame pushed to branch base-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 186910c7 by Pier Angelo Vendrame at 2025-01-20T18:42:24+01:00 fixup! Firefox preference overrides. BB 43402: set browser.startup.blankWindow false. - - - - - 1 changed file: - browser/app/profile/001-base-profile.js Changes: ===================================== browser/app/profile/001-base-profile.js ===================================== @@ -454,6 +454,9 @@ pref("privacy.resistFingerprinting.letterboxing.gradient", true); pref("privacy.resistFingerprinting.letterboxing.rememberSize", false); // tor-browser#41695: How many warnings we show if user closes them without restoring the window size pref("privacy.resistFingerprinting.resizeWarnings", 3); +// tor-browser#43402: Avoid a resize from the skeleton to the newwin size. +// Should be fixed in ESR-140 with Bug 1448423. +pref("browser.startup.blankWindow", false); // Enforce Network Information API as disabled pref("dom.netinfo.enabled", false); pref("network.http.referer.defaultPolicy", 2); // Bug 32948: Make referer behavior consistent regardless of private browing mode status View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/186910c… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/186910c… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser][tor-browser-128.6.0esr-14.5-1] fixup! Firefox preference overrides.
by Pier Angelo Vendrame (＠pierov) 20 Jan '25

20 Jan '25

Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 9e5f7638 by Pier Angelo Vendrame at 2025-01-20T14:39:01+01:00 fixup! Firefox preference overrides. BB 43402: set browser.startup.blankWindow false. - - - - - 1 changed file: - browser/app/profile/001-base-profile.js Changes: ===================================== browser/app/profile/001-base-profile.js ===================================== @@ -454,6 +454,9 @@ pref("privacy.resistFingerprinting.letterboxing.gradient", true); pref("privacy.resistFingerprinting.letterboxing.rememberSize", false); // tor-browser#41695: How many warnings we show if user closes them without restoring the window size pref("privacy.resistFingerprinting.resizeWarnings", 3); +// tor-browser#43402: Avoid a resize from the skeleton to the newwin size. +// Should be fixed in ESR-140 with Bug 1448423. +pref("browser.startup.blankWindow", false); // Enforce Network Information API as disabled pref("dom.netinfo.enabled", false); pref("network.http.referer.defaultPolicy", 2); // Bug 32948: Make referer behavior consistent regardless of private browing mode status View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/9e5f763… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/9e5f763… You're receiving this email because of your account on gitlab.torproject.org.

1 0

[Git][tpo/applications/tor-browser-build][maint-14.0] Bug 41350: Increase timeout in rcodesign-notary-submit to 6000s
by boklm (＠boklm) 16 Jan '25

16 Jan '25

boklm pushed to branch maint-14.0 at The Tor Project / Applications / tor-browser-build Commits: a4137090 by Nicolas Vigier at 2025-01-16T16:44:34+01:00 Bug 41350: Increase timeout in rcodesign-notary-submit to 6000s - - - - - 1 changed file: - tools/signing/rcodesign-notary-submit Changes: ===================================== tools/signing/rcodesign-notary-submit ===================================== @@ -21,7 +21,7 @@ display_name=$(display_name) tar -C "$tmpdir" -xf "$macos_rcodesign_signed_tar_dir/$(project-name)-macos-${tbb_version}-rcodesign-signed.tar.zst" -"$script_dir/../local/rcodesign-128/rcodesign" notary-submit --api-key-path "$appstoreconnect_api_key_path" --staple "$tmpdir/$display_name.app" +"$script_dir/../local/rcodesign-128/rcodesign" notary-submit --max-wait-seconds 6000 --api-key-path "$appstoreconnect_api_key_path" --staple "$tmpdir/$display_name.app" output_file="$(project-name)-${tbb_version}-notarized+stapled.tar.zst" tar -C "$tmpdir" -caf "$tmpdir/$output_file" "$display_name.app" View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/a… -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser-build/-/commit/a… You're receiving this email because of your account on gitlab.torproject.org.

1 0