fix(inode): report symlink size as the length of its target - #4943

Description

SymlinkInode.Attributes never assigned its size return value, so it returned the zero value and stat reported a size of 0 for every symlink.

func (s *SymlinkInode) Attributes(
	ctx context.Context, clobberedCheck bool) (size uint64, mtime time.Time, nlink uint32, err error) {
	mtime = s.mtime
	nlink = 1
	return
}

stat(2) requires st_size of a symlink to be the length of the pathname it contains, without a terminating null byte. The intended semantics were already documented in this same file, on UpdateSize a few lines above:

// The size of a symlink is its target's length, not the backing object's size.

So the contract was written down; Attributes just did not implement it. For a legacy symlink the backing object is empty, which is why the reported size came out as 0 rather than as some other wrong number.

The fix is one line: size = uint64(len(s.target)).

Scope check: fs.getAttributes is the only caller of Attributes, and it assigns the value straight to attr.Size. The rename path reads Source() (the backing object's size) rather than Attributes, so it is unaffected. The change is limited to the size reported to the kernel.

Link to the issue in case of a bug fix.

#2273

I posted the root-cause analysis there first, per the dev guide, including the open question below. Happy to close this PR if you would rather handle it yourselves or scope it differently.

Testing details

1. Manual - NA. Verified through the composite tests below, which exercise a real mount rather than only the inode layer.
2. Unit tests - updated TestAttributes, which previously asserted uint64(0). Added TestAttributesSizeIsTargetLength, covering the legacy metadata representation with a zero-size backing object, a non-zero-size backing object (so the assertion cannot pass by coincidence), an empty target, and a multi-byte target. Added TestAttributesSizeForStandardSymlink for the content-based representation.
3. Integration tests - not run; I do not have a test bucket. The composite suite was run instead: ./internal/fs/... passes, including a new SymlinkTest.LinkSizeIsTargetLength that creates symlinks through the mount and checks Lstat reports the target length.

As a negative control I reverted the one-line fix and confirmed all three new tests fail with actual: 0x0, so they do test the change rather than pass incidentally.

Six packages fail in my container (benchmarks/internal/percentile, internal/block, internal/bufferedread, internal/cache/file, internal/cache/util, internal/gcsx). I checked these against a clean checkout and the failing set is identical, so they are environmental - the container runs as root, which defeats the permission-based cases - and unrelated to this change.

Any backward incompatible change? If so, please explain.

Yes, in observable behaviour, and it is worth an explicit decision rather than being waved through.

stat on a symlink now reports the target length instead of 0. Anything that reads st_size for a symlink sees a different number - though 0 was never a value a POSIX-conforming caller could use, and tools like tar, rsync and ls -l expect the target length.

Concretely, four existing assertions pinned the old value and are updated here:

• internal/fs/inode/symlink_test.go - TestAttributes
• internal/fs/local_modifications_test.go - SymlinkTest.CreateLink, twice, via Lstat and via the readdir entry
• internal/fs/caching_test.go - CachingWithImplicitDirsTest.SymlinksWork
• internal/fs/foreign_modifications_test.go - ForeignModsTest.Symlink

My reading is that these were written to match the implementation rather than to encode a deliberate decision, but that judgement is yours. The GCS object size assertion in CreateLink (ExpectEq(0, m.Size)) is left alone - that one is correctly 0, since a legacy symlink's target lives in metadata and the object really is empty.