fix: normalize semaphore capacity as decimal

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [Unreleased]
+
+### Fixed
+
+- Semaphore capacity is validated as a bounded positive decimal and normalized before storage, arithmetic, and JSON serialization. Leading-zero values such as `01`, `08`, and `010` keep their decimal meaning, including when reading metadata written by older versions. Invalid stored capacities fail with `store-read` (#35).
+
 All notable changes to this project are recorded here. The format follows Keep a Changelog; versions follow SemVer.
 
 ## [Unreleased]

Makefile

@@ -14,6 +14,7 @@ lint:
 
 test:
 	bash test/test.sh
+	python3 test/capacity.py
 
 test-docker: # the same suite inside the official bash image, for a wall between the tests and your machine
 	docker run --rm -v "$(CURDIR)":/src -w /src bash:5.2 bash -c 'apk add --no-cache git python3 py3-jsonschema >/dev/null && git config --global user.email t@example.invalid && git config --global user.name t && bash test/test.sh'

README.md

@@ -261,6 +261,8 @@ In summary, "all or nothing" is never a loop with a rollback. It is one stanza,
 
 A path lock says one holder. Some resources are better described by a number: two GPUs, five build agents. This section shows how git-locks gives a named resource a capacity while keeping the same atomicity, and it needs one new idea, a generation token, that the reader has all the pieces for.
 
+Capacity is a positive decimal integer from `1` through `9223372036854775807`. Leading zeros are accepted and normalized: `--capacity 010` means ten and emits JSON `10`. Reads also normalize leading-zero capacities stored by older versions without rewriting their metadata. Invalid or out-of-range input is refused before semaphore refs are created; invalid stored capacity is a `store-read` error.
+
 A semaphore is three kinds of ref under `refs/locks/sem/<name>/`. `meta` points at a blob holding the capacity. `slots/<job>` is one ref per holder, pointing at a slot record with the holder and an expiry, exactly like a lock record. And `gen` points at a blob whose only purpose is to change: every transaction on the semaphore writes a fresh generation blob and `update`s `gen` from the generation it read. Two acquirers that both read "2 of 3 live" both try to move `gen` from the same old value; git lets exactly one through, and the other re-reads and finds the semaphore full. Here is the example's semaphore, capacity 2, filled by alice and bob, then refused to carol:
 
 ```text

bin/git-locks

@@ -1733,6 +1733,19 @@ with_release_sem() { # record -> releases this invocation's slot, if it is still
 # acquirers who both counted "n of N live" contend on one compare-and-swap and
 # exactly one commits. The other re-reads.
 
+valid_capacity() { # VAR value: canonical positive decimal in Bash's signed 64-bit arithmetic range
+  [[ "$2" =~ ^[0-9]+$ ]] || return 1
+  local _capacity="${2#"${2%%[!0]*}"}" # strip leading zeros without evaluating input as arithmetic
+  [[ -n "${_capacity}" ]] || return 1
+  ((${#_capacity} <= 19)) || return 1
+  # Compare equal-length decimal strings: arithmetic would overflow before rejecting the input.
+  # shellcheck disable=SC2071
+  if ((${#_capacity} == 19)) && [[ "${_capacity}" > 9223372036854775807 ]]; then
+    return 1
+  fi
+  printf -v "$1" '%s' "${_capacity}"
+}
+
 sem_meta_ref() { printf '%s/sem/%s/meta' "${NS}" "$1"; }
 sem_gen_ref() { printf '%s/sem/%s/gen' "${NS}" "$1"; }
 sem_slot_ref() { printf '%s/sem/%s/slots/%s' "${NS}" "$1" "$2"; }
@@ -1771,6 +1784,7 @@ sem_read() {      # name -> 0, or 1 when the semaphore does not exist
   SEM_META_OID="$(ref_oid "${mref}")"
   [[ -n "${SEM_META_OID}" ]] || return 1
   SEM_CAP="$(field "${SEM_META_OID}" capacity)"
+  valid_capacity SEM_CAP "${SEM_CAP}" || store_error "semaphore ${name} has an invalid capacity"
   gref="$(sem_gen_ref "${name}")"
   SEM_GEN_OID="$(ref_oid "${gref}")"
   SLOT_JOBS=()
@@ -2008,7 +2022,7 @@ cmd_sem() {
   done
   case "${verb}" in
     create)
-      [[ "${capacity}" =~ ^[0-9]+$ && "${capacity}" -gt 0 ]] || fail '--capacity is a positive number' 2
+      valid_capacity capacity "${capacity}" || fail '--capacity is a decimal integer from 1 through 9223372036854775807' 2
       if sem_read "${name}"; then
         sem_refusal "${name}" exists
         exit 1

lib/170-semaphores.sh

@@ -5,6 +5,19 @@
 # acquirers who both counted "n of N live" contend on one compare-and-swap and
 # exactly one commits. The other re-reads.
 
+valid_capacity() { # VAR value: canonical positive decimal in Bash's signed 64-bit arithmetic range
+  [[ "$2" =~ ^[0-9]+$ ]] || return 1
+  local _capacity="${2#"${2%%[!0]*}"}" # strip leading zeros without evaluating input as arithmetic
+  [[ -n "${_capacity}" ]] || return 1
+  ((${#_capacity} <= 19)) || return 1
+  # Compare equal-length decimal strings: arithmetic would overflow before rejecting the input.
+  # shellcheck disable=SC2071
+  if ((${#_capacity} == 19)) && [[ "${_capacity}" > 9223372036854775807 ]]; then
+    return 1
+  fi
+  printf -v "$1" '%s' "${_capacity}"
+}
+
 sem_meta_ref() { printf '%s/sem/%s/meta' "${NS}" "$1"; }
 sem_gen_ref() { printf '%s/sem/%s/gen' "${NS}" "$1"; }
 sem_slot_ref() { printf '%s/sem/%s/slots/%s' "${NS}" "$1" "$2"; }
@@ -43,6 +56,7 @@ sem_read() {      # name -> 0, or 1 when the semaphore does not exist
   SEM_META_OID="$(ref_oid "${mref}")"
   [[ -n "${SEM_META_OID}" ]] || return 1
   SEM_CAP="$(field "${SEM_META_OID}" capacity)"
+  valid_capacity SEM_CAP "${SEM_CAP}" || store_error "semaphore ${name} has an invalid capacity"
   gref="$(sem_gen_ref "${name}")"
   SEM_GEN_OID="$(ref_oid "${gref}")"
   SLOT_JOBS=()
@@ -280,7 +294,7 @@ cmd_sem() {
   done
   case "${verb}" in
     create)
-      [[ "${capacity}" =~ ^[0-9]+$ && "${capacity}" -gt 0 ]] || fail '--capacity is a positive number' 2
+      valid_capacity capacity "${capacity}" || fail '--capacity is a decimal integer from 1 through 9223372036854775807' 2
       if sem_read "${name}"; then
         sem_refusal "${name}" exists
         exit 1

test/capacity.py

@@ -0,0 +1,127 @@
+#!/usr/bin/env python3
+"""Exercise decimal capacity through the CLI with an independent integer oracle."""
+import concurrent.futures
+import json
+import os
+from pathlib import Path
+import random
+import subprocess
+import tempfile
+
+import jsonschema
+
+ROOT = Path(__file__).resolve().parents[1]
+CLI = ROOT / 'bin/git-locks'
+SCHEMA = json.loads((ROOT / 'schema/git-locks.schema.json').read_text())
+VALIDATOR = jsonschema.Draft202012Validator(SCHEMA)
+SEED = 3507
+LIMIT = 9223372036854775807
+checks = 0
+
+
+def check(condition, message):
+    global checks
+    assert condition, message
+    checks += 1
+
+
+with tempfile.TemporaryDirectory(prefix='git-locks-capacity-') as tmp:
+    base = Path(tmp)
+    env = {key: value for key, value in os.environ.items()
+           if not key.startswith('GIT_')}
+    env.update(HOME=tmp, GIT_LOCKS_STORE=str(base / 'store.git'),
+               GIT_LOCKS_NOW='1000000', LC_ALL='C')
+
+    def run(*args, expected=0):
+        result = subprocess.run([str(CLI), *args], cwd=tmp, env=env,
+                                text=True, capture_output=True, timeout=45)
+        check(result.returncode == expected,
+              f'{args}: exit {result.returncode}, expected {expected}: {result.stderr}')
+        data = []
+        for output in (result.stdout, result.stderr):
+            for line in output.splitlines():
+                value = json.loads(line)
+                VALIDATOR.validate(value)
+                data.append(value)
+        check(bool(data), f'{args}: missing structured output')
+        return data
+
+    def git(*args, input=None):
+        return subprocess.run(['git', '--git-dir=' + env['GIT_LOCKS_STORE'], *args],
+                              cwd=tmp, env=env, text=True, input=input,
+                              capture_output=True, check=True).stdout.strip()
+
+    rng = random.Random(SEED)
+    values = ['1', '01', '08', '010', str(LIMIT), '000' + str(LIMIT), '0' * 256 + '8']
+    values += ['0' * rng.randrange(1, 25) + str(rng.randrange(1, 1000000)) for _ in range(32)]
+    for index, value in enumerate(values):
+        name = f'valid-{index}'
+        want = int(value, 10)
+        check(run('sem', 'create', name, '--capacity', value)[0]['capacity'] == want,
+              f'create did not normalize {value!r}')
+        check(run('sem', 'show', name)[0]['capacity'] == want, 'show capacity mismatch')
+        meta = git('show', f'refs/locks/sem/{name}/meta')
+        check(f'capacity: {want}' in meta.splitlines(), 'stored capacity is not canonical')
+        check(run('sem', 'acquire', name, '--job', 'a', '--holder', 'alice')[0]['capacity'] == want,
+              'acquire capacity mismatch')
+        check(run('sem', 'release', name, '--job', 'a')[0]['capacity'] == want,
+              'release capacity mismatch')
+        run('sem', 'delete', name)
+
+    invalid = ['', '0', '00', '-1', '+1', '1.0', '1e2', ' 1', '1 ', '08x', '１２',
+               str(LIMIT + 1), str(2**64 + 1), '9' * 100, '0' * 100 + str(LIMIT + 1)]
+    invalid += [str(rng.randrange(LIMIT + 1, 2**100)) for _ in range(16)]
+    before = git('for-each-ref', '--format=%(refname) %(objectname)', 'refs/locks/')
+    for index, value in enumerate(invalid):
+        result = run('sem', 'create', f'invalid-{index}', '--capacity', value, expected=2)
+        check(result[0]['reason'] == 'usage', 'invalid capacity is not a usage error')
+        check(git('for-each-ref', '--format=%(refname) %(objectname)', 'refs/locks/') == before,
+              'invalid input changed authoritative refs')
+
+    # Stores written by 0.7.0 may have leading zeros; reads normalize without rewriting.
+    run('sem', 'create', 'legacy', '--capacity', '1')
+    meta = git('show', 'refs/locks/sem/legacy/meta')
+    oid = git('hash-object', '-w', '--stdin', input=meta.replace('capacity: 1', 'capacity: 01') + '\n')
+    git('update-ref', 'refs/locks/sem/legacy/meta', oid)
+    check(run('sem', 'show', 'legacy')[0]['capacity'] == 1, 'legacy show is not normalized')
+    check(run('sem', 'list')[0]['capacity'] == 1, 'legacy list is not normalized')
+    check(git('rev-parse', 'refs/locks/sem/legacy/meta') == oid, 'read rewrote legacy metadata')
+    run('sem', 'acquire', 'legacy', '--job', 'first', '--holder', 'alice')
+    refused = run('sem', 'acquire', 'legacy', '--job', 'second', '--holder', 'bob', expected=1)[0]
+    check(refused['reason'] == 'capacity' and refused['capacity'] == 1, 'legacy capacity not enforced')
+    run('sem', 'release', 'legacy', '--job', 'first')
+    run('sem', 'delete', 'legacy')
+
+    # A fresh metadata record can be corrupt independently of its JSON serialization.
+    for value in ('0', '08x', str(LIMIT + 1)):
+        run('sem', 'create', 'bad-meta', '--capacity', '1')
+        meta = git('show', 'refs/locks/sem/bad-meta/meta')
+        original = git('rev-parse', 'refs/locks/sem/bad-meta/meta')
+        oid = git('hash-object', '-w', '--stdin', input=meta.replace('capacity: 1', 'capacity: ' + value) + '\n')
+        git('update-ref', 'refs/locks/sem/bad-meta/meta', oid)
+        result = run('sem', 'show', 'bad-meta', expected=2)
+        check(result[0]['reason'] == 'store-read', 'bad stored capacity did not fail closed')
+        check(git('rev-parse', 'refs/locks/sem/bad-meta/meta') == oid, 'bad read changed metadata')
+        git('update-ref', 'refs/locks/sem/bad-meta/meta', original)
+        run('sem', 'delete', 'bad-meta')
+
+    run('sem', 'create', 'race', '--capacity', '03')
+
+    def contender(index):
+        result = subprocess.run([str(CLI), 'sem', 'acquire', 'race', '--job', f'r{index}',
+                                 '--holder', f'h{index}'], cwd=tmp, env=env, text=True,
+                                capture_output=True, timeout=45)
+        assert result.returncode in (0, 1), result
+        for line in (result.stdout + result.stderr).splitlines():
+            VALIDATOR.validate(json.loads(line))
+        return result.returncode
+
+    with concurrent.futures.ThreadPoolExecutor(max_workers=12) as pool:
+        statuses = list(pool.map(contender, range(12)))
+    check(statuses.count(0) == 3, f'12 racers on capacity 03 had {statuses.count(0)} winners')
+    observed = run('sem', 'show', 'race')[0]
+    check(observed['capacity'] == 3 and observed['live'] == 3 and len(observed['slots']) == 3,
+          'post-race slot state violates capacity')
+    check(run('doctor')[0]['healthy'], 'post-race doctor unhealthy')
+
+print(f'capacity: {checks} checks passed; seed {SEED}; {len(values)} valid and {len(invalid)} invalid inputs; 12 racers / 3 winners')

test/test.sh

@@ -727,6 +727,20 @@ valid "sem delete line" "${out}"
 git-locks sem show batch >/dev/null 2>&1
 check "a deleted semaphore is gone" "$?" "1"
 
+# Decimal capacity must preserve its numeric value in storage and every JSON line.
+for capacity_case in '01:1' '08:8' '010:10'; do
+  capacity_input="${capacity_case%:*}"
+  capacity_want="${capacity_case#*:}"
+  out="$(git-locks sem create "decimal-${capacity_want}" --capacity "${capacity_input}" 2>&1)"
+  check "capacity ${capacity_input} creates a semaphore" "$?" "0"
+  jfields "capacity ${capacity_input} emits decimal ${capacity_want}" "${out}" "capacity=${capacity_want}"
+  valid "capacity ${capacity_input} create line" "${out}"
+  out="$(git-locks sem show "decimal-${capacity_want}" 2>&1)"
+  check "capacity ${capacity_input} can be read" "$?" "0"
+  jfields "capacity ${capacity_input} reads decimal ${capacity_want}" "${out}" "capacity=${capacity_want}"
+  valid "capacity ${capacity_input} show line" "${out}"
+done
+
 # exactly K winners under contention
 git-locks sem create race --capacity 3 >/dev/null 2>&1
 wins=0