fix(package): parse manifests by key name and decode JSON escapes

.github/workflows/zsh-n.yml

@@ -18,6 +18,7 @@ on:
       - "tests/hook-ownership.zsh"
       - "tests/load-object-status.zsh"
       - "tests/message-formatting.zsh"
+      - "tests/package-manifest-parsing.zsh"
       - "tests/path-resolution.zsh"
       - "tests/parallel-update.zsh"
       - "tests/plugin-autoload-fpath-scope.zsh"
@@ -47,6 +48,7 @@ on:
       - "tests/hook-ownership.zsh"
       - "tests/load-object-status.zsh"
       - "tests/message-formatting.zsh"
+      - "tests/package-manifest-parsing.zsh"
       - "tests/path-resolution.zsh"
       - "tests/parallel-update.zsh"
       - "tests/plugin-autoload-fpath-scope.zsh"
@@ -143,6 +145,17 @@ jobs:
       - name: Test version reporting
         run: zsh tests/version-reporting.zsh
 
+  package-manifest-parsing:
+    name: Package manifest parsing
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out code
+        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      - name: Install Zsh
+        run: sudo apt update && sudo apt-get install -yq zsh
+      - name: Test package manifest parsing
+        run: zsh -f tests/package-manifest-parsing.zsh
+
   path-resolution:
     name: Path resolution
     runs-on: ubuntu-latest

lib/zsh/install.zsh

@@ -6,8 +6,43 @@
 
 builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col-error]}ERROR:%f%b Couldn't find ${ZI[col-obj]}/lib/zsh/side.zsh%f%b."; return 1; }
 
+# FUNCTION: .zi-unescape-json-string [[[
+# Translates the escapes of a JSON string body into the characters they denote
+# and returns the result in $REPLY. Handles the eight two-character escapes and
+# \uXXXX in the basic multilingual plane; a surrogate pair or an unrecognized
+# escape is left exactly as written, as is a malformed \u.
+.zi-unescape-json-string() {
+  builtin emulate -LR zsh ${=${options[xtrace]:#off}:+-o xtrace}
+  builtin setopt extended_glob warn_create_global typeset_silent
+
+  local ___rest=$1 ___out= ___esc ___tail
+  local -a match mbegin mend
+  local -A ___map=( \" \" \\ \\ / / b $'\b' f $'\f' n $'\n' r $'\r' t $'\t' )
+  integer ___code
+
+  while [[ $___rest = (#b)([^\\]#)\\(?)(*) ]]; do
+    ___out+=$match[1] ___esc=$match[2] ___tail=$match[3]
+    if [[ $___esc == u && $___tail == (#b)([0-9a-fA-F](#c4))(*) ]] {
+      ___code=16#$match[1]
+      ___out+=${(#)___code} ___rest=$match[2]
+    } elif (( ${+___map[$___esc]} )) {
+      ___out+=$___map[$___esc] ___rest=$___tail
+    } else {
+      ___out+="\\$___esc" ___rest=$___tail
+    }
+  done
+  typeset -g REPLY=$___out$___rest
+} # ]]]
 # FUNCTION: .zi-parse-json [[[
 # Retrievies the ice-list from given profile from the JSON of the package.json.
+#
+# Provenance: this began as `@str-parse-json' in z-shell/zsh-string-lib and is
+# now a maintained fork, not a mirror of it. Known divergences, all deliberate:
+# `___pair_map' omits the `('/`)' pair, which JSON never uses; the key lookup
+# selects the smallest object declaring the key instead of one that opens with
+# it; and string bodies are unescaped as they are captured. Do not swap in the
+# library copy without re-running tests/package-manifest-parsing.zsh, which
+# fails against it.
 .zi-parse-json() {
   builtin emulate -LR zsh ${=${options[xtrace]:#off}:+-o xtrace}
   builtin setopt extended_glob warn_create_global typeset_silent
@@ -56,7 +91,10 @@ builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col
 
       [[ ${match[1]} = \" && $___quoting != \' ]] && \
         if [[ $___quoting = '"' ]]; then
-          ___Strings[$___level/${___Counts[$___level]}]+=" ${(q)___input[___sidx,___idx-1]}"
+          # A JSON string body carries escapes; store what they denote, not the
+          # backslashes, so ices are not later executed with a stray `\'.
+          .zi-unescape-json-string "${___input[___sidx,___idx-1]}"
+          ___Strings[$___level/${___Counts[$___level]}]+=" ${(q)REPLY}"
           ___quoting=""
         else
           ___had_quoted_value=1
@@ -98,8 +136,10 @@ builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col
     for ___pair_a ( "${___pair_order[@]}" ) {
       ___pair_b="${___final_pairs[$___pair_a]}"
       ___text="${___input[___pair_b,___pair_a]}"
-      if [[ $___text = [[:space:]]#\{[[:space:]]#[\"\']${___key}[\"\']* ]]; then
-        ___found="$___text"
+      # JSON objects are unordered, so the wanted object is the smallest one
+      # that declares the key, not merely one that opens with it.
+      if [[ $___text = [[:space:]]#\{*[\"\']${___key}[\"\'][[:space:]]#:* ]]; then
+        [[ -z $___found || $#___text -lt $#___found ]] && ___found="$___text"
       fi
     }
   }
@@ -113,6 +153,62 @@ builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col
   }
 }
 # ]]]
+# FUNCTION: .zi-read-package-manifest [[[
+# Resolves one profile out of a package manifest. Kept separate from
+# .zi-get-package so the lookup can be exercised without the network, the
+# filesystem and the install path around it.
+#
+# $1 - the manifest text
+# $2 - the wanted profile name
+# $3 - name of a hash to fill with the `plugin-info' fields
+# $4 - name of an array to fill with the available profile names
+# $5 - name of a hash to fill with the selected profile's ices
+#
+# Returns 0 when the profile was found, 1 otherwise. $3 and $4 are filled either
+# way, so a caller can report what the manifest does offer.
+.zi-read-package-manifest() {
+  builtin emulate -LR zsh ${=${options[xtrace]:#off}:+-o xtrace}
+  builtin setopt extended_glob warn_create_global typeset_silent
+
+  local ___json=$1 ___profile=$2
+  # Not named ___Strings: .zi-parse-json declares a local by that name, which
+  # would shadow the hash passed to it by name and swallow the result.
+  local -A ___parsed
+  .zi-parse-json "$___json" "plugin-info" ___parsed
+
+  # The members keep the order the manifest wrote them in, so look both slots up
+  # by name instead of assuming the canonical order.
+  local -a ___level1
+  ___level1=( "${(@Q)${(@z)___parsed[1/1]}}" )
+  integer ___info_pos=${___level1[(I)plugin-info]} ___ices_pos=${___level1[(I)zi-ices]}
+  integer ___info_slot=$(( (___info_pos + 1) / 2 )) ___ices_slot=$(( (___ices_pos + 1) / 2 ))
+
+  local -a ___info ___profiles
+  (( ___info_pos )) && ___info=( "${(@Q)${(@z)___parsed[2/$___info_slot]}}" )
+  # set -A, not a ${(PAA)} round trip: ice values contain spaces, and joining
+  # the array into one string to re-split it would tear them apart.
+  builtin set -A "$3" "${___info[@]}"
+
+  local ___ices_members=${___parsed[2/$___ices_slot]}
+  ___profiles=( "${(@Q)${(@z)___ices_members}}" )
+  builtin set -A "$4" "${___profiles[@]:#$'\0'--object--$'\0'}"
+
+  integer ___pos=${___profiles[(I)$___profile]}
+  (( ___pos && ___ices_pos )) || return 1
+
+  # Objects are numbered per level across the whole subtree, so anything nested
+  # in a member written before `zi-ices' shifts the profile bodies. Each such
+  # object leaves an --object-- marker in its own member's string, so counting
+  # those markers gives the exact offset.
+  integer ___preceding=0 ___k
+  for (( ___k = 1; ___k < ___ices_slot; ___k ++ )) {
+    local -a ___members=( "${(@Q)${(@z)___parsed[2/$___k]}}" )
+    ___preceding+=${#${(M)___members[@]:#$'\0'--object--$'\0'}}
+  }
+
+  builtin set -A "$5" "${(@Q)${(@z)___parsed[3/$(( ___preceding + (___pos + 1) / 2 ))]}}"
+  return 0
+} # ]]]
 # FUNCTION: .zi-get-package [[[
 .zi-get-package() {
   builtin emulate -LR zsh ${=${options[xtrace]:#off}:+-o xtrace}
@@ -143,23 +239,17 @@ builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col
     return 1
   }
 
-  local -A Strings
-  .zi-parse-json "$pkgjson" "plugin-info" Strings
-  local -A jsondata1
-  jsondata1=( ${(@Q)${(@z)Strings[2/1]}} )
-  local user=${jsondata1[user]} plugin=${jsondata1[plugin]} url=${jsondata1[url]} message=${jsondata1[message]} required=${jsondata1[required]:-${jsondata1[requires]}}
+  local -A jsondata1 profile_ices
   local -a profiles
   local key value
-  integer pos
-  profiles=( ${(@Q)${(@z)Strings[2/2]}} )
-  profiles=( ${profiles[@]:#$'\0'--object--$'\0'} )
-  pos=${${(@Q)${(@z)Strings[2/2]}}[(I)$profile]}
-  if (( pos )) {
-    for key value ( "${(@Q)${(@z)Strings[3/$(( (pos + 1) / 2 ))]}}" ) {
+  integer found=0
+  .zi-read-package-manifest "$pkgjson" "$profile" jsondata1 profiles profile_ices && found=1
+  local user=${jsondata1[user]} plugin=${jsondata1[plugin]} url=${jsondata1[url]} message=${jsondata1[message]} required=${jsondata1[required]:-${jsondata1[requires]}}
+  if (( found )) {
+    for key value ( "${(@kv)profile_ices[@]}" ) {
       (( ${+ICE[$key]} )) && [[ ${ICE[$key]} != +* ]] && continue
       ICE[$key]=$value${ICE[$key]#+}
     }
-    ICE=( "${(kv)ICE[@]//\\\"/\"}" )
     [[ ${ICE[as]} = program ]] && ICE[as]="command"
     [[ -n ${ICE[on-update-of]} ]] && ICE[subscribe]="${ICE[subscribe]:-${ICE[on-update-of]}}"
     [[ -n ${ICE[pick]} ]] && ICE[pick]="${ICE[pick]//\$ZPFX/${ZPFX%/}}"
@@ -170,7 +260,6 @@ builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col
       eval "ICE[id-as]=\"${ICE[id-as]//(#m)[\"\\]/${map[$MATCH]}}\""
     }
   } else {
-    # Assumption: the default profile is the first in the table (see another color).
     +zi-message "{u-warn}Error{b-warn}:{error} the profile {apo}\`{hi}$profile{apo}\` {error}couldn't be found, aborting. Available profiles are: {lhi}${(pj:$epro_sep:)profiles[@]}{error}.{rst}"
     return 1
   }
@@ -236,8 +325,8 @@ builtin source "${ZI[BIN_DIR]}/lib/zsh/side.zsh" || { builtin print -P "${ZI[col
 
   if (( !${+ICE[git]} && !${+ICE[from]} )) {
     (
+      local -A Strings jsondata
       .zi-parse-json "$pkgjson" "_from" Strings
-      local -A jsondata
       jsondata=( "${(@Q)${(@z)Strings[1/1]}}" )
 
       local URL=${jsondata[_resolved]}

tests/package-manifest-parsing.zsh

@@ -0,0 +1,132 @@
+#!/usr/bin/env zsh
+# -*- mode: zsh; sh-indentation: 2; indent-tabs-mode: nil; sh-basic-offset: 2; -*-
+# vim: ft=zsh sw=2 ts=2 et
+
+builtin emulate -R zsh
+setopt pipe_fail
+
+fail() {
+  builtin print -u2 -r -- "not ok - $1"
+  exit 1
+}
+
+typeset project_root="${ZI_TEST_CHECKOUT:-${0:A:h:h}}"
+typeset temp_root
+temp_root="$(command mktemp -d "${TMPDIR:-/tmp}/zi-package-manifest-test.XXXXXXXX")" ||
+  fail "create temporary directory"
+trap 'command rm -rf -- "$temp_root"' EXIT INT TERM
+
+command mkdir -p \
+  "${temp_root}/home" \
+  "${temp_root}/cache" \
+  "${temp_root}/config" \
+  "${temp_root}/data" \
+  "${temp_root}/zdotdir" || fail "create isolated environment"
+
+# `.zi-parse-json' reads package manifests, whose ice values are executed later.
+# JSON objects are unordered and JSON strings carry escapes, so neither the key
+# order nor a backslash may change which ices a profile resolves to.
+env \
+  HOME="${temp_root}/home" \
+  XDG_CACHE_HOME="${temp_root}/cache" \
+  XDG_CONFIG_HOME="${temp_root}/config" \
+  XDG_DATA_HOME="${temp_root}/data" \
+  ZDOTDIR="${temp_root}/zdotdir" \
+  ZI_TEST_CHECKOUT="$project_root" \
+  zsh -f <<'ZSH' || fail ".zi-parse-json does not read manifests independently of key order and escapes"
+builtin emulate -R zsh
+setopt pipe_fail
+
+builtin source "${ZI_TEST_CHECKOUT}/zi.zsh" || return 1
+builtin source "${ZI_TEST_CHECKOUT}/lib/zsh/install.zsh" || return 1
+
+# Exercise the shipped lookup, not a copy of it: .zi-read-package-manifest is
+# exactly what .zi-get-package uses to turn a manifest into a profile's ices.
+resolve() {  # resolve <manifest text> <output hash name> [profile]
+  local -A info
+  local -a names
+  .zi-read-package-manifest "$1" "${3:-default}" info names "$2" || {
+    builtin print -u2 -r -- "profile '${3:-default}' not resolved; available: ${names[*]}"
+    return 1
+  }
+}
+
+local canonical='{"zsh-data":{"plugin-info":{"user":"u","plugin":"p"},
+  "zi-ices":{"default":{"as":"program","pick":"x"},"bgn":{"as":"null"}}}}'
+# The same document with the two `zsh-data' members written the other way round.
+local reordered='{"zsh-data":{"zi-ices":{"default":{"as":"program","pick":"x"},
+  "bgn":{"as":"null"}},"plugin-info":{"user":"u","plugin":"p"}}}'
+
+local -A a b
+resolve "$canonical" a || return 1
+[[ ${a[as]} == program && ${a[pick]} == x ]] || {
+  builtin print -u2 -r -- "canonical order resolved as='${a[as]}' pick='${a[pick]}'"
+  return 1
+}
+resolve "$reordered" b || { builtin print -u2 -r -- "reordered manifest failed to parse"; return 1; }
+[[ ${b[as]} == ${a[as]} && ${b[pick]} == ${a[pick]} ]] || {
+  builtin print -u2 -r -- "key order changed the result: as='${b[as]}' pick='${b[pick]}'"
+  return 1
+}
+
+# Escapes. Build every backslash at run time so no quoting layer can eat one.
+local bs=$'\134' nl=$'\n' tab=$'\t'
+local escaped='{"zsh-data":{"plugin-info":{"user":"u","plugin":"p"},"zi-ices":{"default":{
+  "atclone":"a'$bs$bs'b",
+  "atpull":"say '$bs'"hi'$bs'"",
+  "mv":"p'$bs'tq",
+  "sbin":"l1'$bs'nl2",
+  "pick":"a'$bs'/b",
+  "src":"x'$bs'u0041y",
+  "as":"a'$bs$bs$bs'"b",
+  "ver":"a'$bs'qb"}}}}'
+
+local -A e
+resolve "$escaped" e || return 1
+check() {  # check <key> <expected>
+  [[ ${e[$1]} == $2 ]] || {
+    builtin print -u2 -r -- "escape ${1}: got '${e[$1]}', expected '$2'"
+    return 1
+  }
+}
+check atclone "a${bs}b"   || return 1
+check atpull 'say "hi"'   || return 1
+check mv     "p${tab}q"   || return 1
+check sbin   "l1${nl}l2"  || return 1
+check pick   'a/b'        || return 1
+check src    'xAy'        || return 1
+# JSON "a\\\"b" denotes the literal a\"b, not a"b.
+check as     "a${bs}\"b"  || return 1
+# An unknown escape is not JSON; leave it exactly as written.
+check ver    "a${bs}qb"   || return 1
+
+local -A n c l
+
+# Profile bodies are numbered across the whole subtree, so an object nested in an
+# earlier member must not shift which profile is resolved. Ice values are
+# executed, so picking the wrong one runs the wrong commands.
+resolve '{"zsh-data":{"plugin-info":{"user":"u","plugin":"p","bugs":{"url":"decoy"}},
+  "zi-ices":{"default":{"as":"program","pick":"x"},"bgn":{"as":"null"}}}}' n || return 1
+[[ ${n[as]} == program && ${n[pick]} == x ]] || {
+  builtin print -u2 -r -- "nested member shifted the profile: as='${n[as]}' pick='${n[pick]}' url='${n[url]}'"
+  return 1
+}
+
+# A profile name reused as a key elsewhere must not divert the lookup.
+resolve '{"zsh-data":{"plugin-info":{"user":"u","default":"decoy"},
+  "zi-ices":{"default":{"as":"program","pick":"x"},"bgn":{"as":"null"}}}}' c || return 1
+[[ ${c[as]} == program && ${c[pick]} == x ]] || {
+  builtin print -u2 -r -- "name collision diverted the lookup: as='${c[as]}' pick='${c[pick]}'"
+  return 1
+}
+
+# A later profile, so the offset is exercised with a non-first slot.
+resolve '{"zsh-data":{"plugin-info":{"user":"u","bugs":{"url":"decoy"}},
+  "zi-ices":{"first":{"as":"null"},"default":{"as":"program","pick":"late"}}}}' l || return 1
+[[ ${l[pick]} == late ]] || {
+  builtin print -u2 -r -- "later profile resolved wrong: pick='${l[pick]}'"
+  return 1
+}
+ZSH
+
+builtin print -r -- "ok - .zi-parse-json reads manifests independently of key order and decodes JSON escapes"