Fix create_archive: operator precedence, malformed zip bytea literals, repo scoping

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/diagnose_test.sql \
        test/sql/revert_test.sql \
        test/sql/reset_test.sql \

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;

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;

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;

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/diagnose_test.sql
 test/sql/revert_test.sql
 test/sql/reset_test.sql