Bugs identified in archive.pl1

[Jim Lippard]

The archive command has several bugs that destroy user data or leave the user with unintended access, and several that print misleading messages or leak storage. Numbers 1-19 below are part of a single numbering for a total of 36 identified bugs across the archive programs (archive.pl1, archive_util_.pl1, archive_.pl1, archive_aux_.pl1, and update_handler_.pl1). Line numbers are from MR12.8 and indicate the start of the problem area. Item 6 below is not a bug but documented behavior missing from archive.info; other errors in archive.info are listed below but are in addition to the numbered bugs. I have a tests.archive test kit which has an exec_com for each item marked "Test" which prints the expected behavior and then runs commands. In the examples below, t_ok.archive holds two components, alpha and beta, and seed.txt is any small text segment.

Bug 1: an archive that exists but cannot be read is treated as missing, line 460

For the replace and append keys (r, rd, a, ca, ...), any failure of initiate_file_ on the archive, not only "entry not found", is taken to mean that the archive does not exist. The command then tries to create a new archive, gets "name duplication", and calls nd_handler_, which asks whether to delete the existing archive. Answering yes replaces the whole archive with one holding only the new components.

sa t_ok.archive w
ac r t_ok beta
archive: Name duplication. Do you want to delete the old segment ...>t_ok.archive?

Expected: the error from initiate_file_ (here "No read permission on entry") is reported, and no query is asked. Test: bug01.ec.

Bug 2: updating a protected archive deletes the user's own ACL entry instead of restoring it, lines 672 and 717

When the user lacks w on the archive, the command asks whether to update it, gives the user's process group ID (e.g. Person.Project.a) rw, and afterwards deletes that ACL entry. If an entry for that exact name already existed, it is overwritten and then deleted.

sa t_ok.archive r
sa t_ok.archive r Person.Project.a
answer yes ac r t_ok beta
list_acl t_ok.archive (the Person.Project.a entry is gone)

Expected: the ACL is the same after the update as before it. Test: bug02.ec.

Bug 3: forced access is left on a protected archive after a failure or an interruption, lines 702 and 792

The ACL entry added in bug 2 is removed only on the successful path. After a record quota overflow, or if the command is interrupted and the user releases, the user is left with rw on the archive:

list_acl DIR>t_rqo3.archive
rw Person.Project.a
r Person.Project.*

Expected: the original access is restored on every path. Test: bug03.ec (needs an empty directory with a 60-record quota).

Bug 4: after a record quota overflow, the saved copy of the updated archive is misreported and then destroyed, line 705

When an update fails for lack of quota, the updated archive, built in the process directory as archive_temp_.archive, is renamed to the archive's name and the user is told where to find it. If that name is already taken in the process directory (e.g. by a copy saved by an earlier failure), the rename fails, the message names [pd]>temp.archive, which does not exist, and the segment remains the command's temporary. The next update in the process truncates it to 0 records, destroying the only updated copy; its bit count is left unchanged, so it then lists as a format error.

ac r DIR>t_rqo c2
archive: Record quota overflow. Archive DIR>t_rqo.archive not updated.
A copy of the updated archive can be found in [pd]>t_rqo.archive
ac r DIR>t_rqo c3
archive: Record quota overflow. Archive DIR>t_rqo.archive not updated.
A copy of the updated archive can be found in [pd]>temp.archive
ac r t_ok beta
status [pd]>archive_temp_.archive (records used: 0)
ac t [pd]>archive_temp_
archive: Format error in [pd]>archive_temp_.archive

Expected: the message names the segment that holds the copy, and the copy survives later archive commands. Tests: bug04.ec, bug04d.ec (need an empty directory with a 60-record quota).

Bug 5: extracting with force over a link deletes the link's target instead of the link, line 855

When the destination entry exists and is a link, x and xd ask (through nd_handler_) whether to unlink it, while a similar copy -force's behavior unlinks it with the message "Name duplication. Unlinking the old link ... (Target ... exists.)"; in each case the target is not touched. xf and xdf instead call the command's own delete_seg, which chases the link and deletes its target (which may be anywhere in the hierarchy), then create a new segment through the link in the target's directory. No segment is created in the working directory, the target loses its other names and its ACL, and no message is printed.

add_name keep>alpha keepname
link keep>alpha alpha
ac xf t_ok alpha
status alpha (still a link)
status keep>alpha -names (keepname is gone)

Expected: as the MPM says, an existing entry is handled by the copy command's duplicate name convention: the link is unlinked, as by copy -force, and the component is extracted in the working directory. Test: bug05.ec (also runs copy -force and ac x over a link for comparison).

6: Not a bug: deleting a linked source after rd, ud or ad deletes the link's target, which archive.info does not say

When a source named on the command line is a link, the deletion requested by rd, ud or ad deletes the segment the link points to, not the link. This is documented in the MPM (Commands, archive, Notes: "For segment deletions after replacement requests, if the specified component name was a link to a segment, the segment linked to is deleted. The link is not unlinked."), but archive.info says nothing about it, and the behavior is unexpected: a segment elsewhere in the hierarchy, possibly with other names, is deleted, and the link is left pointing to nothing.

Expected: archive.info includes the MPM's statement.

Bug 7: component names longer than 32 characters are truncated for table and delete, line 535

For t, tl, tb, tlb, d and cd, a component name argument is assigned to a 32-character variable without a length check, so a longer name matches the component named with its first 32 characters. ac d deletes a component that was not named:

ac d t_longname abcdefghijklmnopqrstuvwxyz0123456789

(deletes abcdefghijklmnopqrstuvwxyz012345)

Expected: "Entry name too long", as the other keys already report through expand_pathname_, and nothing is deleted. Test: bug07.ec.

Bug 8: a global extract-and-delete can remove a component that was not extracted, line 835

In xd with no component names, if creating the segment for a component fails (e.g. a directory of that name exists), whether the component is kept in the archive depends on a stray word of storage (see bug 9). In practice it is deleted from the archive without being extracted:

create_dir beta
ac xd t_ok
archive: This operation is not allowed for a directory. ...>beta
archive: All components of ...>t_ok.archive have been deleted.

Expected: a component that was not extracted stays in the archive. Test: bug08.ec.

Bug 9: a failed extraction with no component names writes outside the argument array, line 835

In the same case as bug 8, and for x with no names when a segment cannot be created (e.g. no append permission), the command reads and writes element 0 of the argument array (nonglobal (i) with i = 0), which does not exist, overwriting a word of the stack frame before the array. Found by inspection; exercised by bug08.ec.

Bug 10: a global delete-type update of more than 102 components creates, and leaks, a segment for each further component, line 990

The list of source segments to delete is kept on the stack; when it fills, it is moved to a segment in the process directory. The test for "full" does not check whether the list has already been moved, so for every component after the 102nd a new segment is created and the list copied into it; only the last is deleted. A global rd of a 110-component archive leaves 7 unnamed segments in [pd]. Test: bug10.ec.

Bug 11: the global update messages name the archive's directory instead of the working directory, line 778

The source segments for a global update come from the working directory, but the messages name the directory containing the archive:

cwd work
ac u <t_ok
archive: No matching segments in ...>bug11; no components were updated in archive ...>bug11>t_ok.archive
archive: Archive ...>t_ok.archive contains the latest versions; no components were updated from ...>bug11.

Expected: both name the working directory (...>bug11>work). Test: bug11.ec.

Bug 12: with the copy keys, messages name the original archive, which is unchanged, lines 760 and 769

For cr and its variants, the message for a component added to the archive names the original archive rather than the new copy in the working directory. The "Archive segment overflow" message has the same error.

cwd work
ac cr <t_ok gamma
archive: ...>work>gamma appended to ...>bug12>t_ok.archive

Expected: "appended to ...>bug12>work>t_ok.archive", the archive that was changed. Test: bug12.ec.

Bug 13: an archive overflow while copying leaves an incomplete new archive, line 1087

If a copy key (cr, ca, cd, ...) runs out of room while copying components into the new archive in the working directory, the command stops and leaves the partial archive, with a bit count of 0. Expected: no partial archive is left. Found by inspection (needs an archive near the maximum segment length).

Bug 14: segments are left initiated on error paths, lines 851, 975, 1021 and 1030

When a source's bit count is inconsistent with its length, and when adding a source would overflow the archive, the source segment is not terminated. When xf replaces an existing destination entry, the segment returned by the first hcs_$make_seg is not terminated. Found by inspection.

Bug 15: a global replace sets a flag through a stale index, line 935

If the archive itself matches a component name in a global replace, the command sets flags (i) using the index of the last named component, which in the global case is stale and points into the deletion list. Found by inspection.

Bug 16: an error from fs_util_$get_max_length is ignored when a new archive is created, line 1120

The code tests code instead of error after the call. Found by inspection.

Bug 17: an unexpected error from archive_util_$first_element is ignored, line 473

If the archive's status cannot be obtained, first_element returns a system error code, which the command treats as success. Found by inspection.

Bug 18: the archive is not terminated when the command stops on an early error, line 467

When the archive's mode or maximum length cannot be obtained, the command returns directly (goto RETURN) instead of through its cleanup. Found by inspection.

Bug 19: word counts are computed with 17-bit precision, lines 816, 1008 and 1078

divide (..., 17, 0) is used for component word counts, which can exceed 131,071 for large components. No failure was seen on MR12.8, because the values are held in full words. Found by inspection.

Errors in archive.info

paths is described only for table and delete operations; it is also the source or destination segment for append, replace, update and extract.
tlb is said to suppress "the time portion of dates"; it suppresses the title and column headings, as the MPM says.
ca: "appending components to a copy of the new archive segment" should be "a copy of the archive segment"; cud: "copies, update, and deletes" should be "copies, updates, and deletes".
x, xf: the MPM's statement that an existing entry is handled by the copy command's duplicate name convention is missing.
Notes: the MPM's statement about linked sources (item 6) is missing, as is any mention that the copy operations cannot be used on an archive in the working directory, or of what happens after a record quota overflow.