From a3cde999c3822b2b74911146dce5f79b4631ec07 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:07:24 +0000 Subject: [PATCH 1/8] install.sh and openRepoShape: a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` is respelled with `cygpath -u` or refused before install.sh's fetch, and the PATH note single-quotes a directory bash would still read, and `--install` refuses one holding `:` or a newline, so install.sh names `/d/tools/bin` as the command does and the `export PATH` line does what it says (#667) #649's two leftovers, both from #656's review. install.sh read `$OPENREPOSHAPE_BIN_DIR` as typed, so with `D:\tools\bin` its `preflight:` and `done:` lines (and `install:` for an old release) said `D:\tools\bin/openRepoShape` while the command's own lines said `/d/tools/bin/openRepoShape`, and where no `cygpath` answers the command's `--install` refused (exit 2) only after install.sh had fetched it. install.sh now carries a copy of the command's rule where it reads the variable, before the symlink check and the fetch: a value that starts `[A-Za-z]:/` or `[A-Za-z]:\` is spelled with `cygpath -u --`, and is refused with exit 2 where no `cygpath` answers, fails, or answers a drive spelling again. The refusal does not echo the value. The command's PATH note printed `export PATH="$dir:$PATH"` with the directory between double quotes as it was, so `a"b$HOME c` printed a line `eval` could not parse, and a `$`, a backquote or a `\` in a name was read, not kept. `export_path_line` keeps today's line byte for byte for a directory that double quotes hold (a space and an apostrophe included) and, for one holding a `"`, `$`, backquote, `\` or `!` (bash reads that at a prompt as history), prints it between single quotes with `'\''` for an apostrophe. `--install` also refuses a directory holding a `:` or a newline, which no `$PATH` line can name, before anything is placed; the newline's refusal does not echo it. Tests feed one value through install.sh and through the command, in each spelling and each state of `cygpath`, and pin every `install:`, `preflight:` and `done:` line; the PATH note is checked as printed, by `eval` and by `bash -i`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- install.sh | 36 ++++- openRepoShape | 58 ++++++- tests/test_install_sh.py | 157 ++++++++++++++++++- tests/test_openreposhape_command.py | 229 ++++++++++++++++++++++++++-- 4 files changed, 460 insertions(+), 20 deletions(-) diff --git a/install.sh b/install.sh index 9a8a78fb..fa0fbd57 100755 --- a/install.sh +++ b/install.sh @@ -41,8 +41,10 @@ # does not stay on the release tag (one older than v1.0.1 cannot record # it): the lines above say which; # 2 refused before anything was installed: a version that is not a ref, no -# release found, a fetch that failed, a file that is not the command, or -# a command already there as a symbolic link (pipx's, #563). +# release found, a fetch that failed, a file that is not the command, a +# command already there as a symbolic link (pipx's, #563), or a +# `$OPENREPOSHAPE_BIN_DIR` spelled for Windows where no `cygpath` can +# spell it the way bash does (#667). # git-filter-repo never changes it: only adopting needs it, and # `adopt-project.py` refuses with its own install command where it is missing. # What it changes is the summary (#595): `adoption:` says whether it is there @@ -334,7 +336,7 @@ filter_repo_step() { } main() { - local version="" asked launcher_url bin_dir target shown status=0 pinned=1 + local version="" asked launcher_url bin_dir spelled target shown status=0 pinned=1 local machine="this machine is ready" # NOTHING A CHILD READS IS THIS SCRIPT. Piped, stdin is the pipe the @@ -362,6 +364,33 @@ main() { command -v curl >/dev/null 2>&1 || die "curl is not installed, and this installer fetches with it. Install curl, or install by hand with: gh api repos/$REPO/contents/openRepoShape -H 'Accept: application/vnd.github.raw' | bash -s -- --install" + bin_dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" + # A WINDOWS SPELLING IS SPELLED THE WAY BASH SPELLS IT, THE COMMAND'S OWN + # RULE (#667, after #649), before it is named in a line or looked at: a + # value typed `D:\tools\bin` was echoed as typed, in the `install:`, + # `preflight:` and `done:` lines, while the command's own lines said + # `/d/tools/bin`, and where no `cygpath` answers the command refuses it + # (exit 2), which this file learnt only after the fetch. So the same + # `cygpath -u` respells it here, and the same refusal comes first, before + # anything is asked of the network. The `case` is `install_commands`'s in + # `openRepoShape`, copied: this file arrives alone down a pipe, and + # `tests/test_install_sh.py` feeds one value through both. The refusal + # does not echo the value: it may hold a newline that forges a line. + case "$bin_dir" in + [A-Za-z]:/* | [A-Za-z]:\\*) + spelled="" + if command -v cygpath >/dev/null 2>&1; then + spelled="$(cygpath -u -- "$bin_dir" 2>/dev/null)" || spelled="" + fi + case "$spelled" in + "" | [A-Za-z]:*) + die "\$OPENREPOSHAPE_BIN_DIR is spelled for Windows, and no cygpath here spells it the way bash does, so the directory could not go on \$PATH: the drive's colon splits it. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to the directory as this shell spells it, then run this line again." + ;; + esac + bin_dir="$spelled" + ;; + esac + # A LINK IS NOT A PLACE TO INSTALL TO (#563), and this installer says so # itself, before anything is asked of the network. pipx places the # openreposhape package's `openRepoShape` as a symbolic link in this same @@ -369,7 +398,6 @@ main() { # through a link into the package's own environment. The command refuses # such a target too, but only from the release that added the check: the # release named here may be older, so its refusal cannot be relied on. - bin_dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" target="$bin_dir/openRepoShape" [[ ! -L "$target" ]] || die "$target is a symbolic link, so nothing was installed: pipx places the openreposhape package's openRepoShape that way, and installing this one through the link would overwrite the package's command. Use one installer or the other: diff --git a/openRepoShape b/openRepoShape index 61291e26..da550aee 100755 --- a/openRepoShape +++ b/openRepoShape @@ -403,9 +403,60 @@ record_ref() { fi } +# THE `export PATH` LINE THE PATH NOTE PRINTS, QUOTED FOR BASH (#667). It is +# a line somebody copies and pastes, so it has to do what it says for any +# directory that can be on a `$PATH`. Between double quotes bash still reads +# `"`, `$`, a backquote and `\`, and an interactive bash `!` too (history), +# so a directory holding one of them is printed between single quotes, where +# nothing is read but the quote itself, which is written `'\''`. Every other +# directory keeps the line this command has always printed, byte for byte: +# README says `export PATH=…`, and a space or an apostrophe is safe in +# double quotes. A `:` splits a `$PATH` whatever the quoting, and a newline +# breaks the printed line in two, so `install_commands` refuses both before +# this is asked. +export_path_line() { + local dir="$1" backslash='\' quote="'" rest quoted="" + case "$dir" in + *[\"\$\`\\!]*) + # The apostrophes are walked, not replaced with `${dir//…}`: how a + # pattern substitution reads a quote in its replacement has changed + # between bash releases (this runs on 3.2 as well), and `sed` is + # not one of the programs `--install` uses. + rest="$dir" + while :; do + case "$rest" in + *"$quote"*) + quoted="$quoted${rest%%"$quote"*}$quote$backslash$quote$quote" + rest="${rest#*"$quote"}" + ;; + *) + quoted="$quoted$rest" + break + ;; + esac + done + printf 'export PATH=%s%s%s:"$PATH"' "$quote" "$quoted" "$quote" + ;; + *) printf 'export PATH="%s:$PATH"' "$dir" ;; + esac +} + install_commands() { local dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" local name target source verb placed=0 recording=0 record spelled="" + # NO LINE ABOUT `$PATH` CAN NAME A DIRECTORY HOLDING A NEWLINE OR A `:` + # (#667): the colon is what splits a `$PATH`, and a newline breaks the + # line the note prints in two. Refused before anything is placed, as the + # Windows spelling below is. The newline goes first, and its refusal says + # nothing of the value, so that no refusal after it echoes one that can + # forge a line of output. The colon is checked after the Windows + # spelling, which holds one by definition, is respelled without one, and + # refuses what it cannot respell. + case "$dir" in + *$'\n'*) + die "--install would place this command in a directory whose name holds a newline, which no line about \$PATH can name: it would break in two. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." + ;; + esac # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with # `cygpath -u`, which Git Bash has, before anything is checked or placed: # `D:\tools\bin` becomes `/d/tools/bin`. Kept as typed, every line below @@ -428,6 +479,11 @@ install_commands() { dir="$spelled" ;; esac + case "$dir" in + *:*) + die "--install would place this command in '$dir', which holds a ':', and a ':' is what splits a \$PATH into its directories, so no line about \$PATH can name it. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." + ;; + esac if [ "$REF_FROM_RECORD" -eq 1 ] || { [ "$REF_NAMED" -eq 1 ] && [ "${OPENREPOSHAPE_RECORD_REF:-}" = 1 ]; }; then recording=1 @@ -498,7 +554,7 @@ install_commands() { *":$dir:"*) ;; *) say " $dir is not on \$PATH. Add it:" - say " export PATH=\"$dir:\$PATH\"" + say " $(export_path_line "$dir")" ;; esac # LAST, because it is about the NEXT command rather than about this one. diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index 2c69f66c..bb0bd660 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -46,8 +46,11 @@ import pytest from conftest import BASH, BASH_SKIP, REPO, bash_path -from test_openreposhape_command import (COMMAND, INSTALL_NEEDS, command_env, - put_on_path, write_script) +from test_openreposhape_command import (BIN_DIR_SPELLINGS, COMMAND, + CYGPATH_STATES, INSTALL_NEEDS, + command_env, machine_path, + put_cygpath, put_on_path, run_cmd, + typed_bin_dir, write_script) INSTALLER = REPO / "install.sh" @@ -686,6 +689,156 @@ def test_a_command_that_is_a_symlink_is_refused_before_any_request( assert list(net.temporary.iterdir()) == [] +# --- a Windows-spelled `$OPENREPOSHAPE_BIN_DIR` (#667) ----------------------- + +def in_its_own_directory(tmp_path: Path, name: str) -> Path: + """`tmp_path/name`, made: one run's whole world, so that two runs of one + test cannot see each other's files.""" + directory = tmp_path / name + directory.mkdir() + return directory + + +@BIN_DIR_SPELLINGS +@pytest.mark.parametrize("state", CYGPATH_STATES) +def test_the_installer_and_the_command_answer_one_drive_directory_alike( + tmp_path, spelling, state): + """ONE `$OPENREPOSHAPE_BIN_DIR`, spelled for Windows, through install.sh + and through the command it installs (#667). + + The command respells it with `cygpath -u`, or refuses it where no + `cygpath` can (#649), and install.sh carries a copy of that `case`, + because it arrives alone and cannot share the command's. The two must + never answer one value differently, so each is given the same value, + in each spelling, with `cygpath` in each state: spelled, failing, + answering a drive spelling again, or not there. Where it spells, both + place the command at the directory as bash does and name no other; where + it does not, both refuse with exit 2, and install.sh has asked the + network nothing. + """ + net = Net(in_its_own_directory(tmp_path, "installer")) + net.bin = net.tmp_path / "drive" / "bin" + command_home = in_its_own_directory(tmp_path, "command") + command_bin = command_home / "drive" / "bin" + typed_installer = typed_bin_dir(net.bin, spelling) + typed_command = typed_bin_dir(command_bin, spelling) + put_cygpath(net.path, typed_installer, state, bash_path(net.bin)) + command_env_path = machine_path(command_home, ()) + command_env_path.pop("_log") + put_cygpath(command_home / "machine-path", typed_command, state, + bash_path(command_bin)) + + installer = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed_installer}) + command = run_cmd("--install", home=command_home, cwd=command_home, + env={**command_env_path, + "OPENREPOSHAPE_BIN_DIR": typed_command}) + + if state == "spells": + assert installer.returncode == 0, installer.stdout + installer.stderr + assert command.returncode == 0, command.stdout + command.stderr + net.installed(installer) + assert (command_bin / "openRepoShape").read_bytes() \ + == COMMAND.read_bytes() + assert f"preflight: {bash_path(net.bin)}/openRepoShape --preflight" \ + in installer.stdout.splitlines() + assert "openRepoShape: installed at " \ + f"{bash_path(command_bin)}/openRepoShape" \ + in command.stdout.splitlines() + assert typed_installer not in installer.stdout + assert typed_command not in command.stdout + return + net.refused(installer) + assert command.returncode == 2, command.stdout + command.stderr + for result in (installer, command): + said = " ".join(result.stderr.split()) + assert "is spelled for Windows" in said, said + assert "no cygpath here spells it the way bash does" in said, said + assert "Set $OPENREPOSHAPE_BIN_DIR to the directory as this shell " \ + "spells it, then run this" in said, said + assert net.calls() == [], net.calls() + assert not command_bin.exists() + assert typed_installer not in installer.stderr + + +#: An old launcher that answers as Git Bash makes every launcher answer: +#: the drive spelling and the bash spelling name one directory, so it places +#: itself where `cygpath -u` says whichever it was handed. `OLD_LAUNCHER` +#: takes the variable as typed, which off Windows is a directory of its own. +OLD_LAUNCHER_IN_GIT_BASH = OLD_LAUNCHER.replace( + 'mkdir -p "$OPENREPOSHAPE_BIN_DIR" && ' + 'cp "$0" "$OPENREPOSHAPE_BIN_DIR/openRepoShape"', + 'dir="$(cygpath -u -- "$OPENREPOSHAPE_BIN_DIR")" && mkdir -p "$dir" && ' + 'cp "$0" "$dir/openRepoShape"') +assert OLD_LAUNCHER_IN_GIT_BASH != OLD_LAUNCHER + + +@pytest.mark.parametrize("preflight_exit, old, ends", [ + pytest.param(0, False, "pinned to the release tag, and this machine is " + "ready. Run the same line again to upgrade to the newest " + "release.", id="done"), + pytest.param(1, False, "but the preflight did not pass (exit 1); it " + "names what is missing above.", id="preflight-not-ready"), + pytest.param(2, False, "but the preflight did not run to the end (exit " + "2); the REFUSED line above says why.", + id="preflight-did-not-run"), + pytest.param(0, True, "and this machine is ready, but the command does " + "not stay on v9.9.9 (see above).", id="does-not-stay"), +]) +def test_every_line_names_a_windows_spelled_bin_dir_the_way_bash_does( + tmp_path, preflight_exit, old, ends): + """`D:\\tools\\bin` was echoed as typed by `install:`, `preflight:` and + each of the four `done:` lines, where the command's own lines said + `/d/tools/bin` (#667). With `cygpath` spelling it, every one of them says + the directory the way bash does, and none says it as typed. The fourth + `done:` is the one after `install:`, which an old release earns.""" + net = Net(tmp_path) + net.bin = tmp_path / "drive" / "bin" + typed = typed_bin_dir(net.bin, "backslash") + target = f"{bash_path(net.bin)}/openRepoShape" + put_cygpath(net.path, typed, "spells", bash_path(net.bin)) + net.config["preflight_exit"] = preflight_exit + if old: + launcher = net.root / "old-openRepoShape" + write_script(launcher, OLD_LAUNCHER_IN_GIT_BASH) + net.serve(RELEASE, launcher=launcher) + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, + "OPENREPOSHAPE_REPO": SLUG}) + assert result.returncode == (0 if preflight_exit == 0 and not old else 1), \ + result.stdout + result.stderr + lines = result.stdout.splitlines() + assert f"preflight: {target} --preflight" in lines, result.stdout + assert lines[-1] == f"done: openRepoShape {RELEASE} is installed at " \ + f"{target}{' ' if old else ', '}{ends}", result.stdout + if old: + assert any(line.startswith(f"install: {target} does not stay on " + f"{RELEASE}: ") for line in lines), \ + result.stdout + assert typed not in result.stdout + + +def test_the_symlink_check_looks_at_the_directory_as_bash_spells_it(tmp_path): + """The link check names a target it can find: with the drive spelling + respelled first, pipx's link at the directory as bash spells it is the + one that is found and refused, before any request (#563, #667). Read as + typed, `D:\\tools\\bin/openRepoShape` is no link here, and the install + would go on and copy through the one that is.""" + net = Net(tmp_path) + net.bin = tmp_path / "drive" / "bin" + typed = typed_bin_dir(net.bin, "backslash") + put_cygpath(net.path, typed, "spells", bash_path(net.bin)) + packaged = tmp_path / "pipx" / "openRepoShape" + packaged.parent.mkdir(parents=True) + packaged.write_bytes(b"#!/usr/bin/env python3\n# the package's own\n") + net.bin.mkdir(parents=True) + (net.bin / "openRepoShape").symlink_to(packaged) + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed}) + assert result.returncode == 2, result.stdout + result.stderr + assert f"{bash_path(net.bin)}/openRepoShape is a symbolic link, so " \ + "nothing was installed" in " ".join(result.stderr.split()) + assert net.calls() == [], net.calls() + assert packaged.read_bytes() == b"#!/usr/bin/env python3\n# the package's own\n" + + #: A curl that RUNS, in either script: tab-indented, alone or as the value #: of an assignment. Usage text and refusals carry curl lines too, indented #: with spaces, and are printed, never run. diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index fe2d6759..d1559e8a 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -354,24 +354,31 @@ def test_install_says_how_to_put_it_on_path(tmp_path): WINDOWS_BIN_DIR = "D:\\tools\\bin" WINDOWS_BIN_DIR_SLASH = "D:/tools/bin" -#: The two spellings a person types, for a test to run over both. Git Bash -#: takes `D:\tools\bin` and `D:/tools/bin` alike, and `--install` has an arm -#: for each. -BIN_DIR_SPELLINGS = pytest.mark.parametrize("spelling", - ["backslash", "slash"]) +#: The spellings a person types, for a test to run over each. Git Bash takes +#: `D:\tools\bin` and `D:/tools/bin` alike, and `--install` has an arm for +#: each; a drive letter is a letter in either case (#667). +BIN_DIR_SPELLINGS = pytest.mark.parametrize( + "spelling", ["backslash", "slash", "lower-case"]) def typed_bin_dir(bin_dir: Path, spelling: str) -> str: """`$OPENREPOSHAPE_BIN_DIR` as a person types it for Windows, in - `spelling`, "backslash" or "slash". Off Windows it is `D:\\tools\\bin` - or `D:/tools/bin`, which name no directory of their own. On Windows it - is `bin_dir` itself, a real drive path under the test's directory, with - `str` for the backslash and `as_posix` for the slash: a command placed - where it should not be then lands where the test can see it, and never - on the machine's own `D:` drive.""" + `spelling`: "backslash", "slash", or "lower-case", a backslash path + whose drive letter is lower case. Off Windows it is `D:\\tools\\bin`, + `D:/tools/bin` or `d:\\tools\\bin`, which name no directory of their own. + On Windows it is `bin_dir` itself, a real drive path under the test's + directory, with `str` for the backslash and `as_posix` for the slash: a + command placed where it should not be then lands where the test can see + it, and never on the machine's own `D:` drive.""" if os.name == "nt": - return bin_dir.as_posix() if spelling == "slash" else str(bin_dir) - return WINDOWS_BIN_DIR_SLASH if spelling == "slash" else WINDOWS_BIN_DIR + if spelling == "slash": + return bin_dir.as_posix() + typed = str(bin_dir) + return typed[0].lower() + typed[1:] if spelling == "lower-case" \ + else typed + return {"slash": WINDOWS_BIN_DIR_SLASH, + "lower-case": WINDOWS_BIN_DIR.lower()}.get(spelling, + WINDOWS_BIN_DIR) def fake_cygpath(directory: Path, typed: str, answer: str | None) -> None: @@ -386,6 +393,25 @@ def fake_cygpath(directory: Path, typed: str, answer: str | None) -> None: (directory / "cygpath").chmod(0o755) +#: What `cygpath` does where a test builds a `$PATH` (#667): "spells" the +#: value the way bash does, "fails", answers a drive spelling "again", or is +#: not "there" at all. +CYGPATH_STATES = ("spells", "fails", "again", "there-is-none") + + +def put_cygpath(directory: Path, typed: str, state: str, spelled: str) -> None: + """A `cygpath` in `directory` in `state`: Git Bash's own where it + "spells" on Windows, a fake that answers `spelled` elsewhere.""" + if state == "there-is-none": + return + if state == "spells" and os.name == "nt": + directory.mkdir(exist_ok=True) + put_on_path(directory, "cygpath", shutil.which("cygpath")) + return + fake_cygpath(directory, typed, {"spells": spelled, "fails": None, + "again": WINDOWS_BIN_DIR_SLASH}[state]) + + @BIN_DIR_SPELLINGS @pytest.mark.parametrize("on_path", [False, True], ids=["not-on-path", "on-path"]) @@ -456,6 +482,183 @@ def test_a_windows_spelled_bin_dir_is_refused_where_no_cygpath_spells_it( assert [path.name for path in tmp_path.iterdir()] == ["machine-path"] +# --- the PATH note quotes the directory it prints (#667) --------------------- + +#: Windows names no file with a `"` or a `\` in it. +POSIX_NAME = pytest.mark.skipif( + os.name == "nt", reason="a Windows file name holds no \" or \\") + +#: One directory name per case, and the `export PATH` line `--install` prints +#: for it, with `{d}` for the directory's parent. The lines are written out, +#: not computed, because the launcher's own rule is what is being held: +#: today's line byte for byte where double quotes are enough (a space and an +#: apostrophe are), and single quotes, an apostrophe as `'\''`, where bash +#: would still read a `"`, `$`, backquote or `\` between double quotes, or a +#: `!` at an interactive prompt. +PATH_NOTE_NAMES = [ + pytest.param("plain", 'export PATH="{d}/plain:$PATH"', id="plain"), + pytest.param("two words", 'export PATH="{d}/two words:$PATH"', + id="a-space"), + pytest.param("it's", 'export PATH="{d}/it\'s:$PATH"', id="an-apostrophe"), + pytest.param("a$b", "export PATH='{d}/a$b':\"$PATH\"", id="a-dollar"), + pytest.param("a`b`", "export PATH='{d}/a`b`':\"$PATH\"", + id="a-backquote"), + pytest.param("a!b", "export PATH='{d}/a!b':\"$PATH\"", id="a-bang"), + pytest.param("it's $HOME", "export PATH='{d}/it'\\''s $HOME':\"$PATH\"", + id="an-apostrophe-and-a-dollar"), + pytest.param("a'b'c!", "export PATH='{d}/a'\\''b'\\''c!':\"$PATH\"", + id="two-apostrophes-and-a-bang"), + pytest.param('a"b', "export PATH='{d}/a\"b':\"$PATH\"", + marks=POSIX_NAME, id="a-double-quote"), + pytest.param("a\\b", "export PATH='{d}/a\\b':\"$PATH\"", + marks=POSIX_NAME, id="a-backslash"), + pytest.param("a\\", "export PATH='{d}/a\\':\"$PATH\"", + marks=POSIX_NAME, id="a-trailing-backslash"), + pytest.param('a"b$HOME c', "export PATH='{d}/a\"b$HOME c':\"$PATH\"", + marks=POSIX_NAME, id="the-issue-probe"), +] + + +def path_after(line: str) -> str: + """`$PATH` once bash has run `line` where it held `/sentinel`: the line + through `eval`, which reads it as a shell reads a pasted line. The line + travels in the environment and the script on stdin as bytes, which keep + a `"` and a `\\` whole on Windows.""" + ran = subprocess.run( + [BASH, "-s"], capture_output=True, check=False, + input=b'PATH=/sentinel\neval "$LINE"\nprintf %s "$PATH"\n', + env=command_env(setup_sh=None, env={"LINE": line})) + assert ran.returncode == 0, ran.stderr.decode("utf-8", "replace") + return ran.stdout.decode("utf-8") + + +@pytest.mark.parametrize("name, line", PATH_NOTE_NAMES) +def test_the_path_note_is_a_line_that_does_what_it_says(tmp_path, name, line): + """`--install` prints `export PATH=…` for a directory that is not on + `$PATH`, and a person pastes it. It put the directory between double + quotes as it was, so `a"b$HOME c` printed `export PATH="…/a"b$HOME + c:$PATH"`, which does not parse, and `$HOME` or a backquote in a name + was run or read, not kept (#667). The printed line is the one written + out for that name, and bash, handed it, puts exactly that directory + ahead of the old `$PATH`: no more, no less, nothing expanded.""" + parent = bash_path(tmp_path) + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={"OPENREPOSHAPE_BIN_DIR": f"{parent}/{name}"}) + assert result.returncode == 0, result.stdout + result.stderr + assert (tmp_path / name / "openRepoShape").is_file() + printed = [text for text in result.stdout.splitlines() + if text.startswith(" export PATH=")] + assert printed == [" " + line.format(d=parent)], result.stdout + assert path_after(printed[0]) == f"{parent}/{name}:/sentinel" + + +@pytest.mark.parametrize("name, line", PATH_NOTE_NAMES) +def test_export_path_line_quotes_any_directory_for_bash(name, line): + """The same rule on the function alone, so a `"` or a `\\` is held on + Windows too, where no file can be named with one and the test above + skips them: the function is lifted out of the launcher and handed the + directory in the environment, as `set -euo pipefail` runs it there.""" + directory = f"/srv/tools/{name}" + text = COMMAND.read_text(encoding="utf-8") + [function] = re.findall(r"^export_path_line\(\) \{\n.*?^\}\n", text, + re.M | re.S) + ran = subprocess.run( + [BASH, "-s"], capture_output=True, check=False, + input=("set -euo pipefail\n" + function + + 'export_path_line "$DIR"\n').encode("utf-8"), + env=command_env(setup_sh=None, env={"DIR": directory})) + assert ran.returncode == 0, ran.stderr.decode("utf-8", "replace") + assert ran.stdout.decode("utf-8") == line.format(d="/srv/tools") + assert path_after(ran.stdout.decode("utf-8")) == f"{directory}:/sentinel" + + +@POSIX_NAME +@pytest.mark.parametrize("name", ["a!b", "a$b", "plain"]) +def test_a_pasted_path_note_reaches_an_interactive_bash_as_printed( + tmp_path, name): + """A person pastes the line at a prompt, and an interactive bash reads + `!` inside double quotes as history (`event not found`), which a profile + file never does: so a directory holding one is single-quoted too. Run + through `bash -i`, the line sets `$PATH` to the directory and no + other.""" + parent = bash_path(tmp_path) + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={"OPENREPOSHAPE_BIN_DIR": f"{parent}/{name}"}) + assert result.returncode == 0, result.stdout + result.stderr + [line] = [text.strip() for text in result.stdout.splitlines() + if text.startswith(" export PATH=")] + reached = tmp_path / "reached" + script = f'PATH=/sentinel\n{line}\nprintf %s "$PATH" > "$OUT"\nexit\n' + subprocess.run( + [BASH, "--norc", "--noprofile", "-i"], capture_output=True, + check=False, input=script.encode("utf-8"), start_new_session=True, + env=command_env(setup_sh=None, env={ + "OUT": str(reached), "HISTFILE": os.devnull})) + assert reached.read_text(encoding="utf-8") == f"{parent}/{name}:/sentinel" + + +#: What `--install` refuses, before it places anything: a directory no line +#: about `$PATH` can name, and what its refusal says. `{d}` is `tmp_path` as +#: bash spells it. The newline's refusal never echoes the value. +UNNAMEABLE_BIN_DIRS = [ + pytest.param("{d}/a:b/bin", "holds a ':'", id="a-colon"), + pytest.param("{d}/bin:{d}/other", "holds a ':'", id="two-directories"), + pytest.param("D:tools", "holds a ':'", id="a-drive-relative-name"), + pytest.param("{d}/a\nforged/bin", + "holds a newline", id="a-newline"), + pytest.param("D:\\tools\nforged\\bin", + "holds a newline", id="a-drive-spelling-and-a-newline"), +] + + +@pytest.mark.parametrize("value, said", UNNAMEABLE_BIN_DIRS) +def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( + tmp_path, value, said): + """A `:` splits a `$PATH` and a newline breaks the note's line in two, so + no `export PATH` line could name such a directory (#667): refused with + exit 2 beside the Windows spelling's refusal, before a file is placed or + a directory made, and saying what to set instead. A newline is refused + first and the value is never echoed, so it cannot forge a line.""" + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ + "OPENREPOSHAPE_BIN_DIR": value.format(d=bash_path(tmp_path))}) + assert result.returncode == 2, result.stdout + result.stderr + said_in_full = " ".join(result.stderr.split()) + assert said in said_in_full, said_in_full + assert "Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to a directory " \ + "without one, then run this again." in said_in_full, said_in_full + assert "forged" not in result.stderr + assert result.stdout == "" + assert list(tmp_path.iterdir()) == [] + + +@POSIX_NAME +def test_the_default_directory_is_held_to_the_same_refusal(tmp_path): + """No variable at all, and a `$HOME` that holds a `:`: the directory is + `$HOME/.local/bin`, and it is refused the same, naming it.""" + home = tmp_path / "a:b" + result = run_cmd("--install", home=home, cwd=tmp_path) + assert result.returncode == 2, result.stdout + result.stderr + assert f"--install would place this command in '{home}/.local/bin', " \ + "which holds a ':'" in " ".join(result.stderr.split()) + assert list(tmp_path.iterdir()) == [] + + +@POSIX_NAME +def test_a_respelled_bin_dir_is_held_to_the_same_refusal(tmp_path): + """`cygpath` may answer a directory with a `:` of its own: the colon is + refused after the Windows spelling, not before it, or this one would + pass, and `D:\\tools\\bin` would be refused whatever `cygpath` said.""" + env = machine_path(tmp_path, ()) + env.pop("_log") + fake_cygpath(tmp_path / "machine-path", WINDOWS_BIN_DIR, "/d/a:b/bin") + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={**env, "OPENREPOSHAPE_BIN_DIR": WINDOWS_BIN_DIR}) + assert result.returncode == 2, result.stdout + result.stderr + assert "--install would place this command in '/d/a:b/bin', which " \ + "holds a ':'" in " ".join(result.stderr.split()) + assert [path.name for path in tmp_path.iterdir()] == ["machine-path"] + + def test_install_from_a_file_never_calls_gh(tmp_path): """`--install` run from a file copies THOSE bytes. A machine with no `gh` — or no network — must still be able to install the command, so a `gh` on From 7569ffa8f32ae4d92327d8a9507561e86e145c6d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 10:40:00 +0000 Subject: [PATCH 2/8] install.sh and openRepoShape: both refuse a `$OPENREPOSHAPE_BIN_DIR` holding a control character before they name it, and install.sh refuses a `:` before its fetch, so a carriage return or an escape is no longer echoed as typed (#760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #667's two checklist candidates, with rv667's NIT 2. `--install` refused a newline in `$OPENREPOSHAPE_BIN_DIR` and nothing else that is a control character: a carriage return, an escape, a tab or DEL placed the command and was printed as typed (`/tmp/x^MFORGED is not on $PATH`; an escape sequence reaches the terminal the same way). The newline arm is now `*[[:cntrl:]]*`, first in `install_commands`, ahead of the Windows spelling and the colon, and its refusal still says nothing of the value. It is `[[:cntrl:]]` and not `[![:print:]]`, so a directory with a letter the locale does not print (`café` under LC_ALL=C) is placed. Which C1 controls match is the locale's, measured with bash 5.2.21: C.UTF-8 matches U+0080 to U+009F (and U+2028, U+2029), C and POSIX match none, and the ASCII controls and DEL match in all three. install.sh learnt of a `:` or a newline only after its fetch: it fetched the launcher, ran `--install`, and said only that `--install` refused (exit 2); a release older than #667 placed the command under the directory. It now carries both arms of the command's `case`, copied as #667 copied the drive-letter one: the control-character arm first, before any line names the directory, and the colon arm after the Windows respelling, both before the symlink check and any request. Where the control-character refusal says nothing of the value, the colon refusal names the directory, as the command's does. Tests feed each value through install.sh and through the command (a colon, two directories, a drive-relative name, a newline, a carriage return, an escape, a tab, DEL, a drive spelling with a newline or an escape, a colon with an escape): exit 2, no request, nothing placed, neither stream holds a forged tail. A link in a directory holding a carriage return, a drive spelling, a colon with an escape, a letter under LC_ALL=C and LC_ALL=C.UTF-8, a C1 control under a UTF-8 locale, and the `$HOME` default pin the order and the width of the arms. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- install.sh | 38 ++++++- openRepoShape | 37 +++--- tests/test_install_sh.py | 131 ++++++++++++++++++++- tests/test_openreposhape_command.py | 171 ++++++++++++++++++++++++---- 4 files changed, 337 insertions(+), 40 deletions(-) diff --git a/install.sh b/install.sh index fa0fbd57..7517470f 100755 --- a/install.sh +++ b/install.sh @@ -42,9 +42,10 @@ # it): the lines above say which; # 2 refused before anything was installed: a version that is not a ref, no # release found, a fetch that failed, a file that is not the command, a -# command already there as a symbolic link (pipx's, #563), or a +# command already there as a symbolic link (pipx's, #563), a # `$OPENREPOSHAPE_BIN_DIR` spelled for Windows where no `cygpath` can -# spell it the way bash does (#667). +# spell it the way bash does (#667), or a directory holding a control +# character or a `:`, which no line about `$PATH` can name (#760). # git-filter-repo never changes it: only adopting needs it, and # `adopt-project.py` refuses with its own install command where it is missing. # What it changes is the summary (#595): `adoption:` says whether it is there @@ -365,6 +366,24 @@ main() { die "curl is not installed, and this installer fetches with it. Install curl, or install by hand with: gh api repos/$REPO/contents/openRepoShape -H 'Accept: application/vnd.github.raw' | bash -s -- --install" bin_dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" + # A DIRECTORY HOLDING A CONTROL CHARACTER IS REFUSED FIRST (#760), before + # any line below names it: a newline breaks a line in two, a carriage + # return rewrites it on the terminal and an escape is the terminal's to + # act on, and the `install:`, `preflight:` and `done:` lines would print + # the directory as typed. The command refuses the same value, but only + # from the release that added the check, and only after this file had + # fetched it. The `case` is `install_commands`'s in `openRepoShape`, + # copied for the reason the drive-letter one below is, and the refusal + # says nothing of the value. `[[:cntrl:]]` and not `[![:print:]]`, which + # `checked_repo` uses: a directory with a letter the locale does not + # print (`café` under LC_ALL=C) is no fault of the directory's. Which C1 + # controls (U+0080 to U+009F) match is the locale's, measured with bash + # 5.2.21: C.UTF-8 matches every one of them, C and POSIX none. + case "$bin_dir" in + *[[:cntrl:]]*) + die "the directory to install into holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." + ;; + esac # A WINDOWS SPELLING IS SPELLED THE WAY BASH SPELLS IT, THE COMMAND'S OWN # RULE (#667, after #649), before it is named in a line or looked at: a # value typed `D:\tools\bin` was echoed as typed, in the `install:`, @@ -375,7 +394,7 @@ main() { # anything is asked of the network. The `case` is `install_commands`'s in # `openRepoShape`, copied: this file arrives alone down a pipe, and # `tests/test_install_sh.py` feeds one value through both. The refusal - # does not echo the value: it may hold a newline that forges a line. + # does not echo the value. case "$bin_dir" in [A-Za-z]:/* | [A-Za-z]:\\*) spelled="" @@ -390,6 +409,19 @@ main() { bin_dir="$spelled" ;; esac + # A `:` SPLITS A `$PATH` WHATEVER THE QUOTING (#760), so no line about + # `$PATH` can name such a directory; the command refuses it from the + # release that added the check (#667), and this file learnt of it only + # after the fetch. Checked after the Windows spelling, as the command + # checks it: a drive spelling holds a `:` by definition, is respelled + # without one, and was refused above where it could not be. The + # control-character arm is above, so the directory is named here, as the + # command names it, and no value that can forge a line gets this far. + case "$bin_dir" in + *:*) + die "install.sh would place openRepoShape in '$bin_dir', which holds a ':', and a ':' is what splits a \$PATH into its directories, so no line about \$PATH can name it. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." + ;; + esac # A LINK IS NOT A PLACE TO INSTALL TO (#563), and this installer says so # itself, before anything is asked of the network. pipx places the diff --git a/openRepoShape b/openRepoShape index da550aee..6b757607 100755 --- a/openRepoShape +++ b/openRepoShape @@ -411,9 +411,10 @@ record_ref() { # nothing is read but the quote itself, which is written `'\''`. Every other # directory keeps the line this command has always printed, byte for byte: # README says `export PATH=…`, and a space or an apostrophe is safe in -# double quotes. A `:` splits a `$PATH` whatever the quoting, and a newline -# breaks the printed line in two, so `install_commands` refuses both before -# this is asked. +# double quotes. A `:` splits a `$PATH` whatever the quoting, and a control +# character (a newline breaks the printed line in two, an escape is the +# terminal's to act on) is no name a line can print as it is, so +# `install_commands` refuses both before this is asked. export_path_line() { local dir="$1" backslash='\' quote="'" rest quoted="" case "$dir" in @@ -444,17 +445,27 @@ export_path_line() { install_commands() { local dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" local name target source verb placed=0 recording=0 record spelled="" - # NO LINE ABOUT `$PATH` CAN NAME A DIRECTORY HOLDING A NEWLINE OR A `:` - # (#667): the colon is what splits a `$PATH`, and a newline breaks the - # line the note prints in two. Refused before anything is placed, as the - # Windows spelling below is. The newline goes first, and its refusal says - # nothing of the value, so that no refusal after it echoes one that can - # forge a line of output. The colon is checked after the Windows - # spelling, which holds one by definition, is respelled without one, and - # refuses what it cannot respell. + # NO LINE ABOUT `$PATH` CAN NAME A DIRECTORY HOLDING A CONTROL CHARACTER + # OR A `:` (#667, #760): the colon is what splits a `$PATH`; a newline + # breaks the line the note prints in two; a carriage return rewrites the + # line on the terminal (`/tmp/x^MFORGED is not on $PATH`) and an escape + # is the terminal's to act on, in the note and in every refusal that + # names the directory. Refused before anything is placed, as the Windows + # spelling below is. The control characters go first, and their refusal + # says nothing of the value, so that no refusal after it echoes one that + # can forge a line of output. It is `[[:cntrl:]]` and not + # `[![:print:]]`, which is what this file holds the repository and the + # ref to: a name with a letter the locale does not print, as `café` + # under LC_ALL=C, is no fault of the directory's. Which C1 controls + # (U+0080 to U+009F) match is the locale's, measured with bash 5.2.21: + # C.UTF-8 matches every one of them, and so U+2028 and U+2029; C and + # POSIX match none, and the ASCII controls and DEL match in all three. + # The colon is checked after the Windows spelling, which holds one by + # definition, is respelled without one, and refuses what it cannot + # respell. case "$dir" in - *$'\n'*) - die "--install would place this command in a directory whose name holds a newline, which no line about \$PATH can name: it would break in two. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." + *[[:cntrl:]]*) + die "--install would place this command in a directory whose name holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." ;; esac # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index bb0bd660..fa13b59a 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -46,11 +46,18 @@ import pytest from conftest import BASH, BASH_SKIP, REPO, bash_path -from test_openreposhape_command import (BIN_DIR_SPELLINGS, COMMAND, - CYGPATH_STATES, INSTALL_NEEDS, +from test_openreposhape_command import (BIN_DIR_SPELLINGS, C1_CSI, COMMAND, + COMMAND_REFUSAL_ENDS, CYGPATH_STATES, + HOLDS_A_COLON, + HOLDS_A_CONTROL_CHARACTER, + INSTALL_NEEDS, NO_FILE_NAMED_SO, + UNNAMEABLE_BIN_DIRS, + assert_a_bin_dir_is_refused, command_env, machine_path, put_cygpath, put_on_path, run_cmd, - typed_bin_dir, write_script) + typed_bin_dir, + utf8_locale_with_c1_controls, + write_script) INSTALLER = REPO / "install.sh" @@ -839,6 +846,124 @@ def test_the_symlink_check_looks_at_the_directory_as_bash_spells_it(tmp_path): assert packaged.read_bytes() == b"#!/usr/bin/env python3\n# the package's own\n" +# --- a directory no line about `$PATH` can name (#760) ----------------------- + +#: How install.sh's refusals of one end: the command's remedy, for this file. +INSTALLER_REFUSAL_ENDS = ( + "Nothing was installed. Set $OPENREPOSHAPE_BIN_DIR to a directory " + "without one, then run this line again.") + + +@pytest.mark.parametrize("value, said", UNNAMEABLE_BIN_DIRS) +def test_the_installer_refuses_what_the_command_refuses_before_any_request( + tmp_path, value, said): + """ONE `$OPENREPOSHAPE_BIN_DIR`, a `:` or a control character, through + install.sh and through the command it installs (#760). + + The command refuses it with exit 2, but install.sh learnt of it only + after its fetch, said only "the openRepoShape's --install + refused (exit 2)", and a release older than the refusal placed the + command under it and printed it as typed. So install.sh carries a copy + of both `case` arms, and the two must never answer one value + differently: each is refused with exit 2 and the remedy, nothing is + placed or made, and install.sh has asked the network nothing. A colon's + refusal names the directory; a control character's never does.""" + net = Net(in_its_own_directory(tmp_path, "installer")) + command_home = in_its_own_directory(tmp_path, "command") + seen = {path.name for path in net.tmp_path.iterdir()} + typed_installer = value.format(d=bash_path(net.tmp_path)) + typed_command = value.format(d=bash_path(command_home)) + + installer = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed_installer}) + command = run_cmd("--install", home=command_home, cwd=command_home, + env={"OPENREPOSHAPE_BIN_DIR": typed_command}) + + assert net.calls() == [], net.calls() + assert_a_bin_dir_is_refused(installer, typed_installer, said, + INSTALLER_REFUSAL_ENDS) + assert_a_bin_dir_is_refused(command, typed_command, said, + COMMAND_REFUSAL_ENDS) + net.refused(installer) + assert {path.name for path in net.tmp_path.iterdir()} - seen \ + <= {"gh-config"} + assert list(command_home.iterdir()) == [] + + +@NO_FILE_NAMED_SO +def test_the_installer_holds_the_default_directory_to_the_colon_refusal( + tmp_path): + """No variable at all, and a `$HOME` that holds a `:`: the directory is + `$HOME/.local/bin`, and install.sh refuses it as the command does, + naming it (#667, #760).""" + net = Net(tmp_path) + home = tmp_path / "a:b" + result = net.run(env={"HOME": str(home)}) + assert net.calls() == [], net.calls() + assert_a_bin_dir_is_refused(result, f"{home}/.local/bin", HOLDS_A_COLON, + INSTALLER_REFUSAL_ENDS) + net.refused(result) + + +@NO_FILE_NAMED_SO +def test_a_control_character_is_refused_before_the_symlink_check_names_it( + tmp_path): + """The symlink refusal (#563) names `$target`, so a directory holding a + carriage return that has pipx's link in it would have install.sh print + the value as typed (#760): the control character is refused first, and + the link stays as it was.""" + net = Net(tmp_path) + bin_dir = tmp_path / "a\rforged" / "bin" + bin_dir.mkdir(parents=True) + link = bin_dir / "openRepoShape" + link.symlink_to(tmp_path / "nowhere") + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": str(bin_dir)}) + assert net.calls() == [], net.calls() + assert_a_bin_dir_is_refused(result, str(bin_dir), + HOLDS_A_CONTROL_CHARACTER, + INSTALLER_REFUSAL_ENDS) + assert link.is_symlink() + + +@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " + "environment into Git Bash is not run here") +@pytest.mark.parametrize("locale", ["C", "C.UTF-8"]) +def test_the_installer_places_a_directory_with_an_unprinted_letter( + tmp_path, locale): + """The refusal is `[[:cntrl:]]` and not `[![:print:]]`, which + `checked_repo` uses (#760): under `LC_ALL=C` the two bytes of an e with + an acute accent are not printable, so the second would refuse a + directory called `caf` and that letter for a fault it has not got. It + is installed into, in either locale, and named as typed.""" + net = Net(tmp_path) + net.bin = tmp_path / ("caf" + chr(0xE9)) + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": bash_path(net.bin), + "LC_ALL": locale}) + assert result.returncode == 0, result.stdout + result.stderr + net.installed(result) + assert f"preflight: {bash_path(net.bin)}/openRepoShape --preflight" \ + in result.stdout.splitlines() + + +@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " + "environment into Git Bash is not run here") +def test_the_installer_refuses_a_c1_control_where_the_locale_reads_it_as_one( + tmp_path): + """U+009B, the C1 control sequence introducer, under a UTF-8 locale in + which `[[:cntrl:]]` matches it (#760); under `LC_ALL=C` it is not + matched, a limit the comment above the arm states.""" + locale = utf8_locale_with_c1_controls() + if locale is None: + pytest.skip("no UTF-8 locale here in which bash reads U+009B as a " + "control character") + net = Net(tmp_path) + typed = f"{bash_path(tmp_path)}/a{C1_CSI}forged/bin" + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) + assert net.calls() == [], net.calls() + assert_a_bin_dir_is_refused(result, typed, HOLDS_A_CONTROL_CHARACTER, + INSTALLER_REFUSAL_ENDS) + net.refused(result) + + #: A curl that RUNS, in either script: tab-indented, alone or as the value #: of an assignment. Usage text and refusals carry curl lines too, indented #: with spaces, and are printed, never run. diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index d1559e8a..790bae53 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -597,40 +597,169 @@ def test_a_pasted_path_note_reaches_an_interactive_bash_as_printed( assert reached.read_text(encoding="utf-8") == f"{parent}/{name}:/sentinel" +#: Control characters, typed as their numbers: no escape sequence is written +#: into this file, because a tool that writes it may write the raw character +#: (U+009B is the C1 "control sequence introducer" a terminal runs). +ESCAPE = chr(0x1B) +DELETE = chr(0x7F) +C1_CSI = chr(0x9B) + +HOLDS_A_COLON = "holds a ':'" +HOLDS_A_CONTROL_CHARACTER = "holds a control character" + +#: How `--install`'s two refusals end, and how install.sh's do (#760): the +#: remedy is the same sentence in both, less the verb. +COMMAND_REFUSAL_ENDS = ( + "Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to a directory without " + "one, then run this again.") + #: What `--install` refuses, before it places anything: a directory no line #: about `$PATH` can name, and what its refusal says. `{d}` is `tmp_path` as -#: bash spells it. The newline's refusal never echoes the value. +#: bash spells it. A control character is refused FIRST and its refusal +#: never echoes the value, so every control value ends in `forged`, which +#: shows in no output when the refusal is right; a colon's refusal names the +#: directory, which holds no control character by then (#667, #760). UNNAMEABLE_BIN_DIRS = [ - pytest.param("{d}/a:b/bin", "holds a ':'", id="a-colon"), - pytest.param("{d}/bin:{d}/other", "holds a ':'", id="two-directories"), - pytest.param("D:tools", "holds a ':'", id="a-drive-relative-name"), - pytest.param("{d}/a\nforged/bin", - "holds a newline", id="a-newline"), - pytest.param("D:\\tools\nforged\\bin", - "holds a newline", id="a-drive-spelling-and-a-newline"), + pytest.param("{d}/a:b/bin", HOLDS_A_COLON, id="a-colon"), + pytest.param("{d}/bin:{d}/other", HOLDS_A_COLON, id="two-directories"), + pytest.param("D:tools", HOLDS_A_COLON, id="a-drive-relative-name"), + pytest.param("{d}/a\nforged/bin", HOLDS_A_CONTROL_CHARACTER, + id="a-newline"), + pytest.param("{d}/a\rforged/bin", HOLDS_A_CONTROL_CHARACTER, + id="a-carriage-return"), + pytest.param("{d}/a" + ESCAPE + "[31mforged/bin", + HOLDS_A_CONTROL_CHARACTER, id="an-escape"), + pytest.param("{d}/a\tforged/bin", HOLDS_A_CONTROL_CHARACTER, + id="a-tab"), + pytest.param("{d}/a" + DELETE + "forged/bin", HOLDS_A_CONTROL_CHARACTER, + id="a-delete"), + pytest.param("D:\\tools\nforged\\bin", HOLDS_A_CONTROL_CHARACTER, + id="a-drive-spelling-and-a-newline"), + pytest.param("D:\\tools" + ESCAPE + "forged\\bin", + HOLDS_A_CONTROL_CHARACTER, + id="a-drive-spelling-and-an-escape"), + pytest.param("{d}/a:b" + ESCAPE + "forged/bin", + HOLDS_A_CONTROL_CHARACTER, id="a-colon-and-an-escape"), ] +#: Windows names no file with a control character in it, or with a `:` +#: beyond the drive's. +NO_FILE_NAMED_SO = pytest.mark.skipif( + os.name == "nt", reason="a Windows file name holds no control character") -@pytest.mark.parametrize("value, said", UNNAMEABLE_BIN_DIRS) -def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( - tmp_path, value, said): - """A `:` splits a `$PATH` and a newline breaks the note's line in two, so - no `export PATH` line could name such a directory (#667): refused with - exit 2 beside the Windows spelling's refusal, before a file is placed or - a directory made, and saying what to set instead. A newline is refused - first and the value is never echoed, so it cannot forge a line.""" - result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ - "OPENREPOSHAPE_BIN_DIR": value.format(d=bash_path(tmp_path))}) + +def assert_a_bin_dir_is_refused(result, typed: str, said: str, + ending: str) -> None: + """`result` is a refusal, exit 2, of `typed`, a directory no line about + `$PATH` can name (#667, #760), saying `said` and ending as `ending` + does. A colon's refusal names the directory; nothing else does, and no + control character, and no `forged` tail, reaches either stream.""" assert result.returncode == 2, result.stdout + result.stderr said_in_full = " ".join(result.stderr.split()) assert said in said_in_full, said_in_full - assert "Nothing was placed. Set $OPENREPOSHAPE_BIN_DIR to a directory " \ - "without one, then run this again." in said_in_full, said_in_full - assert "forged" not in result.stderr + assert ending in said_in_full, said_in_full + if said == HOLDS_A_COLON: + assert f"'{typed}', which {said}" in said_in_full, said_in_full + for stream in (result.stdout, result.stderr): + assert "forged" not in stream, stream + assert ESCAPE not in stream, stream + assert DELETE not in stream, stream + assert C1_CSI not in stream, stream assert result.stdout == "" + + +@pytest.mark.parametrize("value, said", UNNAMEABLE_BIN_DIRS) +def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( + tmp_path, value, said): + """A `:` splits a `$PATH`, a newline breaks the note's line in two, a + carriage return rewrites it on the terminal and an escape is the + terminal's to act on, so no `export PATH` line could name such a + directory (#667, #760): refused with exit 2 beside the Windows + spelling's refusal, before a file is placed or a directory made, and + saying what to set instead. Every control character is refused first, + ahead of the Windows spelling and the colon, and the value is never + echoed, so it cannot forge a line or reach the terminal.""" + typed = value.format(d=bash_path(tmp_path)) + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={"OPENREPOSHAPE_BIN_DIR": typed}) + assert_a_bin_dir_is_refused(result, typed, said, COMMAND_REFUSAL_ENDS) assert list(tmp_path.iterdir()) == [] +@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " + "environment into Git Bash is not run here") +@pytest.mark.parametrize("locale", ["C", "C.UTF-8"]) +def test_a_bin_dir_with_an_unprinted_letter_is_placed( + tmp_path, locale): + """The refusal is `[[:cntrl:]]` and not `[![:print:]]` (#760): under + `LC_ALL=C` the two bytes of an e with an acute accent are not + printable, so the second would refuse a directory called `caf` and + that letter for a fault it has not got. It is placed, in either + locale.""" + name = "caf" + chr(0xE9) + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ + "OPENREPOSHAPE_BIN_DIR": f"{bash_path(tmp_path)}/{name}", + "LC_ALL": locale}) + assert result.returncode == 0, result.stdout + result.stderr + assert (tmp_path / name / "openRepoShape").read_bytes() \ + == COMMAND.read_bytes() + + +def utf8_locale_with_c1_controls() -> str | None: + """A UTF-8 locale in which THIS bash reads U+009B as a control + character, or None: which of the C1 controls (U+0080 to U+009F) + `[[:cntrl:]]` matches is the locale's, so a test of one asks first. On + Linux with bash 5.2.21, `C.UTF-8` matches them all and `C` and `POSIX` + none (#760).""" + for locale in ("C.UTF-8", "en_US.UTF-8"): + probe = subprocess.run( + [BASH, "-c", 'case "$1" in *[[:cntrl:]]*) exit 0;; esac; exit 1', + "-", C1_CSI], capture_output=True, check=False, + env={**os.environ, "LC_ALL": locale}) + if probe.returncode == 0: + return locale + return None + + +@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " + "environment into Git Bash is not run here") +def test_a_c1_control_is_refused_where_the_locale_reads_it_as_one(tmp_path): + """U+009B, the C1 control sequence introducer, is two bytes in UTF-8, + and a terminal that reads them as one runs what follows (#760). Under a + UTF-8 locale `[[:cntrl:]]` matches it, so it is refused as an escape + is; under `LC_ALL=C` it is not (the bytes are not controls there), a + limit the launcher's comment states.""" + locale = utf8_locale_with_c1_controls() + if locale is None: + pytest.skip("no UTF-8 locale here in which bash reads U+009B as a " + "control character") + typed = f"{bash_path(tmp_path)}/a{C1_CSI}forged/bin" + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) + assert_a_bin_dir_is_refused(result, typed, HOLDS_A_CONTROL_CHARACTER, + COMMAND_REFUSAL_ENDS) + assert list(tmp_path.iterdir()) == [] + + +@NO_FILE_NAMED_SO +def test_a_control_character_is_refused_before_a_link_in_it_names_it( + tmp_path): + """The symlink refusal (#563) names `$dir/openRepoShape`, so a directory + holding a carriage return that already has pipx's link in it would + have the refusal print the value as typed (#760): the control character + is refused first, and the link stays as it was.""" + bin_dir = tmp_path / "a\rforged" / "bin" + bin_dir.mkdir(parents=True) + link = bin_dir / "openRepoShape" + link.symlink_to(tmp_path / "nowhere") + result = run_cmd("--install", home=tmp_path, cwd=tmp_path, + env={"OPENREPOSHAPE_BIN_DIR": str(bin_dir)}) + assert_a_bin_dir_is_refused(result, str(bin_dir), + HOLDS_A_CONTROL_CHARACTER, + COMMAND_REFUSAL_ENDS) + assert link.is_symlink() + + @POSIX_NAME def test_the_default_directory_is_held_to_the_same_refusal(tmp_path): """No variable at all, and a `$HOME` that holds a `:`: the directory is From efd061a7d3e0ad41b0d053e30013f2e7612ad5c7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 16:26:39 +0000 Subject: [PATCH 3/8] install.sh and openRepoShape: the control-character check runs under `LC_ALL=C` and names a C1 control's two bytes, so U+0080 to U+009F is refused in every locale and whatever else the value holds, where a UTF-8 locale let one through beside a byte that is not UTF-8 (#760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lane 1's review of #804 (finding 1): `*[[:cntrl:]]*` matches a C1 control only in a value that is valid UTF-8. One byte that is not (a lone 0xE9) makes bash match byte by byte, and then U+009B's two bytes (0xC2 0x9B) are not controls, as under `C`. Measured on 7569ffa8 with bash 5.2.21: under `LC_ALL=C.UTF-8` and `$OPENREPOSHAPE_BIN_DIR` set to `/caf`, 0xE9, U+009B, `31mFORGED/bin`, `openRepoShape --install` exited 0, placed the command and printed the value raw in four lines; install.sh, given the same value after a `:`, exited 2 with the colon refusal naming it, the C1 control in the line. Both files now run the check as `(LC_ALL=C; case ... *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']* ...)`: the ASCII controls and DEL byte by byte, and the two bytes of each C1 control named, so the answer is the same in every locale. Every value measured on 7569ffa8 and refused only under C.UTF-8, or not at all, is refused with exit 2, nothing placed, nothing echoed, and install.sh makes no request. `café` and U+00A0 are still placed. U+2028 and U+2029, which only a UTF-8 locale's `[[:cntrl:]]` matched, are now placed in every locale, and a test says so. Both comments say what the check does, and no longer say the C1 controls are the locale's. Tests: the C1 case, which skipped where no UTF-8 locale read U+009B as a control, is now five values (U+009B, the first and the last of the range, a 0xE9 byte before U+009B, and a colon beside that) in each of `C` and `C.UTF-8`, through the command and through install.sh. The placed-letter test gains U+00A0 (the byte after the range's last), U+2028 and U+2029. 18 of the 36 fail on 7569ffa8's code and the other 18 are guards; 18 mutants of the two code hunks are caught. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- install.sh | 19 +++--- openRepoShape | 25 ++++---- tests/test_install_sh.py | 63 ++++++++++--------- tests/test_openreposhape_command.py | 96 ++++++++++++++++++----------- 4 files changed, 121 insertions(+), 82 deletions(-) diff --git a/install.sh b/install.sh index 7517470f..09f3170f 100755 --- a/install.sh +++ b/install.sh @@ -376,14 +376,19 @@ main() { # copied for the reason the drive-letter one below is, and the refusal # says nothing of the value. `[[:cntrl:]]` and not `[![:print:]]`, which # `checked_repo` uses: a directory with a letter the locale does not - # print (`café` under LC_ALL=C) is no fault of the directory's. Which C1 - # controls (U+0080 to U+009F) match is the locale's, measured with bash - # 5.2.21: C.UTF-8 matches every one of them, C and POSIX none. - case "$bin_dir" in - *[[:cntrl:]]*) + # print (`café` under LC_ALL=C) is no fault of the directory's. The check + # runs under `LC_ALL=C` and names U+0080 to U+009F's two bytes (`\302`, + # then `\200` to `\237`), so a C1 control is refused in every locale, + # whatever else the value holds; U+2028 and U+2029 are refused in none. + # Left to the locale, `[[:cntrl:]]` matches a C1 control only in a value + # that is valid UTF-8: one byte that is not (a lone 0xE9) makes bash + # match byte by byte, and then U+009B's two bytes are not controls, as + # under `C` (measured with bash 5.2.21). The locale is `C` in a subshell, + # so that the match is byte by byte whatever a libc says of a byte in a + # UTF-8 locale, and the locale of every line below is as it was. + if (LC_ALL=C; case "$bin_dir" in *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then die "the directory to install into holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." - ;; - esac + fi # A WINDOWS SPELLING IS SPELLED THE WAY BASH SPELLS IT, THE COMMAND'S OWN # RULE (#667, after #649), before it is named in a line or looked at: a # value typed `D:\tools\bin` was echoed as typed, in the `install:`, diff --git a/openRepoShape b/openRepoShape index 6b757607..02db4169 100755 --- a/openRepoShape +++ b/openRepoShape @@ -456,18 +456,21 @@ install_commands() { # can forge a line of output. It is `[[:cntrl:]]` and not # `[![:print:]]`, which is what this file holds the repository and the # ref to: a name with a letter the locale does not print, as `café` - # under LC_ALL=C, is no fault of the directory's. Which C1 controls - # (U+0080 to U+009F) match is the locale's, measured with bash 5.2.21: - # C.UTF-8 matches every one of them, and so U+2028 and U+2029; C and - # POSIX match none, and the ASCII controls and DEL match in all three. - # The colon is checked after the Windows spelling, which holds one by - # definition, is respelled without one, and refuses what it cannot - # respell. - case "$dir" in - *[[:cntrl:]]*) + # under LC_ALL=C, is no fault of the directory's. The check runs under + # `LC_ALL=C` and names U+0080 to U+009F's two bytes (`\302`, then `\200` + # to `\237`), so a C1 control is refused in every locale, whatever else + # the value holds; U+2028 and U+2029 are refused in none. Left to the + # locale, `[[:cntrl:]]` matches a C1 control only in a value that is + # valid UTF-8: one byte that is not (a lone 0xE9) makes bash match byte + # by byte, and then U+009B's two bytes are not controls, as under `C` + # (measured with bash 5.2.21). The locale is `C` in a subshell, so that + # the match is byte by byte whatever a libc says of a byte in a UTF-8 + # locale, and the locale of every line below is as it was. The colon is + # checked after the Windows spelling, which holds one by definition, is + # respelled without one, and refuses what it cannot respell. + if (LC_ALL=C; case "$dir" in *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then die "--install would place this command in a directory whose name holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." - ;; - esac + fi # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with # `cygpath -u`, which Git Bash has, before anything is checked or placed: # `D:\tools\bin` becomes `/d/tools/bin`. Kept as typed, every line below diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index fa13b59a..19604b80 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -46,18 +46,18 @@ import pytest from conftest import BASH, BASH_SKIP, REPO, bash_path -from test_openreposhape_command import (BIN_DIR_SPELLINGS, C1_CSI, COMMAND, - COMMAND_REFUSAL_ENDS, CYGPATH_STATES, - HOLDS_A_COLON, +from test_openreposhape_command import (BIN_DIR_SPELLINGS, C1_BIN_DIRS, + COMMAND, COMMAND_REFUSAL_ENDS, + CYGPATH_STATES, HOLDS_A_COLON, HOLDS_A_CONTROL_CHARACTER, - INSTALL_NEEDS, NO_FILE_NAMED_SO, - UNNAMEABLE_BIN_DIRS, + INSTALL_NEEDS, LOCALES, + NO_FILE_NAMED_SO, + NO_NON_ASCII_INTO_GIT_BASH, + NOT_CONTROLS, UNNAMEABLE_BIN_DIRS, assert_a_bin_dir_is_refused, command_env, machine_path, put_cygpath, put_on_path, run_cmd, - typed_bin_dir, - utf8_locale_with_c1_controls, - write_script) + typed_bin_dir, write_script) INSTALLER = REPO / "install.sh" @@ -924,39 +924,44 @@ def test_a_control_character_is_refused_before_the_symlink_check_names_it( assert link.is_symlink() -@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " - "environment into Git Bash is not run here") -@pytest.mark.parametrize("locale", ["C", "C.UTF-8"]) +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("name", NOT_CONTROLS) def test_the_installer_places_a_directory_with_an_unprinted_letter( - tmp_path, locale): + tmp_path, locale, name): """The refusal is `[[:cntrl:]]` and not `[![:print:]]`, which `checked_repo` uses (#760): under `LC_ALL=C` the two bytes of an e with an acute accent are not printable, so the second would refuse a directory called `caf` and that letter for a fault it has not got. It - is installed into, in either locale, and named as typed.""" + is installed into, in either locale, and named as typed, and so is a + no-break space, whose first byte is the C1 controls' (`0xC2`) and + whose second is the one past them, and so are U+2028 and U+2029 (see + `NOT_CONTROLS`). The line is looked for among the lines split at a + newline and nothing else: `splitlines` splits at those two as well.""" net = Net(tmp_path) - net.bin = tmp_path / ("caf" + chr(0xE9)) + net.bin = tmp_path / name result = net.run(env={"OPENREPOSHAPE_BIN_DIR": bash_path(net.bin), "LC_ALL": locale}) assert result.returncode == 0, result.stdout + result.stderr net.installed(result) assert f"preflight: {bash_path(net.bin)}/openRepoShape --preflight" \ - in result.stdout.splitlines() - - -@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " - "environment into Git Bash is not run here") -def test_the_installer_refuses_a_c1_control_where_the_locale_reads_it_as_one( - tmp_path): - """U+009B, the C1 control sequence introducer, under a UTF-8 locale in - which `[[:cntrl:]]` matches it (#760); under `LC_ALL=C` it is not - matched, a limit the comment above the arm states.""" - locale = utf8_locale_with_c1_controls() - if locale is None: - pytest.skip("no UTF-8 locale here in which bash reads U+009B as a " - "control character") + in result.stdout.split("\n"), result.stdout + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("value", C1_BIN_DIRS) +def test_the_installer_refuses_a_c1_control_in_every_locale_before_any_request( + tmp_path, value, locale): + """A C1 control (U+0080 to U+009F) in the directory, under `C` and + under `C.UTF-8`, alone, after a byte that is not UTF-8, and after a + colon (#760): exit 2 with the control-character refusal, no request, + and nothing of the value in either stream. Under a UTF-8 locale a byte + that is not UTF-8 anywhere in the value makes bash match byte by byte, + and `[[:cntrl:]]` then matches neither byte of U+009B, as it matches + neither under `C`.""" net = Net(tmp_path) - typed = f"{bash_path(tmp_path)}/a{C1_CSI}forged/bin" + typed = value.format(d=bash_path(tmp_path)) result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) assert net.calls() == [], net.calls() assert_a_bin_dir_is_refused(result, typed, HOLDS_A_CONTROL_CHARACTER, diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index 790bae53..49b6247d 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -603,6 +603,9 @@ def test_a_pasted_path_note_reaches_an_interactive_bash_as_printed( ESCAPE = chr(0x1B) DELETE = chr(0x7F) C1_CSI = chr(0x9B) +#: A lone 0xE9 byte, which no UTF-8 text holds, as Python hands one to a +#: child's environment (the byte comes back through `surrogateescape`). +NOT_UTF8 = chr(0xDCE9) HOLDS_A_COLON = "holds a ':'" HOLDS_A_CONTROL_CHARACTER = "holds a control character" @@ -647,6 +650,14 @@ def test_a_pasted_path_note_reaches_an_interactive_bash_as_printed( NO_FILE_NAMED_SO = pytest.mark.skipif( os.name == "nt", reason="a Windows file name holds no control character") +#: The two locales a value is tried under: `C` reads one byte at a time, and +#: `C.UTF-8` reads UTF-8 text where it is valid (#760). +LOCALES = ["C", "C.UTF-8"] + +NO_NON_ASCII_INTO_GIT_BASH = pytest.mark.skipif( + os.name == "nt", reason="a non-ASCII value through the environment into " + "Git Bash is not run here") + def assert_a_bin_dir_is_refused(result, typed: str, said: str, ending: str) -> None: @@ -686,17 +697,32 @@ def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( assert list(tmp_path.iterdir()) == [] -@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " - "environment into Git Bash is not run here") -@pytest.mark.parametrize("locale", ["C", "C.UTF-8"]) +#: Names no arm refuses, in either locale (#760): an e with an acute accent; +#: U+00A0, a no-break space, whose bytes `0xC2 0xA0` come right after the +#: C1 range's last (`0xC2 0x9F`); and U+2028 and U+2029, which `[[:cntrl:]]` +#: alone matches under a UTF-8 locale and not under `C`. The arm runs under +#: `C`, so it answers alike in both, and neither is a control a terminal +#: acts on (a line that holds one prints as it is). +NOT_CONTROLS = [ + pytest.param("caf" + chr(0xE9), id="an-e-acute"), + pytest.param("a" + chr(0xA0) + "b", id="a-no-break-space"), + pytest.param("a" + chr(0x2028) + "b", id="a-line-separator"), + pytest.param("a" + chr(0x2029) + "b", id="a-paragraph-separator"), +] + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("name", NOT_CONTROLS) def test_a_bin_dir_with_an_unprinted_letter_is_placed( - tmp_path, locale): + tmp_path, locale, name): """The refusal is `[[:cntrl:]]` and not `[![:print:]]` (#760): under `LC_ALL=C` the two bytes of an e with an acute accent are not printable, so the second would refuse a directory called `caf` and that letter for a fault it has not got. It is placed, in either - locale.""" - name = "caf" + chr(0xE9) + locale, and so is a no-break space, whose first byte is the C1 + controls' (`0xC2`) and whose second is the one past them, and so are + U+2028 and U+2029, which only a UTF-8 locale's `[[:cntrl:]]` matches.""" result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ "OPENREPOSHAPE_BIN_DIR": f"{bash_path(tmp_path)}/{name}", "LC_ALL": locale}) @@ -705,35 +731,35 @@ def test_a_bin_dir_with_an_unprinted_letter_is_placed( == COMMAND.read_bytes() -def utf8_locale_with_c1_controls() -> str | None: - """A UTF-8 locale in which THIS bash reads U+009B as a control - character, or None: which of the C1 controls (U+0080 to U+009F) - `[[:cntrl:]]` matches is the locale's, so a test of one asks first. On - Linux with bash 5.2.21, `C.UTF-8` matches them all and `C` and `POSIX` - none (#760).""" - for locale in ("C.UTF-8", "en_US.UTF-8"): - probe = subprocess.run( - [BASH, "-c", 'case "$1" in *[[:cntrl:]]*) exit 0;; esac; exit 1', - "-", C1_CSI], capture_output=True, check=False, - env={**os.environ, "LC_ALL": locale}) - if probe.returncode == 0: - return locale - return None - - -@pytest.mark.skipif(os.name == "nt", reason="a non-ASCII value through the " - "environment into Git Bash is not run here") -def test_a_c1_control_is_refused_where_the_locale_reads_it_as_one(tmp_path): - """U+009B, the C1 control sequence introducer, is two bytes in UTF-8, - and a terminal that reads them as one runs what follows (#760). Under a - UTF-8 locale `[[:cntrl:]]` matches it, so it is refused as an escape - is; under `LC_ALL=C` it is not (the bytes are not controls there), a - limit the launcher's comment states.""" - locale = utf8_locale_with_c1_controls() - if locale is None: - pytest.skip("no UTF-8 locale here in which bash reads U+009B as a " - "control character") - typed = f"{bash_path(tmp_path)}/a{C1_CSI}forged/bin" +#: A C1 control (U+0080 to U+009F) is two bytes in UTF-8, `0xC2` and a byte +#: from `0x80` to `0x9F`, and a terminal that reads the two as one acts on +#: it (#760). Each is refused in every locale, and whatever else the value +#: holds: under a UTF-8 locale `[[:cntrl:]]` matches a C1 control only in a +#: value that is valid UTF-8, and a lone `0xE9` byte before it makes bash +#: match byte by byte. The first and the last of the range are named, so an +#: arm that stops one short at either end is caught, and a colon beside the +#: control shows the control arm comes first. +C1_BIN_DIRS = [ + pytest.param("{d}/a" + C1_CSI + "forged/bin", id="a-c1-csi"), + pytest.param("{d}/a" + chr(0x80) + "forged/bin", id="the-first-c1-control"), + pytest.param("{d}/a" + chr(0x9F) + "forged/bin", id="the-last-c1-control"), + pytest.param("{d}/caf" + NOT_UTF8 + C1_CSI + "31mforged/bin", + id="a-byte-that-is-not-utf-8-then-a-c1-csi"), + pytest.param("{d}/a:caf" + NOT_UTF8 + C1_CSI + "31mforged/bin", + id="a-colon-then-a-byte-that-is-not-utf-8-and-a-c1-csi"), +] + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("value", C1_BIN_DIRS) +def test_a_c1_control_is_refused_in_every_locale_whatever_else_the_value_holds( + tmp_path, value, locale): + """Refused as an escape is, exit 2, nothing placed, and never echoed: + under `C`, where `[[:cntrl:]]` reads the two bytes one at a time and + matches neither, and under `C.UTF-8`, where a byte that is not UTF-8 + anywhere in the value makes bash match byte by byte (#760).""" + typed = value.format(d=bash_path(tmp_path)) result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) assert_a_bin_dir_is_refused(result, typed, HOLDS_A_CONTROL_CHARACTER, From cc5ae88eaa2db116bc398e92690d27e819e14bc5 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 17:54:40 +0000 Subject: [PATCH 4/8] install.sh and openRepoShape: both refuse a `$OPENREPOSHAPE_BIN_DIR` holding a bidi control, a character of no width, a soft hyphen or U+2028 before any request and name it by its code point, so no line about `$PATH` shows the directory as another (#837) #804's check refuses the C0 and C1 controls and DEL, and passed every character a terminal draws as something other than itself, under `C` and `C.UTF-8` alike: the command placed itself under `/a` U+202E `forged/bin` and printed the value in four lines, and install.sh made its request. Laid out as a terminal lays out a line (fribidi), `~/` U+202E `.local/bin` reads `~/nib/lacol.`, `/opt/tools-1` U+200F `/2/bin` reads `/opt/tools-12//bin`, and `~/.local` U+200B `/bin` reads `~/.local/bin`. Both files now refuse, right after the control check, before the drive spelling, the colon, the symlink check and any request, a directory holding one of 29 code points, `MISDRAWN_CHARACTERS` in the launcher: U+00AD; the twelve bidirectional controls (U+061C, U+200E, U+200F, U+202A to U+202E, U+2066 to U+2069), as `repo_shape.spelled` escapes them (#391); U+2028 and U+2029, which end a line for every reader that keeps Unicode's line breaks, and which #804 left to a ruling; and U+200B, U+FEFF and U+2060 to U+206F, of no width, the whole block default-ignorable. U+200C and U+200D stay allowed: Persian, the Indic scripts and emoji sequences need them. So does every space. `misdrawn_characters` builds each one's UTF-8 from its code point with printf's octal escapes and looks for the bytes under `LC_ALL=C`, in the subshell #804's check uses, so one list says what is refused and what the refusal names, and no byte is read back as a number. The refusal says "holds a character a terminal draws as something other than itself (U+202E): one that is invisible, ends a line or reorders the text around it, so no line about $PATH can print the name as it reads", names every one found, and nothing else of the value. install.sh carries the list and the function as the launcher has them, and a test holds the two to one text. Tests: #804's C1 locale test in each file now runs `MISDRAWN_BIN_DIRS` too (each code point, two at once, after a byte that is not UTF-8, beside a colon, in a drive spelling), under `C` and `C.UTF-8`, through the command and install.sh, with no request; the link test takes U+202E; the lifted function names exactly its 29 over every code point that shares a first byte with them (896, on stdin, so it runs under Git Bash); `NOT_CONTROLS` drops U+2028 and U+2029 and gains a Persian word with U+200C, an emoji sequence with U+200D, U+2026 and U+FF21. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- install.sh | 50 +++++++- openRepoShape | 80 +++++++++++- tests/test_install_sh.py | 61 +++++----- tests/test_openreposhape_command.py | 181 +++++++++++++++++++++++----- 4 files changed, 311 insertions(+), 61 deletions(-) diff --git a/install.sh b/install.sh index 09f3170f..041e463b 100755 --- a/install.sh +++ b/install.sh @@ -45,7 +45,8 @@ # command already there as a symbolic link (pipx's, #563), a # `$OPENREPOSHAPE_BIN_DIR` spelled for Windows where no `cygpath` can # spell it the way bash does (#667), or a directory holding a control -# character or a `:`, which no line about `$PATH` can name (#760). +# character or a `:`, which no line about `$PATH` can name (#760), or +# a character a terminal draws as something other than itself (#837). # git-filter-repo never changes it: only adopting needs it, and # `adopt-project.py` refuses with its own install command where it is missing. # What it changes is the summary (#595): `adoption:` says whether it is there @@ -336,8 +337,44 @@ filter_repo_step() { adoption_not_ready "it is not installed" "$how $found" } +# THE CHARACTERS A TERMINAL DRAWS AS SOMETHING OTHER THAN THEMSELVES, AND +# WHICH OF THEM "$1" HOLDS (#837): the launcher's `MISDRAWN_CHARACTERS` and +# `misdrawn_characters`, copied as they are, for the reason the control +# check in `main` is copied: this file arrives alone down a pipe. The +# launcher says why these characters and no others, and +# `tests/test_openreposhape_command.py` holds the two copies to one text. +MISDRAWN_CHARACTERS=(00AD 061C 200B 200E-200F 2028-202E 2060-206F FEFF) + +misdrawn_characters() { + local value="$1" entry code last bytes name found="" + for entry in "${MISDRAWN_CHARACTERS[@]}"; do + code=$((0x${entry%-*})) + last=$((0x${entry#*-})) + while [ "$code" -le "$last" ]; do + if [ "$code" -lt 2048 ]; then + printf -v bytes '\\0%o\\0%o' $((0xC0 | (code >> 6))) \ + $((0x80 | (code & 63))) + else + printf -v bytes '\\0%o\\0%o\\0%o' $((0xE0 | (code >> 12))) \ + $((0x80 | ((code >> 6) & 63))) $((0x80 | (code & 63))) + fi + printf -v bytes '%b' "$bytes" + case "$value" in + *"$bytes"*) + printf -v name 'U+%04X' "$code" + found="${found:+$found, }$name" + ;; + *) ;; + esac + code=$((code + 1)) + done + done + printf '%s' "$found" +} + main() { local version="" asked launcher_url bin_dir spelled target shown status=0 pinned=1 + local misdrawn local machine="this machine is ready" # NOTHING A CHILD READS IS THIS SCRIPT. Piped, stdin is the pipe the @@ -379,7 +416,8 @@ main() { # print (`café` under LC_ALL=C) is no fault of the directory's. The check # runs under `LC_ALL=C` and names U+0080 to U+009F's two bytes (`\302`, # then `\200` to `\237`), so a C1 control is refused in every locale, - # whatever else the value holds; U+2028 and U+2029 are refused in none. + # whatever else the value holds; U+2028 and U+2029, which only a UTF-8 + # locale's `[[:cntrl:]]` matched, are the next check's (#837). # Left to the locale, `[[:cntrl:]]` matches a C1 control only in a value # that is valid UTF-8: one byte that is not (a lone 0xE9) makes bash # match byte by byte, and then U+009B's two bytes are not controls, as @@ -389,6 +427,14 @@ main() { if (LC_ALL=C; case "$bin_dir" in *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then die "the directory to install into holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." fi + # A CHARACTER A TERMINAL DRAWS AS SOMETHING OTHER THAN ITSELF IS REFUSED + # NEXT (#837), as the command refuses it, and before any request: the + # name would read as another directory's in every line below. The + # refusal names each one by its code point, because nobody can see it, + # and says nothing else of the value. + misdrawn="$(LC_ALL=C; misdrawn_characters "$bin_dir")" + [ -z "$misdrawn" ] || + die "the directory to install into holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." # A WINDOWS SPELLING IS SPELLED THE WAY BASH SPELLS IT, THE COMMAND'S OWN # RULE (#667, after #649), before it is named in a line or looked at: a # value typed `D:\tools\bin` was echoed as typed, in the `install:`, diff --git a/openRepoShape b/openRepoShape index b5cd1dd7..4da41c11 100755 --- a/openRepoShape +++ b/openRepoShape @@ -413,8 +413,9 @@ record_ref() { # README says `export PATH=…`, and a space or an apostrophe is safe in # double quotes. A `:` splits a `$PATH` whatever the quoting, and a control # character (a newline breaks the printed line in two, an escape is the -# terminal's to act on) is no name a line can print as it is, so -# `install_commands` refuses both before this is asked. +# terminal's to act on) is no name a line can print as it is, and nor is a +# character a terminal draws as something other than itself (#837), so +# `install_commands` refuses all three before this is asked. export_path_line() { local dir="$1" backslash='\' quote="'" rest quoted="" case "$dir" in @@ -442,9 +443,73 @@ export_path_line() { esac } +# THE CHARACTERS A TERMINAL DRAWS AS SOMETHING OTHER THAN THEMSELVES (#837), +# by code point, a range as its first and last. `install_commands` refuses a +# directory that holds one, because every line it prints about the directory +# would show another: laid out as a terminal lays out a line (fribidi), +# `~/` U+202E `.local/bin` reads `~/nib/lacol.`, `/opt/tools-1` U+200F +# `/2/bin` reads `/opt/tools-12//bin`, and `~/.local` U+200B `/bin` reads +# `~/.local/bin`, which is another directory's name. +# 00AD the soft hyphen, drawn as nothing or as a hyphen, never as +# what it is; +# 061C, 200E-200F, 202A-202E, 2066-2069 +# the twelve bidirectional controls, which reorder the text +# around them, and which `spelled` in scripts/repo_shape.py +# escapes for that reason (#391): a mark reorders the +# characters beside it, an embedding, override or isolate the +# rest of the line; +# 2028-2029 the line and paragraph separators, which end a line for any +# reader that keeps Unicode's line breaks (Python's +# `str.splitlines` among them); +# 200B, 2060-206F, FEFF +# the characters of no width: the zero-width space and +# no-break space, the word joiner, the invisible operators and +# the deprecated format controls. Unicode makes every code +# point from U+2060 to U+206F default-ignorable, so the range +# has no gap. +# U+200C and U+200D (ZERO WIDTH NON-JOINER and JOINER) are not here: a name +# in Persian, in an Indic script or with an emoji sequence holds them. Nor is +# any space (U+00A0, U+202F): it is drawn as the space it is. +MISDRAWN_CHARACTERS=(00AD 061C 200B 200E-200F 2028-202E 2060-206F FEFF) + +# WHICH OF THEM "$1" HOLDS, as `U+XXXX`, comma-separated, in that order, or +# nothing (#837). Run under `LC_ALL=C`, in a subshell, as the control check +# is, so the match is byte by byte in every locale: each character's UTF-8 +# is built from its code point with printf's octal escapes, which POSIX +# defines, and looked for in the value as bytes, and no byte is read back as +# a number. So one list says both what is refused and what the refusal +# names. install.sh carries the list and this function as they are here. +misdrawn_characters() { + local value="$1" entry code last bytes name found="" + for entry in "${MISDRAWN_CHARACTERS[@]}"; do + code=$((0x${entry%-*})) + last=$((0x${entry#*-})) + while [ "$code" -le "$last" ]; do + if [ "$code" -lt 2048 ]; then + printf -v bytes '\\0%o\\0%o' $((0xC0 | (code >> 6))) \ + $((0x80 | (code & 63))) + else + printf -v bytes '\\0%o\\0%o\\0%o' $((0xE0 | (code >> 12))) \ + $((0x80 | ((code >> 6) & 63))) $((0x80 | (code & 63))) + fi + printf -v bytes '%b' "$bytes" + case "$value" in + *"$bytes"*) + printf -v name 'U+%04X' "$code" + found="${found:+$found, }$name" + ;; + *) ;; + esac + code=$((code + 1)) + done + done + printf '%s' "$found" +} + install_commands() { local dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" local name target source verb placed=0 recording=0 record spelled="" + local misdrawn # NO LINE ABOUT `$PATH` CAN NAME A DIRECTORY HOLDING A CONTROL CHARACTER # OR A `:` (#667, #760): the colon is what splits a `$PATH`; a newline # breaks the line the note prints in two; a carriage return rewrites the @@ -459,7 +524,8 @@ install_commands() { # under LC_ALL=C, is no fault of the directory's. The check runs under # `LC_ALL=C` and names U+0080 to U+009F's two bytes (`\302`, then `\200` # to `\237`), so a C1 control is refused in every locale, whatever else - # the value holds; U+2028 and U+2029 are refused in none. Left to the + # the value holds; U+2028 and U+2029, which only a UTF-8 locale's + # `[[:cntrl:]]` matched, are the next check's (#837). Left to the # locale, `[[:cntrl:]]` matches a C1 control only in a value that is # valid UTF-8: one byte that is not (a lone 0xE9) makes bash match byte # by byte, and then U+009B's two bytes are not controls, as under `C` @@ -471,6 +537,14 @@ install_commands() { if (LC_ALL=C; case "$dir" in *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then die "--install would place this command in a directory whose name holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." fi + # A CHARACTER A TERMINAL DRAWS AS SOMETHING OTHER THAN ITSELF IS REFUSED + # NEXT (#837), for the control characters' reason and before the same + # lines: the name would read as another directory's. The refusal names + # each one by its code point, because nobody can see it, and says + # nothing else of the value. `misdrawn_characters` says which they are. + misdrawn="$(LC_ALL=C; misdrawn_characters "$dir")" + [ -z "$misdrawn" ] || + die "--install would place this command in a directory whose name holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with # `cygpath -u`, which Git Bash has, before anything is checked or placed: # `D:\tools\bin` becomes `/d/tools/bin`. Kept as typed, every line below diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index 19604b80..5450b248 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -46,14 +46,15 @@ import pytest from conftest import BASH, BASH_SKIP, REPO, bash_path -from test_openreposhape_command import (BIN_DIR_SPELLINGS, C1_BIN_DIRS, - COMMAND, COMMAND_REFUSAL_ENDS, +from test_openreposhape_command import (BIN_DIR_SPELLINGS, COMMAND, + COMMAND_REFUSAL_ENDS, CYGPATH_STATES, HOLDS_A_COLON, - HOLDS_A_CONTROL_CHARACTER, - INSTALL_NEEDS, LOCALES, - NO_FILE_NAMED_SO, + INSTALL_NEEDS, LINKED_BIN_DIRS, + LOCALES, NO_FILE_NAMED_SO, NO_NON_ASCII_INTO_GIT_BASH, - NOT_CONTROLS, UNNAMEABLE_BIN_DIRS, + NOT_CONTROLS, + REFUSED_IN_EVERY_LOCALE, + UNNAMEABLE_BIN_DIRS, assert_a_bin_dir_is_refused, command_env, machine_path, put_cygpath, put_on_path, run_cmd, @@ -905,21 +906,21 @@ def test_the_installer_holds_the_default_directory_to_the_colon_refusal( @NO_FILE_NAMED_SO -def test_a_control_character_is_refused_before_the_symlink_check_names_it( - tmp_path): +@pytest.mark.parametrize("name, said", LINKED_BIN_DIRS) +def test_a_bin_dir_is_refused_before_the_symlink_check_names_it( + tmp_path, name, said): """The symlink refusal (#563) names `$target`, so a directory holding a - carriage return that has pipx's link in it would have install.sh print - the value as typed (#760): the control character is refused first, and - the link stays as it was.""" + carriage return or U+202E that has pipx's link in it would have + install.sh print the value as typed (#760, #837): the value is refused + first, and the link stays as it was.""" net = Net(tmp_path) - bin_dir = tmp_path / "a\rforged" / "bin" + bin_dir = tmp_path / name / "bin" bin_dir.mkdir(parents=True) link = bin_dir / "openRepoShape" link.symlink_to(tmp_path / "nowhere") result = net.run(env={"OPENREPOSHAPE_BIN_DIR": str(bin_dir)}) assert net.calls() == [], net.calls() - assert_a_bin_dir_is_refused(result, str(bin_dir), - HOLDS_A_CONTROL_CHARACTER, + assert_a_bin_dir_is_refused(result, str(bin_dir), said, INSTALLER_REFUSAL_ENDS) assert link.is_symlink() @@ -935,9 +936,10 @@ def test_the_installer_places_a_directory_with_an_unprinted_letter( directory called `caf` and that letter for a fault it has not got. It is installed into, in either locale, and named as typed, and so is a no-break space, whose first byte is the C1 controls' (`0xC2`) and - whose second is the one past them, and so are U+2028 and U+2029 (see - `NOT_CONTROLS`). The line is looked for among the lines split at a - newline and nothing else: `splitlines` splits at those two as well.""" + whose second is the one past them, and so are the zero-width + non-joiner and joiner and the other names in `NOT_CONTROLS` (#837). + The line is looked for among the lines split at a newline and nothing + else, as `splitlines` splits at other characters too.""" net = Net(tmp_path) net.bin = tmp_path / name result = net.run(env={"OPENREPOSHAPE_BIN_DIR": bash_path(net.bin), @@ -950,22 +952,23 @@ def test_the_installer_places_a_directory_with_an_unprinted_letter( @NO_NON_ASCII_INTO_GIT_BASH @pytest.mark.parametrize("locale", LOCALES) -@pytest.mark.parametrize("value", C1_BIN_DIRS) -def test_the_installer_refuses_a_c1_control_in_every_locale_before_any_request( - tmp_path, value, locale): - """A C1 control (U+0080 to U+009F) in the directory, under `C` and - under `C.UTF-8`, alone, after a byte that is not UTF-8, and after a - colon (#760): exit 2 with the control-character refusal, no request, - and nothing of the value in either stream. Under a UTF-8 locale a byte - that is not UTF-8 anywhere in the value makes bash match byte by byte, - and `[[:cntrl:]]` then matches neither byte of U+009B, as it matches - neither under `C`.""" +@pytest.mark.parametrize("value, said", REFUSED_IN_EVERY_LOCALE) +def test_the_installer_refuses_in_every_locale_before_any_request( + tmp_path, value, said, locale): + """A C1 control (U+0080 to U+009F) in the directory (#760), or a + character a terminal draws as something other than itself (#837), + under `C` and under `C.UTF-8`, alone, after a byte that is not UTF-8, + and beside a colon: exit 2 with the refusal that says what it holds, + no request, and nothing of the value in either stream. Under a UTF-8 + locale a byte that is not UTF-8 anywhere in the value makes bash match + byte by byte, and `[[:cntrl:]]` then matches neither byte of U+009B, as + it matches neither under `C`; the second check looks for bytes in any + locale, and names what it found by its code point.""" net = Net(tmp_path) typed = value.format(d=bash_path(tmp_path)) result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) assert net.calls() == [], net.calls() - assert_a_bin_dir_is_refused(result, typed, HOLDS_A_CONTROL_CHARACTER, - INSTALLER_REFUSAL_ENDS) + assert_a_bin_dir_is_refused(result, typed, said, INSTALLER_REFUSAL_ENDS) net.refused(result) diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index 49b6247d..d840e23b 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -607,9 +607,27 @@ def test_a_pasted_path_note_reaches_an_interactive_bash_as_printed( #: child's environment (the byte comes back through `surrogateescape`). NOT_UTF8 = chr(0xDCE9) +#: THE CHARACTERS A TERMINAL DRAWS AS SOMETHING OTHER THAN THEMSELVES (#837), +#: by code point, as the launcher's `MISDRAWN_CHARACTERS` and install.sh's +#: copy list them: the soft hyphen, the twelve bidirectional controls, the +#: line and paragraph separators, and the characters of no width. Written +#: out here, not read from either file, so that a change to that list is a +#: change to this one too. +MISDRAWN = (0x00AD, 0x061C, 0x200B, 0x200E, 0x200F, *range(0x2028, 0x202F), + *range(0x2060, 0x2070), 0xFEFF) + HOLDS_A_COLON = "holds a ':'" HOLDS_A_CONTROL_CHARACTER = "holds a control character" + +def drawn_as_another(named: str) -> str: + """What both files' refusal says of a directory holding `named`: the + `U+XXXX` of each character of `MISDRAWN` in it, comma-separated (#837).""" + return ("holds a character a terminal draws as something other than " + f"itself ({named}): one that is invisible, ends a line or " + "reorders the text around it, so no line about $PATH can print " + "the name as it reads.") + #: How `--install`'s two refusals end, and how install.sh's do (#760): the #: remedy is the same sentence in both, less the verb. COMMAND_REFUSAL_ENDS = ( @@ -664,7 +682,8 @@ def assert_a_bin_dir_is_refused(result, typed: str, said: str, """`result` is a refusal, exit 2, of `typed`, a directory no line about `$PATH` can name (#667, #760), saying `said` and ending as `ending` does. A colon's refusal names the directory; nothing else does, and no - control character, and no `forged` tail, reaches either stream.""" + control character, no character of `MISDRAWN` and no `forged` tail + reaches either stream.""" assert result.returncode == 2, result.stdout + result.stderr said_in_full = " ".join(result.stderr.split()) assert said in said_in_full, said_in_full @@ -676,6 +695,8 @@ def assert_a_bin_dir_is_refused(result, typed: str, said: str, assert ESCAPE not in stream, stream assert DELETE not in stream, stream assert C1_CSI not in stream, stream + for code in MISDRAWN: + assert chr(code) not in stream, (f"U+{code:04X}", stream) assert result.stdout == "" @@ -697,17 +718,25 @@ def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( assert list(tmp_path.iterdir()) == [] -#: Names no arm refuses, in either locale (#760): an e with an acute accent; -#: U+00A0, a no-break space, whose bytes `0xC2 0xA0` come right after the -#: C1 range's last (`0xC2 0x9F`); and U+2028 and U+2029, which `[[:cntrl:]]` -#: alone matches under a UTF-8 locale and not under `C`. The arm runs under -#: `C`, so it answers alike in both, and neither is a control a terminal -#: acts on (a line that holds one prints as it is). +#: Names no check refuses, in either locale (#760, #837): an e with an acute +#: accent; U+00A0, a no-break space, whose bytes `0xC2 0xA0` come right +#: after the C1 range's last (`0xC2 0x9F`); a Persian word (letters) whose +#: two parts U+200C ZERO WIDTH NON-JOINER keeps apart, and an emoji sequence +#: (a woman technologist) that U+200D ZERO WIDTH JOINER makes, which that +#: script and that sequence need; and U+2026 and U+FF21, which start with +#: the bytes `0xE2` and `0xEF` that most of `MISDRAWN` start with. U+2028 +#: and U+2029 were here until #837 refused them with the rest of +#: `MISDRAWN`. NOT_CONTROLS = [ pytest.param("caf" + chr(0xE9), id="an-e-acute"), pytest.param("a" + chr(0xA0) + "b", id="a-no-break-space"), - pytest.param("a" + chr(0x2028) + "b", id="a-line-separator"), - pytest.param("a" + chr(0x2029) + "b", id="a-paragraph-separator"), + pytest.param("".join(map(chr, (0x0646, 0x0627, 0x0645, 0x0647, 0x200C, + 0x0647, 0x0627))), + id="a-persian-word-and-u+200c"), + pytest.param(chr(0x1F469) + chr(0x200D) + chr(0x1F4BB), + id="an-emoji-sequence-and-u+200d"), + pytest.param("a" + chr(0x2026) + "b", id="u+2026"), + pytest.param("a" + chr(0xFF21) + "b", id="u+ff21"), ] @@ -721,8 +750,10 @@ def test_a_bin_dir_with_an_unprinted_letter_is_placed( printable, so the second would refuse a directory called `caf` and that letter for a fault it has not got. It is placed, in either locale, and so is a no-break space, whose first byte is the C1 - controls' (`0xC2`) and whose second is the one past them, and so are - U+2028 and U+2029, which only a UTF-8 locale's `[[:cntrl:]]` matches.""" + controls' (`0xC2`) and whose second is the one past them; and so are + the zero-width non-joiner and joiner, which a script or an emoji + needs, and two characters drawn as themselves whose first bytes are + those of the characters #837 refuses (see `NOT_CONTROLS`).""" result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ "OPENREPOSHAPE_BIN_DIR": f"{bash_path(tmp_path)}/{name}", "LC_ALL": locale}) @@ -749,39 +780,135 @@ def test_a_bin_dir_with_an_unprinted_letter_is_placed( id="a-colon-then-a-byte-that-is-not-utf-8-and-a-c1-csi"), ] +#: Each of `MISDRAWN` in a directory, refused by the words that name it by +#: its code point (#837); then two at once, which are both named, in +#: `MISDRAWN`'s order; one after a byte that is not UTF-8; and one beside a +#: `:` and one in a drive spelling, whose own refusals name the directory, +#: so this one has to come before them. +MISDRAWN_BIN_DIRS = [ + *(pytest.param("{d}/a" + chr(code) + "forged/bin", + drawn_as_another(f"U+{code:04X}"), id=f"u+{code:04x}") + for code in MISDRAWN), + pytest.param("{d}/a" + chr(0x202E) + "b" + chr(0x200B) + "forged/bin", + drawn_as_another("U+200B, U+202E"), id="two-at-once"), + pytest.param("{d}/caf" + NOT_UTF8 + chr(0x202E) + "forged/bin", + drawn_as_another("U+202E"), + id="a-byte-that-is-not-utf-8-then-u+202e"), + pytest.param("{d}/a:b" + chr(0x202E) + "forged/bin", + drawn_as_another("U+202E"), id="a-colon-and-u+202e"), + pytest.param("D:\\tools" + chr(0x202E) + "forged\\bin", + drawn_as_another("U+202E"), + id="a-drive-spelling-and-u+202e"), +] + +#: Every value refused alike under `C` and under `C.UTF-8`, with what its +#: refusal says: each of `C1_BIN_DIRS` as a control character (#760), and +#: each of `MISDRAWN_BIN_DIRS` by what it holds (#837). +REFUSED_IN_EVERY_LOCALE = [ + *(pytest.param(*value.values, HOLDS_A_CONTROL_CHARACTER, id=value.id) + for value in C1_BIN_DIRS), + *MISDRAWN_BIN_DIRS, +] + @NO_NON_ASCII_INTO_GIT_BASH @pytest.mark.parametrize("locale", LOCALES) -@pytest.mark.parametrize("value", C1_BIN_DIRS) -def test_a_c1_control_is_refused_in_every_locale_whatever_else_the_value_holds( - tmp_path, value, locale): - """Refused as an escape is, exit 2, nothing placed, and never echoed: - under `C`, where `[[:cntrl:]]` reads the two bytes one at a time and - matches neither, and under `C.UTF-8`, where a byte that is not UTF-8 - anywhere in the value makes bash match byte by byte (#760).""" +@pytest.mark.parametrize("value, said", REFUSED_IN_EVERY_LOCALE) +def test_a_bin_dir_is_refused_in_every_locale_whatever_else_it_holds( + tmp_path, value, said, locale): + """Refused, exit 2, nothing placed, and never echoed, under `C` and + under `C.UTF-8`. A C1 control is refused as an escape is (#760): under + `C`, `[[:cntrl:]]` reads its two bytes one at a time and matches + neither, and under `C.UTF-8` a byte that is not UTF-8 anywhere in the + value makes bash match byte by byte. A character a terminal draws as + something other than itself is refused in words that name it by its + code point, because nobody can see it (#837), and its bytes are looked + for as bytes, so the locale changes nothing there either.""" typed = value.format(d=bash_path(tmp_path)) result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) - assert_a_bin_dir_is_refused(result, typed, HOLDS_A_CONTROL_CHARACTER, - COMMAND_REFUSAL_ENDS) + assert_a_bin_dir_is_refused(result, typed, said, COMMAND_REFUSAL_ENDS) assert list(tmp_path.iterdir()) == [] +def misdrawn_check(path: Path) -> str: + """`MISDRAWN_CHARACTERS` and `misdrawn_characters`, as the file at + `path` carries them (#837).""" + text = path.read_text(encoding="utf-8") + [listed] = re.findall(r"^MISDRAWN_CHARACTERS=\(.*?\)\n", text, re.M) + [function] = re.findall(r"^misdrawn_characters\(\) \{\n.*?^\}\n", text, + re.M | re.S) + return listed + function + + +def test_install_sh_carries_the_launchers_misdrawn_check_as_it_is(): + """install.sh arrives alone down a pipe, so it carries a copy of the + launcher's list and function, as it carries the control check (#837). + The two are one text, so neither file can refuse a directory the other + places, or name it otherwise.""" + assert misdrawn_check(REPO / "install.sh") == misdrawn_check(COMMAND) + + +#: Every code point whose UTF-8 starts with the bytes one of `MISDRAWN` +#: starts with: U+0080 to U+00BF (`0xC2`), U+0600 to U+063F (`0xD8`), +#: U+2000 to U+20FF (`0xE2 0x80` to `0xE2 0x83`) and U+FE00 to U+FFFF +#: (`0xEF 0xB8` to `0xEF 0xBF`). +SHARING_ITS_BYTES = (*range(0x80, 0xC0), *range(0x600, 0x640), + *range(0x2000, 0x2100), *range(0xFE00, 0x10000)) + + +@pytest.mark.parametrize("name", ["openRepoShape", "install.sh"]) +def test_misdrawn_characters_names_its_own_and_nothing_else(tmp_path, name): + """The function alone, lifted out of the file and run under `LC_ALL=C`, + as both call sites run it, over one directory name for each code point + that starts with the bytes one it refuses starts with (#837): it names + exactly the code points of `MISDRAWN`, each as `U+XXXX`, and nothing + for the rest, so a name that merely holds a byte such as `0xE2` or + `0xEF` is never refused. The names travel on stdin, as bytes, so this + runs under Git Bash too, where no such name crosses the environment.""" + script = tmp_path / "misdrawn.sh" + write_script(script, "LC_ALL=C\n" + misdrawn_check(REPO / name) + + 'while IFS= read -r -d "" value; do\n' + '\tmisdrawn_characters "$value"\n' + "\tprintf '\\n'\n" + "done\n") + ran = subprocess.run( + [BASH, bash_path(script)], capture_output=True, check=False, + input=b"".join(f"/a{chr(code)}b".encode("utf-8") + b"\0" + for code in SHARING_ITS_BYTES), + env=command_env(setup_sh=None)) + assert ran.returncode == 0, ran.stderr.decode("utf-8", "replace") + named = ran.stdout.decode("ascii").split("\n")[:-1] + assert dict(zip(SHARING_ITS_BYTES, named)) == { + code: f"U+{code:04X}" if code in MISDRAWN else "" + for code in SHARING_ITS_BYTES} + + +#: A directory that already holds pipx's link (#563), whose name holds a +#: carriage return (#760) or U+202E (#837), and what its refusal says. +LINKED_BIN_DIRS = [ + pytest.param("a\rforged", HOLDS_A_CONTROL_CHARACTER, + id="a-carriage-return"), + pytest.param("a" + chr(0x202E) + "forged", drawn_as_another("U+202E"), + id="u+202e"), +] + + @NO_FILE_NAMED_SO -def test_a_control_character_is_refused_before_a_link_in_it_names_it( - tmp_path): +@pytest.mark.parametrize("name, said", LINKED_BIN_DIRS) +def test_a_bin_dir_is_refused_before_a_link_in_it_names_it( + tmp_path, name, said): """The symlink refusal (#563) names `$dir/openRepoShape`, so a directory - holding a carriage return that already has pipx's link in it would - have the refusal print the value as typed (#760): the control character + holding a carriage return or U+202E that already has pipx's link in it + would have the refusal print the value as typed (#760, #837): the value is refused first, and the link stays as it was.""" - bin_dir = tmp_path / "a\rforged" / "bin" + bin_dir = tmp_path / name / "bin" bin_dir.mkdir(parents=True) link = bin_dir / "openRepoShape" link.symlink_to(tmp_path / "nowhere") result = run_cmd("--install", home=tmp_path, cwd=tmp_path, env={"OPENREPOSHAPE_BIN_DIR": str(bin_dir)}) - assert_a_bin_dir_is_refused(result, str(bin_dir), - HOLDS_A_CONTROL_CHARACTER, + assert_a_bin_dir_is_refused(result, str(bin_dir), said, COMMAND_REFUSAL_ENDS) assert link.is_symlink() From 4123b9e5368baa0646258e1cd52322050b09e6f0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 20:05:38 +0000 Subject: [PATCH 5/8] install.sh and openRepoShape: the repository, a version, a ref, an option and the redirect's location are refused unechoed for a misdrawn character too, the set takes in U+FFF9 to U+FFFB, U+1D173 to U+1D17A and U+E0001, and the refusal says a character may be drawn as another (#837) Fix round 1 on rv841's review of #841. The set (SHOULD 1). `MISDRAWN_CHARACTERS` gains FFF9-FFFB (the interlinear annotation characters), 1D173-1D17A (the musical beam, tie, slur and phrase controls) and E0001 (the deprecated language tag): format characters of no width (glibc `wcwidth` 0) that nothing needs, which the command placed and printed in four lines. The encoder builds the four-byte form too, with bash 3.2's arithmetic and `%b`'s `\0NNN`. Every other format character is left out for a reason the comment now gives: U+200C, U+200D and U+180E, which Persian, the Indic scripts, emoji and Mongolian need; the Egyptian hieroglyph and shorthand format controls, with which those scripts build their signs; the tags, the variation selectors, U+034F and U+1160, drawn as part of the character before them; the signs drawn over the digits after them, which take a cell; and every space and the blank Hangul fillers. The repository and the ref (SHOULD 2). #837's line named the `[![:print:]]` checks of both files: a UTF-8 locale's U+202E, U+200B, U+FEFF or U+00AD passes them, and the refusal after them echoed the value. Each now asks `misdrawn_characters` first, and refuses by the code point, saying nothing else of the value: `checked_repo`, `checked_version` (the version named and the newest release's tag), the option check and `latest_from_redirect`'s location in install.sh; `shape_checkout` and `ref_fault` (`$OPENREPOSHAPE_REF` and the record) in the command. First, because `[![:print:]]` is the locale's, so the words do not change with it. The words (NIT 3): "one that is invisible, is drawn as another character, ends a line or reorders the text around it", because U+00AD is drawn as a hyphen. The order (NIT 5): a tab before U+202E is refused as a control character, and a row holds it. Tests: `MISDRAWN` and the locale table take the twelve new code points and the tab row; the lifted sweep takes U+1D100 to U+1D1FF, U+E0000 to U+E007F and a sample of the planes past U+FFFF; `NOT_CONTROLS` takes a subdivision flag (tags U+E0020 to U+E007F); a test of each file refuses a value holding U+202E at every new site under `C` and `C.UTF-8`, through both entry points, with no request, nothing placed and neither stream holding it. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- install.sh | 54 ++++++-- openRepoShape | 98 +++++++++++---- tests/test_install_sh.py | 60 ++++++++- tests/test_openreposhape_command.py | 185 ++++++++++++++++++++++------ 4 files changed, 321 insertions(+), 76 deletions(-) diff --git a/install.sh b/install.sh index 041e463b..1428e0b8 100755 --- a/install.sh +++ b/install.sh @@ -123,8 +123,16 @@ USAGE # THE REPOSITORY, AS AN OWNER/NAME, because every URL below is built from it. # The launcher also takes a URL or a path in this variable, for the clone # setup.sh makes; there is no release at either, so this refuses them by -# name. A control character is refused before anything is echoed. +# name. A control character is refused before anything is echoed, and +# before it a character a terminal draws as something other than itself +# (#837), named by its code point: first, because `[![:print:]]` is the +# locale's, matching every byte past ASCII under `C` and none of those +# characters under a UTF-8 locale, where the refusal below would echo them. checked_repo() { + local misdrawn + misdrawn="$(LC_ALL=C; misdrawn_characters "$REPO")" + [ -z "$misdrawn" ] || + die "\$OPENREPOSHAPE_REPO holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it. It must be an owner/name on github.com, such as opensoft/openRepoShape." case "$REPO" in *[![:print:]]*) die "\$OPENREPOSHAPE_REPO carries a control character; it must be an owner/name on github.com, such as opensoft/openRepoShape." ;; esac @@ -140,14 +148,19 @@ checked_repo() { # THE LAUNCHER'S `checked_ref`, TRANSCRIBED: the version reaches a URL here # and `git checkout` once the command runs, so it is held to the same three # checks in the same order (length, then a control character, then the -# shape, which is the only one that echoes it), plus the empty value. Not -# shared, because this file arrives alone down a pipe. +# shape, which is the only one that echoes it), plus the empty value, and, +# between the length and the control character, a character a terminal +# draws as something other than itself, as the launcher's `ref_fault` has +# it (#837). Not shared, because this file arrives alone down a pipe. checked_version() { - local ref="$1" what="$2" + local ref="$1" what="$2" misdrawn [ -n "$ref" ] || die "$what is empty. Name a release, such as v1.0.1, or none for the newest." if [ "${#ref}" -gt 255 ]; then die "$what is ${#ref} characters long; a release tag, branch or commit is at most 255 characters." fi + misdrawn="$(LC_ALL=C; misdrawn_characters "$ref")" + [ -z "$misdrawn" ] || + die "$what holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it. No release tag, branch or commit holds one." case "$ref" in *[![:print:]]*) die "$what carries a control character, which is not a release tag, branch or commit." ;; esac @@ -196,10 +209,16 @@ latest_from_redirect() { return 0 ;; esac - case "$value" in - *[![:print:]]*) FAULT="it redirects to a location that names no release" ;; - *) FAULT="it redirects to $value, which names no release" ;; - esac + # The location is named only when it is text a terminal draws as + # it is: no control character, and no character it draws as + # something other than itself (#837). + FAULT="it redirects to a location that names no release" + if [ -z "$(LC_ALL=C; misdrawn_characters "$value")" ]; then + case "$value" in + *[![:print:]]*) ;; + *) FAULT="it redirects to $value, which names no release" ;; + esac + fi return 1 ;; esac @@ -341,9 +360,13 @@ filter_repo_step() { # WHICH OF THEM "$1" HOLDS (#837): the launcher's `MISDRAWN_CHARACTERS` and # `misdrawn_characters`, copied as they are, for the reason the control # check in `main` is copied: this file arrives alone down a pipe. The -# launcher says why these characters and no others, and +# launcher says why these characters and no others, and which forms of +# UTF-8 the function builds (two, three and four bytes), and # `tests/test_openreposhape_command.py` holds the two copies to one text. -MISDRAWN_CHARACTERS=(00AD 061C 200B 200E-200F 2028-202E 2060-206F FEFF) +# `main` asks it of the directory, of an option, `checked_repo` of the +# repository, `checked_version` of a version and `latest_from_redirect` +# of the location it reads, each before any line prints the value. +MISDRAWN_CHARACTERS=(00AD 061C 200B 200E-200F 2028-202E 2060-206F FEFF FFF9-FFFB 1D173-1D17A E0001) misdrawn_characters() { local value="$1" entry code last bytes name found="" @@ -354,9 +377,13 @@ misdrawn_characters() { if [ "$code" -lt 2048 ]; then printf -v bytes '\\0%o\\0%o' $((0xC0 | (code >> 6))) \ $((0x80 | (code & 63))) - else + elif [ "$code" -lt 65536 ]; then printf -v bytes '\\0%o\\0%o\\0%o' $((0xE0 | (code >> 12))) \ $((0x80 | ((code >> 6) & 63))) $((0x80 | (code & 63))) + else + printf -v bytes '\\0%o\\0%o\\0%o\\0%o' \ + $((0xF0 | (code >> 18))) $((0x80 | ((code >> 12) & 63))) \ + $((0x80 | ((code >> 6) & 63))) $((0x80 | (code & 63))) fi printf -v bytes '%b' "$bytes" case "$value" in @@ -387,6 +414,9 @@ main() { -h | --help) usage; exit 0 ;; --) shift; continue ;; -*) + misdrawn="$(LC_ALL=C; misdrawn_characters "$1")" + [ -z "$misdrawn" ] || + die "an option holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it. The one argument is a version, such as v1.0.1; with none, the newest release." case "$1" in *[![:print:]]*) die "an option carries a control character. The one argument is a version, such as v1.0.1; with none, the newest release." ;; esac @@ -434,7 +464,7 @@ main() { # and says nothing else of the value. misdrawn="$(LC_ALL=C; misdrawn_characters "$bin_dir")" [ -z "$misdrawn" ] || - die "the directory to install into holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." + die "the directory to install into holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." # A WINDOWS SPELLING IS SPELLED THE WAY BASH SPELLS IT, THE COMMAND'S OWN # RULE (#667, after #649), before it is named in a line or looked at: a # value typed `D:\tools\bin` was echoed as typed, in the `install:`, diff --git a/openRepoShape b/openRepoShape index 4da41c11..a94e5192 100755 --- a/openRepoShape +++ b/openRepoShape @@ -444,14 +444,16 @@ export_path_line() { } # THE CHARACTERS A TERMINAL DRAWS AS SOMETHING OTHER THAN THEMSELVES (#837), -# by code point, a range as its first and last. `install_commands` refuses a -# directory that holds one, because every line it prints about the directory -# would show another: laid out as a terminal lays out a line (fribidi), +# by code point, a range as its first and last. A value holding one is +# refused wherever this command would print it, without printing it: +# `install_commands` refuses such a directory, `shape_checkout` such a +# repository and `ref_fault` such a ref. Every line that printed it would +# show another value: laid out as a terminal lays out a line (fribidi), # `~/` U+202E `.local/bin` reads `~/nib/lacol.`, `/opt/tools-1` U+200F # `/2/bin` reads `/opt/tools-12//bin`, and `~/.local` U+200B `/bin` reads # `~/.local/bin`, which is another directory's name. -# 00AD the soft hyphen, drawn as nothing or as a hyphen, never as -# what it is; +# 00AD the soft hyphen, drawn as a hyphen in a terminal cell and as +# nothing elsewhere, never as what it is; # 061C, 200E-200F, 202A-202E, 2066-2069 # the twelve bidirectional controls, which reorder the text # around them, and which `spelled` in scripts/repo_shape.py @@ -461,24 +463,49 @@ export_path_line() { # 2028-2029 the line and paragraph separators, which end a line for any # reader that keeps Unicode's line breaks (Python's # `str.splitlines` among them); -# 200B, 2060-206F, FEFF -# the characters of no width: the zero-width space and -# no-break space, the word joiner, the invisible operators and -# the deprecated format controls. Unicode makes every code -# point from U+2060 to U+206F default-ignorable, so the range -# has no gap. -# U+200C and U+200D (ZERO WIDTH NON-JOINER and JOINER) are not here: a name -# in Persian, in an Indic script or with an emoji sequence holds them. Nor is -# any space (U+00A0, U+202F): it is drawn as the space it is. -MISDRAWN_CHARACTERS=(00AD 061C 200B 200E-200F 2028-202E 2060-206F FEFF) +# 200B, 2060-206F, FEFF, FFF9-FFFB, 1D173-1D17A, E0001 +# the format characters of no width that nothing needs: the +# zero-width space and no-break space, the word joiner, the +# invisible operators, the deprecated format controls, the +# interlinear annotation characters, the musical beam, tie, +# slur and phrase controls, and the deprecated language tag. +# Unicode makes every code point from U+2060 to U+206F +# default-ignorable, so that range has no gap. +# Left out, each for its reason: +# * U+200C and U+200D (ZERO WIDTH NON-JOINER and JOINER): a name in +# Persian, in an Indic script or with an emoji sequence holds them; and +# U+180E (MONGOLIAN VOWEL SEPARATOR), which Mongolian needs as Persian +# needs U+200C; +# * the other format characters of no width, with which a script builds +# its signs: the Egyptian hieroglyph format controls (U+13430 to +# U+1343F), which join signs into a group, and the shorthand format +# controls (U+1BCA0 to U+1BCA3), which overlap and step Duployan signs; +# * the tags (U+E0020 to U+E007F), which make an emoji subdivision flag, +# the variation selectors (U+FE00 to U+FE0F, U+E0100 to U+E01EF), which +# choose how the character before them is drawn, and U+034F (COMBINING +# GRAPHEME JOINER), which keeps the characters around it one grapheme: +# each is drawn as part of the character before it; +# * U+1160 (HANGUL JUNGSEONG FILLER), which forms a syllable block with +# the consonant before it and is drawn as that block; +# * the format characters drawn over the digits after them (U+0600 to +# U+0605, U+06DD, U+070F, U+0890, U+0891, U+08E2, U+110BD, U+110CD), +# which take a cell and are drawn as themselves; +# * every space (U+00A0, U+202F, U+3000) and the blank Hangul fillers +# (U+115F, U+3164, U+FFA0): each is drawn as a blank cell, as a space +# is. +# Every entry is U+0080 or above: `misdrawn_characters` builds the two-, +# three- and four-byte forms of UTF-8 (U+0080 to U+10FFFF), and no other. +MISDRAWN_CHARACTERS=(00AD 061C 200B 200E-200F 2028-202E 2060-206F FEFF FFF9-FFFB 1D173-1D17A E0001) # WHICH OF THEM "$1" HOLDS, as `U+XXXX`, comma-separated, in that order, or # nothing (#837). Run under `LC_ALL=C`, in a subshell, as the control check # is, so the match is byte by byte in every locale: each character's UTF-8 -# is built from its code point with printf's octal escapes, which POSIX -# defines, and looked for in the value as bytes, and no byte is read back as -# a number. So one list says both what is refused and what the refusal -# names. install.sh carries the list and this function as they are here. +# is built from its code point with printf's octal escapes (`%b` reads +# `\0NNN`, as POSIX defines it), in two bytes below U+0800, three below +# U+10000 and four above, and looked for in the value as bytes, and no byte +# is read back as a number. So one list says both what is refused and what +# the refusal names. install.sh carries the list and this function as they +# are here. misdrawn_characters() { local value="$1" entry code last bytes name found="" for entry in "${MISDRAWN_CHARACTERS[@]}"; do @@ -488,9 +515,13 @@ misdrawn_characters() { if [ "$code" -lt 2048 ]; then printf -v bytes '\\0%o\\0%o' $((0xC0 | (code >> 6))) \ $((0x80 | (code & 63))) - else + elif [ "$code" -lt 65536 ]; then printf -v bytes '\\0%o\\0%o\\0%o' $((0xE0 | (code >> 12))) \ $((0x80 | ((code >> 6) & 63))) $((0x80 | (code & 63))) + else + printf -v bytes '\\0%o\\0%o\\0%o\\0%o' \ + $((0xF0 | (code >> 18))) $((0x80 | ((code >> 12) & 63))) \ + $((0x80 | ((code >> 6) & 63))) $((0x80 | (code & 63))) fi printf -v bytes '%b' "$bytes" case "$value" in @@ -544,7 +575,7 @@ install_commands() { # nothing else of the value. `misdrawn_characters` says which they are. misdrawn="$(LC_ALL=C; misdrawn_characters "$dir")" [ -z "$misdrawn" ] || - die "--install would place this command in a directory whose name holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." + die "--install would place this command in a directory whose name holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with # `cygpath -u`, which Git Bash has, before anything is checked or placed: # `D:\tools\bin` becomes `/d/tools/bin`. Kept as typed, every line below @@ -681,9 +712,19 @@ find_python() { # a `git clone` command line, where a leading `-` is an option rather than a # repository and a control character could forge a line of output before this # script has said a word. The refusal does not echo the value back. +# +# A CHARACTER A TERMINAL DRAWS AS SOMETHING OTHER THAN ITSELF IS REFUSED +# FIRST (#837), before the clone's refusal names `$REPO`, and in words that +# name it by its code point and nothing else of the value. First, because +# `[![:print:]]` is the locale's: under `C` it matches every byte past ASCII, +# and under a UTF-8 locale none of these characters, so after it the words +# would change with the locale. shape_checkout() { - local url + local url misdrawn url="$(shape_clone_url)" + misdrawn="$(LC_ALL=C; misdrawn_characters "$url")" + [ -z "$misdrawn" ] || + die "OPENREPOSHAPE_REPO holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it. This tool will not put it on a \`git clone\` command line. Set it to a URL or a path." case "$url" in -* | *[![:print:]]*) die "OPENREPOSHAPE_REPO is not a value this tool will put on a \`git clone\` command line: it starts with '-' or carries a control character. Set it to a URL or a path." ;; esac @@ -720,13 +761,24 @@ shape_checkout() { # refusing spelling. An EMPTY ref is a fault too, which no caller could reach # before the record: an empty variable is no ref at all, while an empty file # is one. +# +# A CHARACTER A TERMINAL DRAWS AS SOMETHING OTHER THAN ITSELF (#837) is a +# fault between the length and the control character, named by its code +# point and never echoed, as `shape_checkout` refuses one: before the +# control check, whose `[![:print:]]` is the locale's, so the words do not +# change with the locale, and before the shape, which echoes the value. ref_fault() { - local ref="$1" what="$2" + local ref="$1" what="$2" misdrawn if [ -z "$ref" ]; then printf '%s' "$what is empty, which is not a branch, tag or commit this tool will pass to \`git checkout\`." elif [ "${#ref}" -gt 255 ]; then printf '%s' "$what is ${#ref} characters long; a branch, tag or commit this tool will pass to \`git checkout\` is at most 255 characters." else + misdrawn="$(LC_ALL=C; misdrawn_characters "$ref")" + if [ -n "$misdrawn" ]; then + printf '%s' "$what holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it. No branch, tag or commit this tool will pass to \`git checkout\` holds one." + return 0 + fi case "$ref" in *[![:print:]]*) printf '%s' "$what carries a control character, which is not a branch, tag or commit this tool will pass to \`git checkout\`." ;; [!A-Za-z0-9]* | *..* | *.lock | *[!A-Za-z0-9._/-]*) printf '%s' "$what is '$ref', which is not a branch, tag or commit this tool will pass to \`git checkout\`: a ref starts with a letter or digit, uses only letters, digits, '.', '_', '/', '-', has no '..' and does not end in '.lock'." ;; diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index 5450b248..d35ece00 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -53,9 +53,11 @@ LOCALES, NO_FILE_NAMED_SO, NO_NON_ASCII_INTO_GIT_BASH, NOT_CONTROLS, - REFUSED_IN_EVERY_LOCALE, + REFUSED_IN_EVERY_LOCALE, RLO, UNNAMEABLE_BIN_DIRS, assert_a_bin_dir_is_refused, + assert_refused_saying, + drawn_as_another, command_env, machine_path, put_cygpath, put_on_path, run_cmd, typed_bin_dir, write_script) @@ -972,6 +974,62 @@ def test_the_installer_refuses_in_every_locale_before_any_request( net.refused(result) +# A version, an option, the repository, the newest release's tag or the +# location github.com redirects to, holding U+202E (#837). Each arranges +# `net` and returns the arguments, the environment, what the refusal says, +# and whether the value comes from the network, so that the run asks it. + +def a_version(net): + return (("v1" + RLO + "forged",), {}, + f"the version {drawn_as_another('U+202E')}.", False) + + +def an_option(net): + return (("-x" + RLO + "forged",), {}, + f"an option {drawn_as_another('U+202E')}.", False) + + +def the_repository(net): + return ((), {"OPENREPOSHAPE_REPO": "acme/x" + RLO + "forged"}, + f"$OPENREPOSHAPE_REPO {drawn_as_another('U+202E')}.", False) + + +def the_newest_tag(net): + net.config["redirects"][latest()] = tag_page("v1" + RLO + "forged") + return ((), {}, "the newest release's tag " + f"{drawn_as_another('U+202E')}.", True) + + +def a_location(net): + net.config["redirects"][latest()] = ( + f"https://github.com/{SLUG}/releases/" + RLO + "forged") + return ((), {}, "it redirects to a location that names no release", + True) + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("arrange", [a_version, an_option, the_repository, + the_newest_tag, a_location], + ids=lambda arrange: arrange.__name__) +def test_the_installer_refuses_a_value_drawn_as_another_unechoed( + net, arrange, locale): + """`checked_repo`, `checked_version`, the option check and the + redirect's reading held their value to `[![:print:]]`, which a UTF-8 + locale's U+202E passes, and the refusal or fault after it printed the + value as typed (#837). Each now looks for a character of `MISDRAWN` + first and names it by its code point, under `C` and `C.UTF-8` alike: + exit 2, nothing placed, nothing fetched, no request at all where the + value is not the network's, and neither stream holds the value.""" + args, env, said, asks = arrange(net) + result = net.run(*args, env={**env, "LC_ALL": locale}) + assert_refused_saying(result, said) + net.refused(result) + calls = net.calls() + assert not any("-fsSL" in call for call in calls), calls + assert bool(calls) is asks, calls + + #: A curl that RUNS, in either script: tab-indented, alone or as the value #: of an assignment. Usage text and refusals carry curl lines too, indented #: with spaces, and are printed, never run. diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index d840e23b..fe481d32 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -610,22 +610,34 @@ def test_a_pasted_path_note_reaches_an_interactive_bash_as_printed( #: THE CHARACTERS A TERMINAL DRAWS AS SOMETHING OTHER THAN THEMSELVES (#837), #: by code point, as the launcher's `MISDRAWN_CHARACTERS` and install.sh's #: copy list them: the soft hyphen, the twelve bidirectional controls, the -#: line and paragraph separators, and the characters of no width. Written -#: out here, not read from either file, so that a change to that list is a -#: change to this one too. +#: line and paragraph separators, and the format characters of no width +#: that nothing needs, three of them past U+FFFF. Written out here, not read +#: from either file, so that a change to that list is a change to this one +#: too. MISDRAWN = (0x00AD, 0x061C, 0x200B, 0x200E, 0x200F, *range(0x2028, 0x202F), - *range(0x2060, 0x2070), 0xFEFF) + *range(0x2060, 0x2070), 0xFEFF, *range(0xFFF9, 0xFFFC), + *range(0x1D173, 0x1D17B), 0xE0001) + +#: U+202E RIGHT-TO-LEFT OVERRIDE, the one of `MISDRAWN` the tests of each +#: place it is refused hold, after a `forged` tail's start. +RLO = chr(0x202E) HOLDS_A_COLON = "holds a ':'" HOLDS_A_CONTROL_CHARACTER = "holds a control character" def drawn_as_another(named: str) -> str: - """What both files' refusal says of a directory holding `named`: the - `U+XXXX` of each character of `MISDRAWN` in it, comma-separated (#837).""" + """What every refusal of a value holding `named` says of it, in both + files: the `U+XXXX` of each character of `MISDRAWN` in it, + comma-separated (#837). Each place goes on in words of its own.""" return ("holds a character a terminal draws as something other than " - f"itself ({named}): one that is invisible, ends a line or " - "reorders the text around it, so no line about $PATH can print " + f"itself ({named}): one that is invisible, is drawn as another " + "character, ends a line or reorders the text around it") + + +def bin_dir_drawn_as_another(named: str) -> str: + """The directory's refusal of `named`, in both files (#837).""" + return (drawn_as_another(named) + ", so no line about $PATH can print " "the name as it reads.") #: How `--install`'s two refusals end, and how install.sh's do (#760): the @@ -677,19 +689,14 @@ def drawn_as_another(named: str) -> str: "Git Bash is not run here") -def assert_a_bin_dir_is_refused(result, typed: str, said: str, - ending: str) -> None: - """`result` is a refusal, exit 2, of `typed`, a directory no line about - `$PATH` can name (#667, #760), saying `said` and ending as `ending` - does. A colon's refusal names the directory; nothing else does, and no - control character, no character of `MISDRAWN` and no `forged` tail - reaches either stream.""" +def assert_refused_saying(result, said: str) -> str: + """`result` is a refusal, exit 2, that says `said` on stderr and + nothing on stdout, and no control character, no character of + `MISDRAWN` and no `forged` tail reaches either stream (#760, #837). + The refusal, its whitespace collapsed, is returned.""" assert result.returncode == 2, result.stdout + result.stderr said_in_full = " ".join(result.stderr.split()) assert said in said_in_full, said_in_full - assert ending in said_in_full, said_in_full - if said == HOLDS_A_COLON: - assert f"'{typed}', which {said}" in said_in_full, said_in_full for stream in (result.stdout, result.stderr): assert "forged" not in stream, stream assert ESCAPE not in stream, stream @@ -698,6 +705,19 @@ def assert_a_bin_dir_is_refused(result, typed: str, said: str, for code in MISDRAWN: assert chr(code) not in stream, (f"U+{code:04X}", stream) assert result.stdout == "" + return said_in_full + + +def assert_a_bin_dir_is_refused(result, typed: str, said: str, + ending: str) -> None: + """`result` is a refusal, exit 2, of `typed`, a directory no line about + `$PATH` can name (#667, #760, #837), saying `said` and ending as + `ending` does. A colon's refusal names the directory; nothing else + does (see `assert_refused_saying`).""" + said_in_full = assert_refused_saying(result, said) + assert ending in said_in_full, said_in_full + if said == HOLDS_A_COLON: + assert f"'{typed}', which {said}" in said_in_full, said_in_full @pytest.mark.parametrize("value, said", UNNAMEABLE_BIN_DIRS) @@ -723,10 +743,12 @@ def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( #: after the C1 range's last (`0xC2 0x9F`); a Persian word (letters) whose #: two parts U+200C ZERO WIDTH NON-JOINER keeps apart, and an emoji sequence #: (a woman technologist) that U+200D ZERO WIDTH JOINER makes, which that -#: script and that sequence need; and U+2026 and U+FF21, which start with -#: the bytes `0xE2` and `0xEF` that most of `MISDRAWN` start with. U+2028 -#: and U+2029 were here until #837 refused them with the rest of -#: `MISDRAWN`. +#: script and that sequence need; U+2026 and U+FF21, which start with the +#: bytes `0xE2` and `0xEF` that most of `MISDRAWN` start with; and a +#: subdivision flag (England's), which the tags U+E0020 to U+E007F make, +#: whose first three bytes are U+E0001's (`0xF3 0xA0 0x80` and +#: `0xF3 0xA0 0x81`). U+2028 and U+2029 were here until #837 refused them +#: with the rest of `MISDRAWN`. NOT_CONTROLS = [ pytest.param("caf" + chr(0xE9), id="an-e-acute"), pytest.param("a" + chr(0xA0) + "b", id="a-no-break-space"), @@ -737,6 +759,9 @@ def test_a_bin_dir_no_path_line_can_name_is_refused_before_it_places_anything( id="an-emoji-sequence-and-u+200d"), pytest.param("a" + chr(0x2026) + "b", id="u+2026"), pytest.param("a" + chr(0xFF21) + "b", id="u+ff21"), + pytest.param("".join(map(chr, (0x1F3F4, 0xE0067, 0xE0062, 0xE0065, + 0xE006E, 0xE0067, 0xE007F))), + id="a-subdivision-flag"), ] @@ -787,27 +812,33 @@ def test_a_bin_dir_with_an_unprinted_letter_is_placed( #: so this one has to come before them. MISDRAWN_BIN_DIRS = [ *(pytest.param("{d}/a" + chr(code) + "forged/bin", - drawn_as_another(f"U+{code:04X}"), id=f"u+{code:04x}") + bin_dir_drawn_as_another(f"U+{code:04X}"), + id=f"u+{code:04x}") for code in MISDRAWN), - pytest.param("{d}/a" + chr(0x202E) + "b" + chr(0x200B) + "forged/bin", - drawn_as_another("U+200B, U+202E"), id="two-at-once"), - pytest.param("{d}/caf" + NOT_UTF8 + chr(0x202E) + "forged/bin", - drawn_as_another("U+202E"), + pytest.param("{d}/a" + RLO + "b" + chr(0x200B) + "forged/bin", + bin_dir_drawn_as_another("U+200B, U+202E"), + id="two-at-once"), + pytest.param("{d}/caf" + NOT_UTF8 + RLO + "forged/bin", + bin_dir_drawn_as_another("U+202E"), id="a-byte-that-is-not-utf-8-then-u+202e"), - pytest.param("{d}/a:b" + chr(0x202E) + "forged/bin", - drawn_as_another("U+202E"), id="a-colon-and-u+202e"), - pytest.param("D:\\tools" + chr(0x202E) + "forged\\bin", - drawn_as_another("U+202E"), + pytest.param("{d}/a:b" + RLO + "forged/bin", + bin_dir_drawn_as_another("U+202E"), + id="a-colon-and-u+202e"), + pytest.param("D:\\tools" + RLO + "forged\\bin", + bin_dir_drawn_as_another("U+202E"), id="a-drive-spelling-and-u+202e"), ] #: Every value refused alike under `C` and under `C.UTF-8`, with what its -#: refusal says: each of `C1_BIN_DIRS` as a control character (#760), and -#: each of `MISDRAWN_BIN_DIRS` by what it holds (#837). +#: refusal says: each of `C1_BIN_DIRS` as a control character (#760), each +#: of `MISDRAWN_BIN_DIRS` by what it holds (#837), and a tab before U+202E +#: as a control character, because the control check comes first. REFUSED_IN_EVERY_LOCALE = [ *(pytest.param(*value.values, HOLDS_A_CONTROL_CHARACTER, id=value.id) for value in C1_BIN_DIRS), *MISDRAWN_BIN_DIRS, + pytest.param("{d}/a\t" + RLO + "forged/bin", HOLDS_A_CONTROL_CHARACTER, + id="a-tab-then-u+202e"), ] @@ -851,10 +882,20 @@ def test_install_sh_carries_the_launchers_misdrawn_check_as_it_is(): #: Every code point whose UTF-8 starts with the bytes one of `MISDRAWN` #: starts with: U+0080 to U+00BF (`0xC2`), U+0600 to U+063F (`0xD8`), -#: U+2000 to U+20FF (`0xE2 0x80` to `0xE2 0x83`) and U+FE00 to U+FFFF -#: (`0xEF 0xB8` to `0xEF 0xBF`). +#: U+2000 to U+20FF (`0xE2 0x80` to `0xE2 0x83`), U+FE00 to U+FFFF +#: (`0xEF 0xB8` to `0xEF 0xBF`), U+1D100 to U+1D1FF (`0xF0 0x9D 0x84` to +#: `0xF0 0x9D 0x87`) and U+E0000 to U+E007F (`0xF3 0xA0 0x80` and +#: `0xF3 0xA0 0x81`); and a sample of the other planes past U+FFFF: their +#: first and last code points, an emoji, a CJK ideograph, the variation +#: selectors past U+FFFF, and the format characters a script needs, which +#: stay allowed (the Kaithi number signs, the Egyptian hieroglyph and the +#: shorthand format controls). SHARING_ITS_BYTES = (*range(0x80, 0xC0), *range(0x600, 0x640), - *range(0x2000, 0x2100), *range(0xFE00, 0x10000)) + *range(0x2000, 0x2100), *range(0xFE00, 0x10000), + *range(0x1D100, 0x1D200), *range(0xE0000, 0xE0080), + 0x10000, 0x110BD, 0x110CD, *range(0x13430, 0x13440), + *range(0x1BCA0, 0x1BCA4), 0x1F600, 0x20000, 0xE0100, + 0xE01EF, 0xFFFFD, 0x10FFFF) @pytest.mark.parametrize("name", ["openRepoShape", "install.sh"]) @@ -863,9 +904,10 @@ def test_misdrawn_characters_names_its_own_and_nothing_else(tmp_path, name): as both call sites run it, over one directory name for each code point that starts with the bytes one it refuses starts with (#837): it names exactly the code points of `MISDRAWN`, each as `U+XXXX`, and nothing - for the rest, so a name that merely holds a byte such as `0xE2` or - `0xEF` is never refused. The names travel on stdin, as bytes, so this - runs under Git Bash too, where no such name crosses the environment.""" + for the rest, so a name that merely holds a byte such as `0xE2`, `0xEF` + or `0xF3` is never refused, and the four-byte form is built right. The + names travel on stdin, as bytes, so this runs under Git Bash too, where + no such name crosses the environment.""" script = tmp_path / "misdrawn.sh" write_script(script, "LC_ALL=C\n" + misdrawn_check(REPO / name) + 'while IFS= read -r -d "" value; do\n' @@ -889,7 +931,7 @@ def test_misdrawn_characters_names_its_own_and_nothing_else(tmp_path, name): LINKED_BIN_DIRS = [ pytest.param("a\rforged", HOLDS_A_CONTROL_CHARACTER, id="a-carriage-return"), - pytest.param("a" + chr(0x202E) + "forged", drawn_as_another("U+202E"), + pytest.param("a" + RLO + "forged", bin_dir_drawn_as_another("U+202E"), id="u+202e"), ] @@ -913,6 +955,69 @@ def test_a_bin_dir_is_refused_before_a_link_in_it_names_it( assert link.is_symlink() +# A ref or a repository the command would print, holding U+202E (#837). Each +# arranges its run in `tmp_path` and returns the arguments, the +# environment, the command to run, whose value the refusal names, and a +# path the run must not make (or None). + +def a_ref_to_record(tmp_path): + """`$OPENREPOSHAPE_REF`, which `--install` is asked to record (#564).""" + return (("--install",), + {"OPENREPOSHAPE_BIN_DIR": bash_path(tmp_path / "bin"), + "OPENREPOSHAPE_REF": "v1" + RLO + "forged", + "OPENREPOSHAPE_RECORD_REF": "1"}, + COMMAND, "$OPENREPOSHAPE_REF", tmp_path / "bin") + + +def a_recorded_ref(tmp_path): + """The record beside an installed command, which every run reads.""" + _result, installed = install_at(tmp_path, PINNED) + record = record_of(installed) + record.write_bytes(("v1" + RLO + "forged\n").encode("utf-8")) + return (("--version",), {}, installed, + f"the ref recorded in {bash_path(record)}", None) + + +def a_repository_to_clone(tmp_path): + """`$OPENREPOSHAPE_REPO`, which `--doctor` would clone, run from a copy + of the command with no checkout beside it, and a `git` that leaves a + mark if it runs.""" + fake = tmp_path / "fake-path" + fake.mkdir() + ran = tmp_path / "git-ran" + write_script(fake / "git", f"#!/bin/sh\n: > '{ran}'\nexit 1\n") + (fake / "git").chmod(0o755) + copy = tmp_path / "copy" / "openRepoShape" + copy.parent.mkdir() + copy.write_bytes(COMMAND.read_bytes()) + return (("--doctor", str(tmp_path)), + {"OPENREPOSHAPE_REPO": "acme/x" + RLO + "forged", + "TMPDIR": str(tmp_path), + "PATH": f"{fake}{os.pathsep}{os.environ['PATH']}"}, + copy, "OPENREPOSHAPE_REPO", ran) + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("arrange", [a_ref_to_record, a_recorded_ref, + a_repository_to_clone], + ids=lambda arrange: arrange.__name__) +def test_a_ref_or_repository_drawn_as_another_is_refused_unechoed( + tmp_path, arrange, locale): + """`ref_fault` and `shape_checkout` held a ref and a repository to + `[![:print:]]`, which a UTF-8 locale's U+202E passes: the shape + refusal then printed the ref as typed, and the clone's refusal the + repository (#837). Both now refuse a character of `MISDRAWN` first, by + its code point, under `C` and `C.UTF-8` alike: exit 2, nothing placed, + no clone, and neither stream holds the value.""" + args, env, command, what, untouched = arrange(tmp_path) + result = run_cmd(*args, home=tmp_path, cwd=tmp_path, command=command, + env={**env, "LC_ALL": locale}) + assert_refused_saying(result, f"{what} {drawn_as_another('U+202E')}.") + if untouched is not None: + assert not untouched.exists(), untouched + + @POSIX_NAME def test_the_default_directory_is_held_to_the_same_refusal(tmp_path): """No variable at all, and a `$HOME` that holds a `:`: the directory is From e3baca964a57410a4cdba3c60541fc39d6d85271 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 01:20:10 +0000 Subject: [PATCH 6/8] install.sh and openRepoShape: a `$OPENREPOSHAPE_BIN_DIR` the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before (#858) #804's control check and #837's `misdrawn_characters` look for UTF-8's bytes under `LC_ALL=C`, and a multibyte charset that is not UTF-8 reads some of those bytes as text: GBK, GB18030 and CP936 read `0xC2 0x80` to `0xC2 0x9F` as 32 hanzi; GBK, Big5, EUC-JP and EUC-KR read U+00AD's two bytes as one character; and in Shift_JIS a kanji whose second byte is `0xC2` before a common kanji puts a C1 control's two bytes side by side (390 in 100,000 random pairs of common kanji, measured with Python's codecs). Such a directory, the default `$HOME/.local/bin` under a user name among them, was refused as holding a control character or a character drawn as another, in a locale where it holds neither. Both files now carry `locale_reading` (install.sh as a copy, held to one text by a test). Where the bytes refuse a directory, it asks `locale charmap` under the person's own locale, and where the answer is neither empty, UTF-8 nor ASCII, `iconv` reads the whole value from that charset into UTF-8; the reading is used only where it has fewer characters than the value has bytes, so a single-byte charset keeps the byte rule, as the issue's Direction says. The C1 bytes and `misdrawn_characters` are then asked of the reading, and `[[:cntrl:]]` of the bytes as typed. Only a value the bytes refuse is read again, so no name placed before is refused; with no `locale` or `iconv`, a charset either does not know, or a value the charset cannot read, the bytes decide, in #804's and #837's words. A lone `0x9B` stays placed, the limit #804 states: a terminal that reads UTF-8 draws it as a character it cannot read, only one set to an 8-bit charset acts on it, and a rule needs a UTF-8 validity step bash does not have. The launcher's comment says so. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- install.sh | 51 +++++++-- openRepoShape | 76 +++++++++++-- tests/test_install_sh.py | 44 +++++++- tests/test_openreposhape_command.py | 162 +++++++++++++++++++++++++++- 4 files changed, 316 insertions(+), 17 deletions(-) diff --git a/install.sh b/install.sh index 1428e0b8..953a6998 100755 --- a/install.sh +++ b/install.sh @@ -399,9 +399,32 @@ misdrawn_characters() { printf '%s' "$found" } +# THE VALUE AS THE PERSON'S LOCALE READS IT, IN UTF-8, WHERE THAT LOCALE +# READS TWO OR MORE OF ITS BYTES AS ONE CHARACTER (#858): the launcher's +# `locale_reading`, copied as it is, for the reason the control check in +# `main` is copied. The launcher says when it reads the value again and +# when the bytes decide, and `tests/test_openreposhape_command.py` holds the +# two copies to one text. `main` asks it of a directory the bytes refuse. +locale_reading() { + local charmap reading + charmap="$(locale charmap 2>/dev/null)" || return 1 + case "$charmap" in + "" | [Uu][Tt][Ff]-8 | [Uu][Tt][Ff]8 | ANSI_X3.4-1968 | *ASCII*) return 1 ;; + *) ;; + esac + reading="$(printf '%s' "$1" | iconv -f "$charmap" -t UTF-8 2>/dev/null)" || + return 1 + ( + LC_ALL=C + letters=${reading//[$'\200'-$'\277']/} + [ "${#letters}" -lt "${#1}" ] || exit 1 + printf '%s' "$reading" + ) +} + main() { local version="" asked launcher_url bin_dir spelled target shown status=0 pinned=1 - local misdrawn + local misdrawn seen local machine="this machine is ready" # NOTHING A CHILD READS IS THIS SCRIPT. Piped, stdin is the pipe the @@ -446,23 +469,39 @@ main() { # print (`café` under LC_ALL=C) is no fault of the directory's. The check # runs under `LC_ALL=C` and names U+0080 to U+009F's two bytes (`\302`, # then `\200` to `\237`), so a C1 control is refused in every locale, - # whatever else the value holds; U+2028 and U+2029, which only a UTF-8 - # locale's `[[:cntrl:]]` matched, are the next check's (#837). + # whatever else the value holds, unless the locale reads those bytes as + # text (#858, below); U+2028 and U+2029, which only a UTF-8 locale's + # `[[:cntrl:]]` matched, are the next check's (#837). # Left to the locale, `[[:cntrl:]]` matches a C1 control only in a value # that is valid UTF-8: one byte that is not (a lone 0xE9) makes bash # match byte by byte, and then U+009B's two bytes are not controls, as # under `C` (measured with bash 5.2.21). The locale is `C` in a subshell, # so that the match is byte by byte whatever a libc says of a byte in a # UTF-8 locale, and the locale of every line below is as it was. - if (LC_ALL=C; case "$bin_dir" in *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then + # WHERE THE BYTES ARE REFUSED, THE PERSON'S LOCALE IS ASKED HOW IT READS + # THEM (#858), as the command asks it: a multibyte charset that is not + # UTF-8 may read them as text, and then the C1 bytes and + # `misdrawn_characters` are looked for in `locale_reading`'s UTF-8 + # instead. Only a value the bytes refuse is read again, so no name + # placed before is refused, and where nothing reads it again the bytes + # decide, as before. `[[:cntrl:]]` is asked of the bytes as typed. + seen="$bin_dir" + if (LC_ALL=C; case "$bin_dir" in *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac) || + [ -n "$(LC_ALL=C; misdrawn_characters "$bin_dir")" ]; then + seen="$(locale_reading "$bin_dir")" || seen="$bin_dir" + fi + if (LC_ALL=C + case "$bin_dir" in *[[:cntrl:]]*) exit 0 ;; *) ;; esac + case "$seen" in *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then die "the directory to install into holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." fi # A CHARACTER A TERMINAL DRAWS AS SOMETHING OTHER THAN ITSELF IS REFUSED # NEXT (#837), as the command refuses it, and before any request: the # name would read as another directory's in every line below. The # refusal names each one by its code point, because nobody can see it, - # and says nothing else of the value. - misdrawn="$(LC_ALL=C; misdrawn_characters "$bin_dir")" + # and says nothing else of the value. It is asked of the locale's + # reading where there is one (#858). + misdrawn="$(LC_ALL=C; misdrawn_characters "$seen")" [ -z "$misdrawn" ] || die "the directory to install into holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was installed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this line again." # A WINDOWS SPELLING IS SPELLED THE WAY BASH SPELLS IT, THE COMMAND'S OWN diff --git a/openRepoShape b/openRepoShape index a94e5192..891afe96 100755 --- a/openRepoShape +++ b/openRepoShape @@ -537,10 +537,52 @@ misdrawn_characters() { printf '%s' "$found" } +# THE VALUE AS THE PERSON'S LOCALE READS IT, IN UTF-8, WHERE THAT LOCALE +# READS TWO OR MORE OF ITS BYTES AS ONE CHARACTER (#858). The control check +# and `misdrawn_characters` look for UTF-8's bytes under `LC_ALL=C`, and a +# multibyte charset that is not UTF-8 reads some of those bytes as text: +# GBK, GB18030 and CP936 read `0xC2 0x80` to `0xC2 0x9F` as 32 hanzi; GBK, +# Big5, EUC-JP and EUC-KR read U+00AD's two bytes as a hanzi, a kanji or a +# Hangul syllable; and in Shift_JIS a kanji whose second byte is `0xC2`, +# before one of the common kanji (first byte `0x88` to `0x98`), puts a C1 +# control's two bytes side by side (measured with Python's codecs). +# `install_commands` asks this of a value those checks refuse, and asks them +# again of what it prints. It prints the reading, and succeeds, only where: +# * `locale charmap`, asked under the person's own `LC_ALL`, `LC_CTYPE` +# and `LANG`, answers with a charset that is neither UTF-8, whose +# reading is the bytes, nor ASCII, which reads no byte past 0x7F; +# * `iconv` reads the whole value in that charset; and +# * the reading has fewer characters than the value has bytes, counted +# under `LC_ALL=C` as the bytes of UTF-8 that start a character (0x80 +# to 0xBF continue one). A single-byte charset reads each byte alone, +# and there the bytes decide, as before: under a Latin-1 `LANG` left in +# a terminal that reads UTF-8, U+FEFF's three bytes would read as three +# letters. +# Otherwise it prints nothing and fails, and the bytes decide: no `locale` +# or no `iconv` (a minimal container), a charset either does not know, or +# a value the charset cannot read. install.sh carries this function as it +# is here. +locale_reading() { + local charmap reading + charmap="$(locale charmap 2>/dev/null)" || return 1 + case "$charmap" in + "" | [Uu][Tt][Ff]-8 | [Uu][Tt][Ff]8 | ANSI_X3.4-1968 | *ASCII*) return 1 ;; + *) ;; + esac + reading="$(printf '%s' "$1" | iconv -f "$charmap" -t UTF-8 2>/dev/null)" || + return 1 + ( + LC_ALL=C + letters=${reading//[$'\200'-$'\277']/} + [ "${#letters}" -lt "${#1}" ] || exit 1 + printf '%s' "$reading" + ) +} + install_commands() { local dir="${OPENREPOSHAPE_BIN_DIR:-$HOME/.local/bin}" local name target source verb placed=0 recording=0 record spelled="" - local misdrawn + local misdrawn seen # NO LINE ABOUT `$PATH` CAN NAME A DIRECTORY HOLDING A CONTROL CHARACTER # OR A `:` (#667, #760): the colon is what splits a `$PATH`; a newline # breaks the line the note prints in two; a carriage return rewrites the @@ -555,9 +597,10 @@ install_commands() { # under LC_ALL=C, is no fault of the directory's. The check runs under # `LC_ALL=C` and names U+0080 to U+009F's two bytes (`\302`, then `\200` # to `\237`), so a C1 control is refused in every locale, whatever else - # the value holds; U+2028 and U+2029, which only a UTF-8 locale's - # `[[:cntrl:]]` matched, are the next check's (#837). Left to the - # locale, `[[:cntrl:]]` matches a C1 control only in a value that is + # the value holds, unless the locale reads those bytes as text (#858, + # below); U+2028 and U+2029, which only a UTF-8 locale's `[[:cntrl:]]` + # matched, are the next check's (#837). Left to the locale, + # `[[:cntrl:]]` matches a C1 control only in a value that is # valid UTF-8: one byte that is not (a lone 0xE9) makes bash match byte # by byte, and then U+009B's two bytes are not controls, as under `C` # (measured with bash 5.2.21). The locale is `C` in a subshell, so that @@ -565,15 +608,34 @@ install_commands() { # locale, and the locale of every line below is as it was. The colon is # checked after the Windows spelling, which holds one by definition, is # respelled without one, and refuses what it cannot respell. - if (LC_ALL=C; case "$dir" in *[[:cntrl:]]* | *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then + # WHERE THE BYTES ARE REFUSED, THE PERSON'S LOCALE IS ASKED HOW IT READS + # THEM (#858): a multibyte charset that is not UTF-8 may read them as + # text, and then the C1 bytes and `misdrawn_characters` are looked for + # in `locale_reading`'s UTF-8 instead. Only a value the bytes refuse is + # read again, so no name placed before is refused, and where nothing + # reads it again the bytes decide, as before. `[[:cntrl:]]` is asked of + # the bytes as typed: those characters are ASCII's in every such + # charset, and `$(…)` drops a newline at the end of the reading. A lone + # C1 byte that is not UTF-8 (`0x9B`) is placed, as before: a terminal + # that reads UTF-8 draws it as a character it cannot read, and only one + # set to an 8-bit charset acts on it. + seen="$dir" + if (LC_ALL=C; case "$dir" in *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac) || + [ -n "$(LC_ALL=C; misdrawn_characters "$dir")" ]; then + seen="$(locale_reading "$dir")" || seen="$dir" + fi + if (LC_ALL=C + case "$dir" in *[[:cntrl:]]*) exit 0 ;; *) ;; esac + case "$seen" in *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac); then die "--install would place this command in a directory whose name holds a control character, such as a newline, a carriage return, a tab or an escape, and no line about \$PATH can print such a name as it is: a terminal acts on it. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." fi # A CHARACTER A TERMINAL DRAWS AS SOMETHING OTHER THAN ITSELF IS REFUSED # NEXT (#837), for the control characters' reason and before the same # lines: the name would read as another directory's. The refusal names # each one by its code point, because nobody can see it, and says - # nothing else of the value. `misdrawn_characters` says which they are. - misdrawn="$(LC_ALL=C; misdrawn_characters "$dir")" + # nothing else of the value. `misdrawn_characters` says which they are, + # in the locale's reading where there is one (#858). + misdrawn="$(LC_ALL=C; misdrawn_characters "$seen")" [ -z "$misdrawn" ] || die "--install would place this command in a directory whose name holds a character a terminal draws as something other than itself ($misdrawn): one that is invisible, is drawn as another character, ends a line or reorders the text around it, so no line about \$PATH can print the name as it reads. Nothing was placed. Set \$OPENREPOSHAPE_BIN_DIR to a directory without one, then run this again." # A WINDOWS SPELLING IS SPELLED AGAIN THE WAY BASH SPELLS IT (#649), with diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index d35ece00..486f0d73 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -53,13 +53,16 @@ LOCALES, NO_FILE_NAMED_SO, NO_NON_ASCII_INTO_GIT_BASH, NOT_CONTROLS, + PLACED_AS_THE_LOCALE_READS, + REFUSED_AS_THE_BYTES_SAY, REFUSED_IN_EVERY_LOCALE, RLO, UNNAMEABLE_BIN_DIRS, assert_a_bin_dir_is_refused, assert_refused_saying, drawn_as_another, command_env, machine_path, - put_cygpath, put_on_path, run_cmd, + put_a_locale, put_cygpath, + put_on_path, run_cmd, typed_bin_dir, write_script) INSTALLER = REPO / "install.sh" @@ -974,6 +977,45 @@ def test_the_installer_refuses_in_every_locale_before_any_request( net.refused(result) +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("charmap, name", PLACED_AS_THE_LOCALE_READS) +def test_the_installer_places_a_directory_a_multibyte_locale_reads_as_text( + tmp_path, charmap, name, locale): + """A name the bytes refuse, read as text by the locale's multibyte + charset (#858): install.sh asks `locale charmap` and reads it through + `iconv`, as the command does, and the command it fetched places itself + there, under the same `locale` and `iconv`, with the release recorded. + Neither refuses what the other places.""" + net = Net(tmp_path) + put_a_locale(net.path, charmap) + net.bin = tmp_path / name + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": bash_path(net.bin), + "LC_ALL": locale}) + assert result.returncode == 0, result.stdout + result.stderr + net.installed(result) + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("charmap, iconv, name, said", + REFUSED_AS_THE_BYTES_SAY) +def test_the_installer_refuses_as_the_bytes_say_where_nothing_reads_them( + tmp_path, charmap, iconv, name, said, locale): + """Where nothing reads the name again (#858), install.sh refuses it as + the bytes say, in #804's or #837's words, before any request: exit 2, + nothing placed, nothing of the value in either stream. With no `locale` + on this `$PATH` at all, every other test of the directory here runs + that way too.""" + net = Net(tmp_path) + put_a_locale(net.path, charmap, iconv) + typed = f"{bash_path(tmp_path)}/{name}" + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) + assert net.calls() == [], net.calls() + assert_a_bin_dir_is_refused(result, typed, said, INSTALLER_REFUSAL_ENDS) + net.refused(result) + + # A version, an option, the repository, the newest release's tag or the # location github.com redirects to, holding U+202E (#837). Each arranges # `net` and returns the arguments, the environment, what the refusal says, diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index fe481d32..7ea8db2f 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -862,14 +862,20 @@ def test_a_bin_dir_is_refused_in_every_locale_whatever_else_it_holds( assert list(tmp_path.iterdir()) == [] +def shell_function(path: Path, name: str) -> str: + """The function `name`, from its first line to its closing brace, as + the file at `path` carries it.""" + [function] = re.findall(rf"^{name}\(\) \{{\n.*?^\}}\n", + path.read_text(encoding="utf-8"), re.M | re.S) + return function + + def misdrawn_check(path: Path) -> str: """`MISDRAWN_CHARACTERS` and `misdrawn_characters`, as the file at `path` carries them (#837).""" text = path.read_text(encoding="utf-8") [listed] = re.findall(r"^MISDRAWN_CHARACTERS=\(.*?\)\n", text, re.M) - [function] = re.findall(r"^misdrawn_characters\(\) \{\n.*?^\}\n", text, - re.M | re.S) - return listed + function + return listed + shell_function(path, "misdrawn_characters") def test_install_sh_carries_the_launchers_misdrawn_check_as_it_is(): @@ -926,6 +932,156 @@ def test_misdrawn_characters_names_its_own_and_nothing_else(tmp_path, name): for code in SHARING_ITS_BYTES} +def test_install_sh_carries_the_launchers_locale_reading_as_it_is(): + """install.sh carries the launcher's `locale_reading` as well (#858), + so the two read a directory the bytes refuse alike, and neither places + one the other refuses.""" + assert shell_function(REPO / "install.sh", "locale_reading") \ + == shell_function(COMMAND, "locale_reading") + + +#: HOW A `locale` OR AN `iconv` A TEST PUTS FIRST ON `$PATH` BEHAVES WHERE +#: IT DOES NOT ANSWER WITH A CHARSET'S NAME (#858): `FAILS`, a `locale` that +#: answers `GB18030` and exits 1; `MISSING`, one that exits 127, as a shell +#: answers a name it cannot find; and `READS_ALL`, an `iconv` that reads any +#: value at all as the word `read`, which would place it if it were asked. +FAILS, MISSING, READS_ALL = "fails", "missing", "reads-all" + + +def put_a_locale(directory: Path, charmap: str, + iconv: str | None = None) -> None: + """A `locale` in `directory` that answers `locale charmap` with + `charmap`, or as `FAILS` or `MISSING` says, and an `iconv` beside it: + this machine's, or one that behaves as `MISSING` or `READS_ALL` says + (#858). No GB18030, GBK or Shift_JIS locale is installed here or on + CI, and none can be built, so it is `locale` that says the charset, + and bash's own locale stays `C` or `C.UTF-8`.""" + answers = {FAILS: "printf 'GB18030\\n'\nexit 1\n", MISSING: "exit 127\n"} + write_script(directory / "locale", "#!/bin/sh\n" + answers.get( + charmap, '[ "$1" = charmap ] || exit 2\n' + f"printf '%s\\n' {shlex.quote(charmap)}\n")) + (directory / "locale").chmod(0o755) + if iconv is None: + real = shutil.which("iconv") + assert real, "this machine has no iconv, which these cases read with" + put_on_path(directory, "iconv", real) + return + write_script(directory / "iconv", "#!/bin/sh\n" + { + MISSING: "exit 127\n", + READS_ALL: "cat >/dev/null\nprintf read\n"}[iconv]) + (directory / "iconv").chmod(0o755) + + +#: One of the 32 hanzi that GBK, GB18030 and CP936 read `0xC2 0x80` to +#: `0xC2 0x9F` as, which in UTF-8 are a C1 control's two bytes: Python +#: hands `chr(0x9B)` to a child as `0xC2 0x9B`, U+8078 in GB18030 (#858). +A_HANZI_OF_C1_BYTES = "a" + C1_CSI + "b" + +#: Names the bytes refuse that a multibyte charset reads as text, each with +#: that charset (#858): the first, the issue's and the last of the 32 hanzi, +#: so that a C1 range that stops one short at either end, where it decides +#: whether to ask the locale, is caught; U+00AD's two bytes, U+9885 in GBK; +#: U+2028's three, a kanji and a half-width katakana in Shift_JIS; and in +#: Shift_JIS a half-width katakana (`0xC2`) before a character whose first +#: byte is `0x81`, read as U+FF82 and then U+3005 or U+FF3F, whose UTF-8 +#: holds a `0x80` or a `0xBF`: the first and the last byte that continues a +#: character, which the count of the reading's characters must not count. +PLACED_AS_THE_LOCALE_READS = [ + pytest.param("GB18030", "a" + chr(0x80) + "b", id="gb18030-0xc2-0x80"), + pytest.param("GB18030", A_HANZI_OF_C1_BYTES, id="gb18030-0xc2-0x9b"), + pytest.param("GB18030", "a" + chr(0x9F) + "b", id="gb18030-0xc2-0x9f"), + pytest.param("GBK", "a" + chr(0xAD) + "b", id="gbk-u+00ad-bytes"), + pytest.param("SHIFT_JIS", "a" + chr(0x2028) + "b", + id="shift_jis-u+2028-bytes"), + pytest.param("SHIFT_JIS", "a" + chr(0x81) + "Xb", + id="shift_jis-0xc2-0x81-0x58"), + pytest.param("SHIFT_JIS", "a" + chr(0x81) + "Qb", + id="shift_jis-0xc2-0x81-0x51"), +] + +#: Names the bytes refuse where nothing reads them again, and what the +#: refusal says (#858): under UTF-8, three of `PLACED_AS_THE_LOCALE_READS`, +#: in #804's and #837's words; where `locale` fails or is not there; where +#: `iconv` is not there; where GB18030 cannot read a byte (`0xFF`); under a +#: single-byte charset, as Latin-1 reads U+FEFF's three bytes, each alone; +#: with a newline at the end, which `$(…)` drops from a reading; and where +#: `locale` answers nothing, UTF-8 or ASCII, under which `iconv` is never +#: asked, though the one there would read anything as text. +REFUSED_AS_THE_BYTES_SAY = [ + pytest.param("UTF-8", None, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, id="utf-8-0xc2-0x9b"), + pytest.param("UTF-8", None, "a" + chr(0xAD) + "b", + bin_dir_drawn_as_another("U+00AD"), id="utf-8-u+00ad"), + pytest.param("UTF-8", None, "a" + chr(0x2028) + "b", + bin_dir_drawn_as_another("U+2028"), id="utf-8-u+2028"), + pytest.param(FAILS, None, A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, + id="a-locale-that-fails"), + pytest.param(MISSING, None, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, id="no-locale"), + pytest.param("GB18030", MISSING, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, id="no-iconv"), + pytest.param("GB18030", None, A_HANZI_OF_C1_BYTES + chr(0xDCFF), + HOLDS_A_CONTROL_CHARACTER, id="a-byte-gb18030-cannot-read"), + pytest.param("ISO-8859-1", None, "a" + chr(0xFEFF) + "b", + bin_dir_drawn_as_another("U+FEFF"), + id="a-single-byte-charset"), + pytest.param("GB18030", None, A_HANZI_OF_C1_BYTES + "\n", + HOLDS_A_CONTROL_CHARACTER, id="a-newline-at-the-end"), + *(pytest.param(answer, READS_ALL, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, + id=f"iconv-unasked-under-{answer or 'no-answer'}") + for answer in ("", "UTF-8", "utf8", "ANSI_X3.4-1968", "US-ASCII")), +] + + +def install_under_a_locale(tmp_path: Path, typed: str, charmap: str, + iconv: str | None, locale: str): + """`--install` into `typed`, under bash's `locale`, with + `put_a_locale`'s `locale` and `iconv` first on `$PATH` (#858).""" + tools = tmp_path / "tools" + tools.mkdir() + put_a_locale(tools, charmap, iconv) + return run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ + "OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale, + "PATH": f"{tools}{os.pathsep}{os.environ['PATH']}"}) + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("charmap, name", PLACED_AS_THE_LOCALE_READS) +def test_a_bin_dir_a_multibyte_locale_reads_as_text_is_placed( + tmp_path, charmap, name, locale): + """The control check and `misdrawn_characters` look for UTF-8's bytes, + and GB18030 reads `0xC2 0x9B` as a hanzi, not as U+009B: the directory + was refused as holding a control character, in a locale where it holds + none (#858). Where the bytes are refused, `locale charmap` is asked, + and a multibyte charset's reading of the name, through `iconv`, has + the last word. So a name GB18030, GBK or Shift_JIS reads as text is + placed, under bash's `C` and `C.UTF-8` alike.""" + result = install_under_a_locale( + tmp_path, f"{bash_path(tmp_path)}/{name}", charmap, None, locale) + assert result.returncode == 0, result.stdout + result.stderr + assert (tmp_path / name / "openRepoShape").read_bytes() \ + == COMMAND.read_bytes() + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("charmap, iconv, name, said", + REFUSED_AS_THE_BYTES_SAY) +def test_a_bin_dir_is_refused_as_its_bytes_say_where_nothing_reads_them_again( + tmp_path, charmap, iconv, name, said, locale): + """Where the locale's charset is UTF-8, ASCII, single-byte or not said, + where `iconv` is not there or cannot read the name, the bytes decide, + as before (#858): refused, exit 2, nothing placed, never echoed, and in + #804's or #837's words. A newline at the end is looked for in the bytes + as typed, not in the reading.""" + typed = f"{bash_path(tmp_path)}/{name}" + result = install_under_a_locale(tmp_path, typed, charmap, iconv, locale) + assert_a_bin_dir_is_refused(result, typed, said, COMMAND_REFUSAL_ENDS) + assert [path.name for path in tmp_path.iterdir()] == ["tools"] + + #: A directory that already holds pipx's link (#563), whose name holds a #: carriage return (#760) or U+202E (#837), and what its refusal says. LINKED_BIN_DIRS = [ From 18ae33b1da210b943582ea821689e2f0ed5fa050 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 03:08:08 +0000 Subject: [PATCH 7/8] install.sh and openRepoShape: the locale's reading of `$OPENREPOSHAPE_BIN_DIR` must write back into the value byte for byte, and tests hold that the reading is itself checked, that `locale charmap` is asked under the person's own locale and that a name the bytes pass is never read again (#858) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 1 on rv858's review of #867 (READY, 2 SHOULDs, 5 NITs). SHOULD 1: no test had a multibyte charset read a name whose reading still holds a C1 control or a character drawn as another, so `locale_reading` printing anything at all (rv858's R1) shipped green. Two rows now do, through both files: a GB18030 hanzi of the C1 bytes beside U+009B in GB18030's own four-byte form (`0x81 0x30 0x83 0x37`), refused as a control character, and beside U+202E (`0x81 0x36 0xA7 0x31`), refused naming U+202E, install.sh before any request. SHOULD 2: the stub `locale` answered one charmap whatever it was asked under, so `LC_ALL=C locale charmap` (R2) shipped green. `put_a_locale`'s stub now answers as glibc's does, from the first of `$LC_ALL`, `$LC_CTYPE` and `$LANG` set: a marker `xx_XX.` gives that charmap, `C.UTF-8` gives UTF-8, anything else `ANSI_X3.4-1968`; the cases run with `LANG` the marker and `LC_ALL` and `LC_CTYPE` empty, so bash runs in `C` (it says nothing of a `LANG` that is not installed). `C` and `C.UTF-8` stay where they mean something: as the person's own locale, in a test of three names refused there with `iconv` never asked; and as bash's own locale, in a test of `locale_reading` lifted out of each file with a `locale` function, `C.UTF-8` standing in for a multibyte locale of bash's own, where the readings are Python's codecs' and the count stays in bytes. NIT 3: a name the bytes pass (`café`) under a GB18030 locale, with an `iconv` that leaves a mark if it runs: placed, no mark (R3, the trigger replaced by `if :; then`). NIT 4: an `iconv` that exits 0 and prints nothing passed the count. The reading must now write back into the charset as the value, byte for byte (`iconv -f UTF-8 -t `), in both copies; a broken `iconv` falls back to today's refusal, and a test holds it. The rows where `iconv` must not be asked use the marking `iconv` instead of one that reads anything, which the round trip would refuse anyway. NIT 5: `locale_reading`'s comment, both callers' and `export_path_line`'s say the trade: a multibyte locale that is not UTF-8, in a terminal that reads UTF-8, is handed C1 bytes it reads as letters. NIT 7: the launcher's comment names the ISO 8859 case (the charset is known and `a`, `0x9B`, `b` is placed) as a candidate, left. NIT 6 is the PR body's. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- install.sh | 7 +- openRepoShape | 33 +++- tests/test_install_sh.py | 63 +++++-- tests/test_openreposhape_command.py | 271 ++++++++++++++++++++-------- 4 files changed, 269 insertions(+), 105 deletions(-) diff --git a/install.sh b/install.sh index 953a6998..cfc84568 100755 --- a/install.sh +++ b/install.sh @@ -414,6 +414,8 @@ locale_reading() { esac reading="$(printf '%s' "$1" | iconv -f "$charmap" -t UTF-8 2>/dev/null)" || return 1 + [ "$(printf '%s' "$reading" | iconv -f UTF-8 -t "$charmap" 2>/dev/null)" = "$1" ] || + return 1 ( LC_ALL=C letters=${reading//[$'\200'-$'\277']/} @@ -484,7 +486,10 @@ main() { # `misdrawn_characters` are looked for in `locale_reading`'s UTF-8 # instead. Only a value the bytes refuse is read again, so no name # placed before is refused, and where nothing reads it again the bytes - # decide, as before. `[[:cntrl:]]` is asked of the bytes as typed. + # decide, as before. `[[:cntrl:]]` is asked of the bytes as typed. The + # trade: such a locale, left in a terminal that reads UTF-8, has C1 + # bytes it reads as letters installed into and printed in the lines + # below, and that terminal acts on them. seen="$bin_dir" if (LC_ALL=C; case "$bin_dir" in *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac) || [ -n "$(LC_ALL=C; misdrawn_characters "$bin_dir")" ]; then diff --git a/openRepoShape b/openRepoShape index 891afe96..2ed8e2e5 100755 --- a/openRepoShape +++ b/openRepoShape @@ -415,7 +415,10 @@ record_ref() { # character (a newline breaks the printed line in two, an escape is the # terminal's to act on) is no name a line can print as it is, and nor is a # character a terminal draws as something other than itself (#837), so -# `install_commands` refuses all three before this is asked. +# `install_commands` refuses all three before this is asked. The C1 +# controls and those characters are refused as the person's locale reads +# the name (#858): a multibyte locale that is not UTF-8 can hand this line +# C1 bytes it reads as letters, which a terminal that reads UTF-8 acts on. export_path_line() { local dir="$1" backslash='\' quote="'" rest quoted="" case "$dir" in @@ -551,7 +554,10 @@ misdrawn_characters() { # * `locale charmap`, asked under the person's own `LC_ALL`, `LC_CTYPE` # and `LANG`, answers with a charset that is neither UTF-8, whose # reading is the bytes, nor ASCII, which reads no byte past 0x7F; -# * `iconv` reads the whole value in that charset; and +# * `iconv` reads the whole value in that charset; +# * the reading, written back into that charset by `iconv`, is the value +# again, byte for byte, so an `iconv` that drops or changes a byte, or +# prints nothing at all and exits 0, gives no reading; and # * the reading has fewer characters than the value has bytes, counted # under `LC_ALL=C` as the bytes of UTF-8 that start a character (0x80 # to 0xBF continue one). A single-byte charset reads each byte alone, @@ -560,8 +566,11 @@ misdrawn_characters() { # letters. # Otherwise it prints nothing and fails, and the bytes decide: no `locale` # or no `iconv` (a minimal container), a charset either does not know, or -# a value the charset cannot read. install.sh carries this function as it -# is here. +# a value the charset cannot read. The locale is taken at its word, and +# that is the trade: a multibyte locale that is not UTF-8, left in a +# terminal that reads UTF-8, is handed the C1 bytes and the bytes of +# `MISDRAWN_CHARACTERS` it reads as letters, and that terminal acts on +# them. install.sh carries this function as it is here. locale_reading() { local charmap reading charmap="$(locale charmap 2>/dev/null)" || return 1 @@ -571,6 +580,8 @@ locale_reading() { esac reading="$(printf '%s' "$1" | iconv -f "$charmap" -t UTF-8 2>/dev/null)" || return 1 + [ "$(printf '%s' "$reading" | iconv -f UTF-8 -t "$charmap" 2>/dev/null)" = "$1" ] || + return 1 ( LC_ALL=C letters=${reading//[$'\200'-$'\277']/} @@ -615,10 +626,16 @@ install_commands() { # read again, so no name placed before is refused, and where nothing # reads it again the bytes decide, as before. `[[:cntrl:]]` is asked of # the bytes as typed: those characters are ASCII's in every such - # charset, and `$(…)` drops a newline at the end of the reading. A lone - # C1 byte that is not UTF-8 (`0x9B`) is placed, as before: a terminal - # that reads UTF-8 draws it as a character it cannot read, and only one - # set to an 8-bit charset acts on it. + # charset, and `$(…)` drops a newline at the end of the reading. The + # trade: such a locale, left in a terminal that reads UTF-8, has C1 bytes + # it reads as letters placed and printed in the lines below, and that + # terminal acts on them. A lone C1 byte that is not UTF-8 (`0x9B`) is + # placed, as before: a terminal that reads UTF-8 draws it as a character + # it cannot read, and only one set to an 8-bit charset acts on it. Under + # an ISO 8859 charmap, the one such a terminal goes with, the charset is + # known and reads that byte as U+009B, but nothing here reads the name + # again, so `a`, `0x9B`, `b` is placed there too: a candidate, left + # (#858). seen="$dir" if (LC_ALL=C; case "$dir" in *$'\302'[$'\200'-$'\237']*) exit 0 ;; *) exit 1 ;; esac) || [ -n "$(LC_ALL=C; misdrawn_characters "$dir")" ]; then diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index 486f0d73..86c781a2 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -53,8 +53,10 @@ LOCALES, NO_FILE_NAMED_SO, NO_NON_ASCII_INTO_GIT_BASH, NOT_CONTROLS, + HONEST, MARKS, PLACED_AS_THE_LOCALE_READS, REFUSED_AS_THE_BYTES_SAY, + REFUSED_UNDER_C_OR_UTF_8, REFUSED_IN_EVERY_LOCALE, RLO, UNNAMEABLE_BIN_DIRS, assert_a_bin_dir_is_refused, @@ -63,6 +65,7 @@ command_env, machine_path, put_a_locale, put_cygpath, put_on_path, run_cmd, + the_persons_locale, typed_bin_dir, write_script) INSTALLER = REPO / "install.sh" @@ -978,42 +981,64 @@ def test_the_installer_refuses_in_every_locale_before_any_request( @NO_NON_ASCII_INTO_GIT_BASH -@pytest.mark.parametrize("locale", LOCALES) -@pytest.mark.parametrize("charmap, name", PLACED_AS_THE_LOCALE_READS) +@pytest.mark.parametrize("charmap, iconv, name", PLACED_AS_THE_LOCALE_READS) def test_the_installer_places_a_directory_a_multibyte_locale_reads_as_text( - tmp_path, charmap, name, locale): + tmp_path, charmap, iconv, name): """A name the bytes refuse, read as text by the locale's multibyte - charset (#858): install.sh asks `locale charmap` and reads it through - `iconv`, as the command does, and the command it fetched places itself - there, under the same `locale` and `iconv`, with the release recorded. - Neither refuses what the other places.""" + charset (#858): install.sh asks `locale charmap` under the person's own + `LANG` and reads the name through `iconv`, as the command does, and the + command it fetched places itself there, under the same `locale` and + `iconv`, with the release recorded. Neither refuses what the other + places, and neither asks `iconv` of a name the bytes do not refuse.""" net = Net(tmp_path) - put_a_locale(net.path, charmap) + put_a_locale(net.path, HONEST, iconv) net.bin = tmp_path / name result = net.run(env={"OPENREPOSHAPE_BIN_DIR": bash_path(net.bin), - "LC_ALL": locale}) + **the_persons_locale(charmap)}) assert result.returncode == 0, result.stdout + result.stderr net.installed(result) + assert not (net.path / "iconv-ran").exists() @NO_NON_ASCII_INTO_GIT_BASH -@pytest.mark.parametrize("locale", LOCALES) -@pytest.mark.parametrize("charmap, iconv, name, said", +@pytest.mark.parametrize("charmap, kind, iconv, name, said", REFUSED_AS_THE_BYTES_SAY) -def test_the_installer_refuses_as_the_bytes_say_where_nothing_reads_them( - tmp_path, charmap, iconv, name, said, locale): - """Where nothing reads the name again (#858), install.sh refuses it as - the bytes say, in #804's or #837's words, before any request: exit 2, - nothing placed, nothing of the value in either stream. With no `locale` - on this `$PATH` at all, every other test of the directory here runs - that way too.""" +def test_the_installer_refuses_as_the_bytes_or_the_reading_say( + tmp_path, charmap, kind, iconv, name, said): + """Where the locale's reading still holds a C1 control or a character + drawn as another, or where nothing reads the name again (#858), + install.sh refuses it in #804's or #837's words, before any request: + exit 2, nothing placed, nothing of the value in either stream. With no + `locale` on this `$PATH` at all, every other test of the directory here + runs that way too.""" + net = Net(tmp_path) + put_a_locale(net.path, kind, iconv) + typed = f"{bash_path(tmp_path)}/{name}" + result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, + **the_persons_locale(charmap)}) + assert net.calls() == [], net.calls() + assert_a_bin_dir_is_refused(result, typed, said, INSTALLER_REFUSAL_ENDS) + net.refused(result) + assert not (net.path / "iconv-ran").exists() + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("name, said", REFUSED_UNDER_C_OR_UTF_8) +def test_the_installer_refuses_as_the_bytes_say_under_c_or_c_utf_8( + tmp_path, name, said, locale): + """The person's locale is `C` or `C.UTF-8`, and `locale charmap` + answers as glibc's does: install.sh refuses as the bytes say, in #804's + and #837's words, before any request, and asks `iconv` nothing + (#858).""" net = Net(tmp_path) - put_a_locale(net.path, charmap, iconv) + put_a_locale(net.path, HONEST, MARKS) typed = f"{bash_path(tmp_path)}/{name}" result = net.run(env={"OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale}) assert net.calls() == [], net.calls() assert_a_bin_dir_is_refused(result, typed, said, INSTALLER_REFUSAL_ENDS) net.refused(result) + assert not (net.path / "iconv-ran").exists() # A version, an option, the repository, the newest release's tag or the diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index 7ea8db2f..4f51d865 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -940,26 +940,40 @@ def test_install_sh_carries_the_launchers_locale_reading_as_it_is(): == shell_function(COMMAND, "locale_reading") -#: HOW A `locale` OR AN `iconv` A TEST PUTS FIRST ON `$PATH` BEHAVES WHERE -#: IT DOES NOT ANSWER WITH A CHARSET'S NAME (#858): `FAILS`, a `locale` that -#: answers `GB18030` and exits 1; `MISSING`, one that exits 127, as a shell -#: answers a name it cannot find; and `READS_ALL`, an `iconv` that reads any -#: value at all as the word `read`, which would place it if it were asked. -FAILS, MISSING, READS_ALL = "fails", "missing", "reads-all" - - -def put_a_locale(directory: Path, charmap: str, +#: HOW A `locale` OR AN `iconv` A TEST PUTS FIRST ON `$PATH` BEHAVES (#858). +#: Every `locale` answers `locale charmap` as glibc's does, from the first of +#: `$LC_ALL`, `$LC_CTYPE` and `$LANG` that is set and not empty: a marker +#: locale `xx_XX.` gives that charmap, `C.UTF-8` gives `UTF-8`, and +#: anything else `ANSI_X3.4-1968`. `HONEST` stops there; `FAILS` answers and +#: then exits 1; `MISSING` exits 127, as a shell answers a name it cannot +#: find. An `iconv` is this machine's (None), or `MISSING`, or `EMPTY`, one +#: that prints nothing and exits 0, as no working one does, or `MARKS`, one +#: that leaves `iconv-ran` beside itself and exits 1, so that a run which +#: must not ask it shows whether it did. +HONEST, FAILS, MISSING, EMPTY, MARKS = ( + "honest", "fails", "missing", "empty", "marks") + +#: What a `locale` stub answers `locale charmap` with, as described above. +CHARMAP_ANSWER = ( + '[ "$1" = charmap ] || exit 2\n' + 'name=${LC_ALL:-${LC_CTYPE:-$LANG}}\n' + 'case "$name" in\n' + 'xx_XX.*) printf \'%s\\n\' "${name#xx_XX.}" ;;\n' + "C.UTF-8) printf 'UTF-8\\n' ;;\n" + "*) printf 'ANSI_X3.4-1968\\n' ;;\n" + "esac\n") + + +def put_a_locale(directory: Path, kind: str = HONEST, iconv: str | None = None) -> None: - """A `locale` in `directory` that answers `locale charmap` with - `charmap`, or as `FAILS` or `MISSING` says, and an `iconv` beside it: - this machine's, or one that behaves as `MISSING` or `READS_ALL` says - (#858). No GB18030, GBK or Shift_JIS locale is installed here or on - CI, and none can be built, so it is `locale` that says the charset, - and bash's own locale stays `C` or `C.UTF-8`.""" - answers = {FAILS: "printf 'GB18030\\n'\nexit 1\n", MISSING: "exit 127\n"} - write_script(directory / "locale", "#!/bin/sh\n" + answers.get( - charmap, '[ "$1" = charmap ] || exit 2\n' - f"printf '%s\\n' {shlex.quote(charmap)}\n")) + """A `locale` in `directory` that behaves as `kind` says, and an + `iconv` beside it that behaves as `iconv` says (#858). No GB18030, GBK + or Shift_JIS locale is installed here or on CI, and none can be built, + so the person's locale is a marker name the stub alone knows, and bash + itself runs in `C` under it.""" + write_script(directory / "locale", "#!/bin/sh\n" + { + HONEST: CHARMAP_ANSWER, FAILS: CHARMAP_ANSWER + "exit 1\n", + MISSING: "exit 127\n"}[kind]) (directory / "locale").chmod(0o755) if iconv is None: real = shutil.which("iconv") @@ -967,119 +981,222 @@ def put_a_locale(directory: Path, charmap: str, put_on_path(directory, "iconv", real) return write_script(directory / "iconv", "#!/bin/sh\n" + { - MISSING: "exit 127\n", - READS_ALL: "cat >/dev/null\nprintf read\n"}[iconv]) + MISSING: "exit 127\n", EMPTY: "cat >/dev/null\nexit 0\n", + MARKS: f": > {shlex.quote(str(directory / 'iconv-ran'))}\nexit 1\n", + }[iconv]) (directory / "iconv").chmod(0o755) +def the_persons_locale(charmap: str) -> dict: + """The environment of a person whose locale reads `charmap`: `LANG` the + marker locale, and `LC_ALL` and `LC_CTYPE` empty, as unset (#858). bash + says nothing of a `LANG` that is not installed, and runs in `C`.""" + return {"LANG": f"xx_XX.{charmap}", "LC_ALL": "", "LC_CTYPE": ""} + + #: One of the 32 hanzi that GBK, GB18030 and CP936 read `0xC2 0x80` to #: `0xC2 0x9F` as, which in UTF-8 are a C1 control's two bytes: Python #: hands `chr(0x9B)` to a child as `0xC2 0x9B`, U+8078 in GB18030 (#858). A_HANZI_OF_C1_BYTES = "a" + C1_CSI + "b" -#: Names the bytes refuse that a multibyte charset reads as text, each with -#: that charset (#858): the first, the issue's and the last of the 32 hanzi, -#: so that a C1 range that stops one short at either end, where it decides +#: Names a multibyte charset reads as text, each with that charset and its +#: `iconv` (#858): the first, the issue's and the last of the 32 hanzi, so +#: that a C1 range that stops one short at either end, where it decides #: whether to ask the locale, is caught; U+00AD's two bytes, U+9885 in GBK; -#: U+2028's three, a kanji and a half-width katakana in Shift_JIS; and in +#: U+2028's three, a kanji and a half-width katakana in Shift_JIS; in #: Shift_JIS a half-width katakana (`0xC2`) before a character whose first #: byte is `0x81`, read as U+FF82 and then U+3005 or U+FF3F, whose UTF-8 -#: holds a `0x80` or a `0xBF`: the first and the last byte that continues a -#: character, which the count of the reading's characters must not count. +#: holds a `0x80` or a `0xBF`, the first and the last byte that continues a +#: character, which the count of the reading's characters must not count; +#: and a name the bytes do not refuse, which is never read again: the +#: `iconv` there would leave its mark. PLACED_AS_THE_LOCALE_READS = [ - pytest.param("GB18030", "a" + chr(0x80) + "b", id="gb18030-0xc2-0x80"), - pytest.param("GB18030", A_HANZI_OF_C1_BYTES, id="gb18030-0xc2-0x9b"), - pytest.param("GB18030", "a" + chr(0x9F) + "b", id="gb18030-0xc2-0x9f"), - pytest.param("GBK", "a" + chr(0xAD) + "b", id="gbk-u+00ad-bytes"), - pytest.param("SHIFT_JIS", "a" + chr(0x2028) + "b", + pytest.param("GB18030", None, "a" + chr(0x80) + "b", + id="gb18030-0xc2-0x80"), + pytest.param("GB18030", None, A_HANZI_OF_C1_BYTES, + id="gb18030-0xc2-0x9b"), + pytest.param("GB18030", None, "a" + chr(0x9F) + "b", + id="gb18030-0xc2-0x9f"), + pytest.param("GBK", None, "a" + chr(0xAD) + "b", id="gbk-u+00ad-bytes"), + pytest.param("SHIFT_JIS", None, "a" + chr(0x2028) + "b", id="shift_jis-u+2028-bytes"), - pytest.param("SHIFT_JIS", "a" + chr(0x81) + "Xb", + pytest.param("SHIFT_JIS", None, "a" + chr(0x81) + "Xb", id="shift_jis-0xc2-0x81-0x58"), - pytest.param("SHIFT_JIS", "a" + chr(0x81) + "Qb", + pytest.param("SHIFT_JIS", None, "a" + chr(0x81) + "Qb", id="shift_jis-0xc2-0x81-0x51"), + pytest.param("GB18030", MARKS, "caf" + chr(0xE9), + id="no-byte-refused-never-read-again"), ] -#: Names the bytes refuse where nothing reads them again, and what the -#: refusal says (#858): under UTF-8, three of `PLACED_AS_THE_LOCALE_READS`, -#: in #804's and #837's words; where `locale` fails or is not there; where -#: `iconv` is not there; where GB18030 cannot read a byte (`0xFF`); under a +#: Names refused under a marker locale, and what the refusal says (#858). +#: The locale's reading is checked itself: GB18030 writes U+009B as +#: `0x81 0x30 0x83 0x37` and U+202E as `0x81 0x36 0xA7 0x31`, which are no +#: UTF-8, so a name holding either beside a hanzi the bytes refuse is +#: refused as its reading says. And the bytes decide, in #804's and #837's +#: words, where `locale` fails or is not there; where `iconv` is not there, +#: or prints nothing; where GB18030 cannot read a byte (`0xFF`); under a #: single-byte charset, as Latin-1 reads U+FEFF's three bytes, each alone; #: with a newline at the end, which `$(…)` drops from a reading; and where #: `locale` answers nothing, UTF-8 or ASCII, under which `iconv` is never -#: asked, though the one there would read anything as text. +#: asked. REFUSED_AS_THE_BYTES_SAY = [ - pytest.param("UTF-8", None, A_HANZI_OF_C1_BYTES, - HOLDS_A_CONTROL_CHARACTER, id="utf-8-0xc2-0x9b"), - pytest.param("UTF-8", None, "a" + chr(0xAD) + "b", - bin_dir_drawn_as_another("U+00AD"), id="utf-8-u+00ad"), - pytest.param("UTF-8", None, "a" + chr(0x2028) + "b", - bin_dir_drawn_as_another("U+2028"), id="utf-8-u+2028"), - pytest.param(FAILS, None, A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, - id="a-locale-that-fails"), - pytest.param(MISSING, None, A_HANZI_OF_C1_BYTES, + pytest.param("GB18030", HONEST, None, + A_HANZI_OF_C1_BYTES + "\udc81" + "0" + "\udc83" + "7", + HOLDS_A_CONTROL_CHARACTER, id="gb18030-reads-u+009b"), + pytest.param("GB18030", HONEST, None, + A_HANZI_OF_C1_BYTES + "\udc81" + "6" + "\udca7" + "1", + bin_dir_drawn_as_another("U+202E"), + id="gb18030-reads-u+202e"), + pytest.param("GB18030", FAILS, None, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, id="a-locale-that-fails"), + pytest.param("GB18030", MISSING, None, A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, id="no-locale"), - pytest.param("GB18030", MISSING, A_HANZI_OF_C1_BYTES, + pytest.param("GB18030", HONEST, MISSING, A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, id="no-iconv"), - pytest.param("GB18030", None, A_HANZI_OF_C1_BYTES + chr(0xDCFF), + pytest.param("GB18030", HONEST, EMPTY, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, id="an-iconv-that-prints-nothing"), + pytest.param("GB18030", HONEST, None, A_HANZI_OF_C1_BYTES + chr(0xDCFF), HOLDS_A_CONTROL_CHARACTER, id="a-byte-gb18030-cannot-read"), - pytest.param("ISO-8859-1", None, "a" + chr(0xFEFF) + "b", + pytest.param("ISO-8859-1", HONEST, None, "a" + chr(0xFEFF) + "b", bin_dir_drawn_as_another("U+FEFF"), id="a-single-byte-charset"), - pytest.param("GB18030", None, A_HANZI_OF_C1_BYTES + "\n", + pytest.param("GB18030", HONEST, None, A_HANZI_OF_C1_BYTES + "\n", HOLDS_A_CONTROL_CHARACTER, id="a-newline-at-the-end"), - *(pytest.param(answer, READS_ALL, A_HANZI_OF_C1_BYTES, + *(pytest.param(answer, HONEST, MARKS, A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, id=f"iconv-unasked-under-{answer or 'no-answer'}") for answer in ("", "UTF-8", "utf8", "ANSI_X3.4-1968", "US-ASCII")), ] +#: Names refused when the person's locale is `C` or `C.UTF-8` themselves +#: (#858), in #804's and #837's words, `iconv` never asked: three of +#: `PLACED_AS_THE_LOCALE_READS`. +REFUSED_UNDER_C_OR_UTF_8 = [ + pytest.param(A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, + id="0xc2-0x9b"), + pytest.param("a" + chr(0xAD) + "b", bin_dir_drawn_as_another("U+00AD"), + id="u+00ad"), + pytest.param("a" + chr(0x2028) + "b", bin_dir_drawn_as_another("U+2028"), + id="u+2028"), +] + -def install_under_a_locale(tmp_path: Path, typed: str, charmap: str, - iconv: str | None, locale: str): - """`--install` into `typed`, under bash's `locale`, with - `put_a_locale`'s `locale` and `iconv` first on `$PATH` (#858).""" +def install_under_a_locale(tmp_path: Path, typed: str, environment: dict, + kind: str, iconv: str | None): + """`--install` into `typed`, with `put_a_locale`'s `locale` and `iconv` + first on `$PATH` in `tmp_path/tools`, and `environment` naming the + person's locale (#858).""" tools = tmp_path / "tools" tools.mkdir() - put_a_locale(tools, charmap, iconv) + put_a_locale(tools, kind, iconv) return run_cmd("--install", home=tmp_path, cwd=tmp_path, env={ - "OPENREPOSHAPE_BIN_DIR": typed, "LC_ALL": locale, + "OPENREPOSHAPE_BIN_DIR": typed, **environment, "PATH": f"{tools}{os.pathsep}{os.environ['PATH']}"}) @NO_NON_ASCII_INTO_GIT_BASH -@pytest.mark.parametrize("locale", LOCALES) -@pytest.mark.parametrize("charmap, name", PLACED_AS_THE_LOCALE_READS) +@pytest.mark.parametrize("charmap, iconv, name", PLACED_AS_THE_LOCALE_READS) def test_a_bin_dir_a_multibyte_locale_reads_as_text_is_placed( - tmp_path, charmap, name, locale): + tmp_path, charmap, iconv, name): """The control check and `misdrawn_characters` look for UTF-8's bytes, and GB18030 reads `0xC2 0x9B` as a hanzi, not as U+009B: the directory was refused as holding a control character, in a locale where it holds - none (#858). Where the bytes are refused, `locale charmap` is asked, - and a multibyte charset's reading of the name, through `iconv`, has - the last word. So a name GB18030, GBK or Shift_JIS reads as text is - placed, under bash's `C` and `C.UTF-8` alike.""" + none (#858). Where the bytes are refused, `locale charmap` is asked + under the person's own `LANG`, and a multibyte charset's reading of the + name, through `iconv`, has the last word. So a name GB18030, GBK or + Shift_JIS reads as text is placed; and a name the bytes do not refuse + is placed without `iconv` being asked.""" result = install_under_a_locale( - tmp_path, f"{bash_path(tmp_path)}/{name}", charmap, None, locale) + tmp_path, f"{bash_path(tmp_path)}/{name}", + the_persons_locale(charmap), HONEST, iconv) assert result.returncode == 0, result.stdout + result.stderr assert (tmp_path / name / "openRepoShape").read_bytes() \ == COMMAND.read_bytes() + assert not (tmp_path / "tools" / "iconv-ran").exists() @NO_NON_ASCII_INTO_GIT_BASH -@pytest.mark.parametrize("locale", LOCALES) -@pytest.mark.parametrize("charmap, iconv, name, said", +@pytest.mark.parametrize("charmap, kind, iconv, name, said", REFUSED_AS_THE_BYTES_SAY) -def test_a_bin_dir_is_refused_as_its_bytes_say_where_nothing_reads_them_again( - tmp_path, charmap, iconv, name, said, locale): - """Where the locale's charset is UTF-8, ASCII, single-byte or not said, - where `iconv` is not there or cannot read the name, the bytes decide, - as before (#858): refused, exit 2, nothing placed, never echoed, and in - #804's or #837's words. A newline at the end is looked for in the bytes - as typed, not in the reading.""" +def test_a_bin_dir_is_refused_as_its_bytes_or_its_reading_say( + tmp_path, charmap, kind, iconv, name, said): + """Where the locale's reading still holds a C1 control or a character + drawn as another, it is refused as the reading says; and where the + locale's charset is UTF-8, ASCII, single-byte or not said, where + `iconv` is not there, prints nothing or cannot read the name, the bytes + decide, as before (#858): refused, exit 2, nothing placed, never + echoed, in #804's or #837's words. A newline at the end is looked for + in the bytes as typed, not in the reading.""" typed = f"{bash_path(tmp_path)}/{name}" - result = install_under_a_locale(tmp_path, typed, charmap, iconv, locale) + result = install_under_a_locale(tmp_path, typed, + the_persons_locale(charmap), kind, iconv) assert_a_bin_dir_is_refused(result, typed, said, COMMAND_REFUSAL_ENDS) assert [path.name for path in tmp_path.iterdir()] == ["tools"] + assert not (tmp_path / "tools" / "iconv-ran").exists() + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("name, said", REFUSED_UNDER_C_OR_UTF_8) +def test_a_bin_dir_is_refused_as_its_bytes_say_under_c_or_c_utf_8( + tmp_path, name, said, locale): + """The person's locale is `C` or `C.UTF-8`, as `LC_ALL` says, and + `locale charmap` answers `ANSI_X3.4-1968` or `UTF-8`, as glibc's does: + the bytes decide, in #804's and #837's words, and `iconv` is never + asked (#858).""" + typed = f"{bash_path(tmp_path)}/{name}" + result = install_under_a_locale(tmp_path, typed, {"LC_ALL": locale}, + HONEST, MARKS) + assert_a_bin_dir_is_refused(result, typed, said, COMMAND_REFUSAL_ENDS) + assert not (tmp_path / "tools" / "iconv-ran").exists() + + +#: The Python codec that reads as each charset of +#: `PLACED_AS_THE_LOCALE_READS` does, and Latin-1's. +CODECS = {"GB18030": "gb18030", "GBK": "gbk", "SHIFT_JIS": "shift_jis", + "ISO-8859-1": "latin-1"} + + +@NO_NON_ASCII_INTO_GIT_BASH +@pytest.mark.parametrize("locale", LOCALES) +@pytest.mark.parametrize("name", ["openRepoShape", "install.sh"]) +def test_locale_reading_counts_bytes_whatever_bash_itself_reads( + tmp_path, name, locale): + """`locale_reading` alone, lifted out of each file, with a `locale` + function that answers each name's charset, under bash's own `C` and + `C.UTF-8` (#858). In a GB18030 locale bash itself reads GB18030, and + no such locale can be installed here, so `C.UTF-8` stands in for a + multibyte locale of bash's own. For each name of + `PLACED_AS_THE_LOCALE_READS` the bytes refuse, the reading is the one + Python's codec gives; for Latin-1's U+FEFF bytes and a byte GB18030 + cannot read there is none. So the count is of bytes in either.""" + rows = [(param.values[0], param.values[2]) + for param in PLACED_AS_THE_LOCALE_READS + if param.values[1] is None] + rows += [("ISO-8859-1", "a" + chr(0xFEFF) + "b"), + ("GB18030", A_HANZI_OF_C1_BYTES + chr(0xDCFF))] + script = tmp_path / "reading.sh" + write_script(script, f"LC_ALL={locale}\n" + "locale() { printf '%s\\n' \"$CHARMAP\"; }\n" + + shell_function(REPO / name, "locale_reading") + + 'while IFS= read -r -d "" CHARMAP && ' + 'IFS= read -r -d "" value; do\n' + '\tif reading="$(locale_reading "$value")"; then\n' + "\t\tprintf 'read %s\\0' \"$reading\"\n" + "\telse\n" + "\t\tprintf 'none\\0'\n" + "\tfi\n" + "done\n") + ran = subprocess.run( + [BASH, bash_path(script)], capture_output=True, check=False, + input=b"".join(charmap.encode("ascii") + b"\0" + + value.encode("utf-8", "surrogateescape") + b"\0" + for charmap, value in rows), + env=command_env(setup_sh=None)) + assert ran.returncode == 0, ran.stderr.decode("utf-8", "replace") + expected = [b"read " + value.encode("utf-8", "surrogateescape").decode( + CODECS[charmap]).encode("utf-8") for charmap, value in rows[:-2]] + assert ran.stdout.split(b"\0")[:-1] == [*expected, b"none", b"none"] #: A directory that already holds pipx's link (#563), whose name holds a From 8bb85cbfaf53e10287426aa2c25f127a0faee589 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 03:26:39 +0000 Subject: [PATCH 8/8] tests: an `iconv` that prints the whole reading of a `$OPENREPOSHAPE_BIN_DIR` and then fails gives no reading, so the first `iconv`'s exit is pinned beside the round trip (#858) Fix round 1 on rv858's review of #867, its last step. With the round trip in `locale_reading`, dropping the first `iconv`'s `|| return 1` (F07) survived the mutant table: a failing `iconv` that printed part of a reading, or nothing, is refused by the round trip anyway. One that prints the whole reading and then exits 1 is not: the round trip passes. A row of `REFUSED_AS_THE_BYTES_SAY` now has such an `iconv` (`READS_THEN_FAILS`, this machine's run in full, then `exit 1`) read a hanzi of the C1 bytes under GB18030, through both files: refused as a control character, as the bytes say, and install.sh before any request. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_openreposhape_command.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index 4f51d865..a0516af1 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -947,11 +947,13 @@ def test_install_sh_carries_the_launchers_locale_reading_as_it_is(): #: anything else `ANSI_X3.4-1968`. `HONEST` stops there; `FAILS` answers and #: then exits 1; `MISSING` exits 127, as a shell answers a name it cannot #: find. An `iconv` is this machine's (None), or `MISSING`, or `EMPTY`, one -#: that prints nothing and exits 0, as no working one does, or `MARKS`, one +#: that prints nothing and exits 0, as no working one does, or +#: `READS_THEN_FAILS`, this machine's run in full and then exit 1, so that +#: the whole reading is printed by an `iconv` that failed, or `MARKS`, one #: that leaves `iconv-ran` beside itself and exits 1, so that a run which #: must not ask it shows whether it did. -HONEST, FAILS, MISSING, EMPTY, MARKS = ( - "honest", "fails", "missing", "empty", "marks") +HONEST, FAILS, MISSING, EMPTY, READS_THEN_FAILS, MARKS = ( + "honest", "fails", "missing", "empty", "reads-then-fails", "marks") #: What a `locale` stub answers `locale charmap` with, as described above. CHARMAP_ANSWER = ( @@ -975,13 +977,14 @@ def put_a_locale(directory: Path, kind: str = HONEST, HONEST: CHARMAP_ANSWER, FAILS: CHARMAP_ANSWER + "exit 1\n", MISSING: "exit 127\n"}[kind]) (directory / "locale").chmod(0o755) + real = shutil.which("iconv") + assert real, "this machine has no iconv, which these cases read with" if iconv is None: - real = shutil.which("iconv") - assert real, "this machine has no iconv, which these cases read with" put_on_path(directory, "iconv", real) return write_script(directory / "iconv", "#!/bin/sh\n" + { MISSING: "exit 127\n", EMPTY: "cat >/dev/null\nexit 0\n", + READS_THEN_FAILS: f'{shlex.quote(real)} "$@"\nexit 1\n', MARKS: f": > {shlex.quote(str(directory / 'iconv-ran'))}\nexit 1\n", }[iconv]) (directory / "iconv").chmod(0o755) @@ -1034,11 +1037,12 @@ def the_persons_locale(charmap: str) -> dict: #: UTF-8, so a name holding either beside a hanzi the bytes refuse is #: refused as its reading says. And the bytes decide, in #804's and #837's #: words, where `locale` fails or is not there; where `iconv` is not there, -#: or prints nothing; where GB18030 cannot read a byte (`0xFF`); under a -#: single-byte charset, as Latin-1 reads U+FEFF's three bytes, each alone; -#: with a newline at the end, which `$(…)` drops from a reading; and where -#: `locale` answers nothing, UTF-8 or ASCII, under which `iconv` is never -#: asked. +#: prints nothing, or prints the whole reading and then fails, which the +#: round trip alone would let through; where GB18030 cannot read a byte +#: (`0xFF`); under a single-byte charset, as Latin-1 reads U+FEFF's three +#: bytes, each alone; with a newline at the end, which `$(…)` drops from a +#: reading; and where `locale` answers nothing, UTF-8 or ASCII, under which +#: `iconv` is never asked. REFUSED_AS_THE_BYTES_SAY = [ pytest.param("GB18030", HONEST, None, A_HANZI_OF_C1_BYTES + "\udc81" + "0" + "\udc83" + "7", @@ -1055,6 +1059,9 @@ def the_persons_locale(charmap: str) -> dict: HOLDS_A_CONTROL_CHARACTER, id="no-iconv"), pytest.param("GB18030", HONEST, EMPTY, A_HANZI_OF_C1_BYTES, HOLDS_A_CONTROL_CHARACTER, id="an-iconv-that-prints-nothing"), + pytest.param("GB18030", HONEST, READS_THEN_FAILS, A_HANZI_OF_C1_BYTES, + HOLDS_A_CONTROL_CHARACTER, + id="an-iconv-that-reads-then-fails"), pytest.param("GB18030", HONEST, None, A_HANZI_OF_C1_BYTES + chr(0xDCFF), HOLDS_A_CONTROL_CHARACTER, id="a-byte-gb18030-cannot-read"), pytest.param("ISO-8859-1", HONEST, None, "a" + chr(0xFEFF) + "b",