From f34eac8ebbd230d975fc940d1b0fe185fb58b7fd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 15:43:59 +0000 Subject: [PATCH] Fix create_archive: operator precedence, zip bytea literals, repo scoping create_archive errored on every call: - tar path: `tf.path || '/' || e->>'name'` parsed as `(tf.path || '/' || e) ->> 'name'` because || binds tighter than ->>, raising "operator does not exist: text ->> unknown". Parenthesized the accessor. - zip path: the header/footer bytea literals '\x50\x4B\x03\x04' repeated the \x prefix, which the bytea hex input format rejects as "invalid hexadecimal digit". Use single-prefix literals ('\x504B0304' / '\x504B0506'). Also scope the tree-ish resolution, tree walk and blob join to the target repository (they previously matched objects across all repositories), and qualify the archived columns via their table aliases. Add test/sql/archive_test.sql covering tar output (non-empty, embeds file paths, explicit-commit form) and the zip PK signature. Wired into the core suite. --- makefile | 1 + sql/pg_git--0.4.0.sql | 40 ++++++++++++++------------- sql/pgit-archive.sql | 40 ++++++++++++++------------- test/sql/archive_test.sql | 57 +++++++++++++++++++++++++++++++++++++++ test/sql/manifest.txt | 1 + 5 files changed, 103 insertions(+), 36 deletions(-) create mode 100644 test/sql/archive_test.sql diff --git a/makefile b/makefile index f53b3a3..2d5a579 100644 --- a/makefile +++ b/makefile @@ -62,6 +62,7 @@ CORE_TESTS := \ test/sql/merge_conflicts_test.sql \ test/sql/remote_test.sql \ test/sql/advanced_test.sql \ + test/sql/archive_test.sql \ test/sql/reset_test.sql \ test/sql/search_path_qualification_test.sql \ test/sql/gc_test.sql \ diff --git a/sql/pg_git--0.4.0.sql b/sql/pg_git--0.4.0.sql index 9f73d9a..48b4f5e 100644 --- a/sql/pg_git--0.4.0.sql +++ b/sql/pg_git--0.4.0.sql @@ -1685,16 +1685,16 @@ DECLARE v_header BYTEA; v_footer BYTEA; BEGIN - -- Resolve tree-ish to tree hash + -- Resolve tree-ish to tree hash (scoped to this repository). IF p_tree_ish = 'HEAD' THEN SELECT tree_hash INTO v_tree_hash FROM commits c - JOIN refs r ON c.hash = r.commit_hash - WHERE r.name = 'HEAD'; + JOIN refs r ON c.repo_id = r.repo_id AND c.hash = r.commit_hash + WHERE r.repo_id = p_repo_id AND r.name = 'HEAD'; ELSE SELECT tree_hash INTO v_tree_hash FROM commits - WHERE hash = p_tree_ish; + WHERE repo_id = p_repo_id AND hash = p_tree_ish; END IF; -- Initialize archive based on format @@ -1703,8 +1703,10 @@ BEGIN v_header := '\x00'::BYTEA; -- tar header v_footer := '\x00'::BYTEA; -- tar footer WHEN 'zip' THEN - v_header := '\x50\x4B\x03\x04'::BYTEA; -- ZIP header - v_footer := '\x50\x4B\x05\x06'::BYTEA; -- ZIP footer + -- bytea hex format is a single \x prefix followed by all hex digits; + -- '\x50\x4B...' repeats the prefix and is rejected as invalid hex. + v_header := '\x504B0304'::BYTEA; -- ZIP header + v_footer := '\x504B0506'::BYTEA; -- ZIP footer END CASE; -- Build archive content @@ -1714,36 +1716,38 @@ BEGIN e->>'mode' as mode FROM trees, jsonb_array_elements(entries) e - WHERE hash = v_tree_hash - + WHERE trees.repo_id = p_repo_id AND trees.hash = v_tree_hash + UNION ALL - - SELECT tf.path || '/' || e->>'name', + + -- Parenthesize (e->>'name'): || binds tighter than ->>, so without the + -- parens this parses as (tf.path || '/' || e) ->> 'name' and fails. + SELECT tf.path || '/' || (e->>'name'), e->>'hash', e->>'mode' FROM tree_files tf - JOIN trees t ON tf.hash = t.hash, + JOIN trees t ON t.repo_id = p_repo_id AND tf.hash = t.hash, jsonb_array_elements(t.entries) e WHERE e->>'type' = 'tree' ) - SELECT v_header || + SELECT v_header || string_agg( CASE p_format WHEN 'tar' THEN -- tar file header (simplified) - convert_to(rpad(path, 100, '\0'), 'UTF8') || - convert_to(rpad(mode, 8, '\0'), 'UTF8') || - content + convert_to(rpad(tf.path, 100, '\0'), 'UTF8') || + convert_to(rpad(tf.mode, 8, '\0'), 'UTF8') || + b.content WHEN 'zip' THEN -- zip file header (simplified) - convert_to(path || '\n', 'UTF8') || - content + convert_to(tf.path || '\n', 'UTF8') || + b.content END, ''::BYTEA ) || v_footer INTO v_archive FROM tree_files tf - JOIN blobs b ON tf.hash = b.hash; + JOIN blobs b ON b.repo_id = p_repo_id AND tf.hash = b.hash; RETURN v_archive; END; diff --git a/sql/pgit-archive.sql b/sql/pgit-archive.sql index cbfd0b9..b4541c3 100644 --- a/sql/pgit-archive.sql +++ b/sql/pgit-archive.sql @@ -12,16 +12,16 @@ DECLARE v_header BYTEA; v_footer BYTEA; BEGIN - -- Resolve tree-ish to tree hash + -- Resolve tree-ish to tree hash (scoped to this repository). IF p_tree_ish = 'HEAD' THEN SELECT tree_hash INTO v_tree_hash FROM commits c - JOIN refs r ON c.hash = r.commit_hash - WHERE r.name = 'HEAD'; + JOIN refs r ON c.repo_id = r.repo_id AND c.hash = r.commit_hash + WHERE r.repo_id = p_repo_id AND r.name = 'HEAD'; ELSE SELECT tree_hash INTO v_tree_hash FROM commits - WHERE hash = p_tree_ish; + WHERE repo_id = p_repo_id AND hash = p_tree_ish; END IF; -- Initialize archive based on format @@ -30,8 +30,10 @@ BEGIN v_header := '\x00'::BYTEA; -- tar header v_footer := '\x00'::BYTEA; -- tar footer WHEN 'zip' THEN - v_header := '\x50\x4B\x03\x04'::BYTEA; -- ZIP header - v_footer := '\x50\x4B\x05\x06'::BYTEA; -- ZIP footer + -- bytea hex format is a single \x prefix followed by all hex digits; + -- '\x50\x4B...' repeats the prefix and is rejected as invalid hex. + v_header := '\x504B0304'::BYTEA; -- ZIP header + v_footer := '\x504B0506'::BYTEA; -- ZIP footer END CASE; -- Build archive content @@ -41,36 +43,38 @@ BEGIN e->>'mode' as mode FROM trees, jsonb_array_elements(entries) e - WHERE hash = v_tree_hash - + WHERE trees.repo_id = p_repo_id AND trees.hash = v_tree_hash + UNION ALL - - SELECT tf.path || '/' || e->>'name', + + -- Parenthesize (e->>'name'): || binds tighter than ->>, so without the + -- parens this parses as (tf.path || '/' || e) ->> 'name' and fails. + SELECT tf.path || '/' || (e->>'name'), e->>'hash', e->>'mode' FROM tree_files tf - JOIN trees t ON tf.hash = t.hash, + JOIN trees t ON t.repo_id = p_repo_id AND tf.hash = t.hash, jsonb_array_elements(t.entries) e WHERE e->>'type' = 'tree' ) - SELECT v_header || + SELECT v_header || string_agg( CASE p_format WHEN 'tar' THEN -- tar file header (simplified) - convert_to(rpad(path, 100, '\0'), 'UTF8') || - convert_to(rpad(mode, 8, '\0'), 'UTF8') || - content + convert_to(rpad(tf.path, 100, '\0'), 'UTF8') || + convert_to(rpad(tf.mode, 8, '\0'), 'UTF8') || + b.content WHEN 'zip' THEN -- zip file header (simplified) - convert_to(path || '\n', 'UTF8') || - content + convert_to(tf.path || '\n', 'UTF8') || + b.content END, ''::BYTEA ) || v_footer INTO v_archive FROM tree_files tf - JOIN blobs b ON tf.hash = b.hash; + JOIN blobs b ON b.repo_id = p_repo_id AND tf.hash = b.hash; RETURN v_archive; END; diff --git a/test/sql/archive_test.sql b/test/sql/archive_test.sql new file mode 100644 index 0000000..934489b --- /dev/null +++ b/test/sql/archive_test.sql @@ -0,0 +1,57 @@ +-- Path: /test/sql/archive_test.sql +-- pg_git archive tests +-- Regression: create_archive errored on every call. The tar path hit an +-- operator-precedence bug (tf.path || '/' || e->>'name' parsed as +-- (... || e) ->> 'name'); the zip path used malformed bytea hex literals +-- ('\x50\x4B...' repeats the \x prefix). + +BEGIN; + +SELECT plan(5); + +SELECT pggit.init_repository('archive_repo', '/archive/path') AS repo_id \gset +SELECT set_config('vars.repo_id', :'repo_id', false); + +SELECT pggit.stage_file((current_setting('vars.repo_id')::int), 'a.txt', 'hello'::bytea); +SELECT pggit.stage_file((current_setting('vars.repo_id')::int), 'b.txt', 'world'::bytea); +SELECT pggit.commit_index((current_setting('vars.repo_id')::int), 'tester', 'c1') AS c1 \gset +SELECT set_config('vars.c1', :'c1', false); + +-- tar archive of HEAD is produced and non-empty. +SELECT cmp_ok( + octet_length(pggit.create_archive((current_setting('vars.repo_id')::int))), + '>', 0, + 'create_archive (tar, HEAD) produces a non-empty archive' +); + +-- The tar stream embeds the file paths. +SELECT ok( + position('a.txt' in encode( + pggit.create_archive((current_setting('vars.repo_id')::int)), 'escape')) > 0, + 'tar archive contains the first file path' +); +SELECT ok( + position('b.txt' in encode( + pggit.create_archive((current_setting('vars.repo_id')::int)), 'escape')) > 0, + 'tar archive contains the second file path' +); + +-- An explicit commit argument works too. +SELECT cmp_ok( + octet_length(pggit.create_archive( + (current_setting('vars.repo_id')::int), current_setting('vars.c1'), 'tar')), + '>', 0, + 'create_archive accepts an explicit commit' +); + +-- The zip archive begins with the PK\x03\x04 local-file-header magic. +SELECT is( + substring(pggit.create_archive( + (current_setting('vars.repo_id')::int), current_setting('vars.c1'), 'zip') + from 1 for 4), + '\x504b0304'::bytea, + 'zip archive starts with the PK signature' +); + +SELECT * FROM finish(); +ROLLBACK; diff --git a/test/sql/manifest.txt b/test/sql/manifest.txt index def781a..c99337d 100644 --- a/test/sql/manifest.txt +++ b/test/sql/manifest.txt @@ -10,6 +10,7 @@ test/sql/merge_test.sql test/sql/merge_conflicts_test.sql test/sql/remote_test.sql test/sql/advanced_test.sql +test/sql/archive_test.sql test/sql/reset_test.sql test/sql/search_path_qualification_test.sql test/sql/gc_test.sql