Skip to content

fix: validate --max-hamt-fanout CLI flag per UnixFS spec#11230

Open
lidel wants to merge 1 commit intomasterfrom
fix/validate-hamt-fanout-cli
Open

fix: validate --max-hamt-fanout CLI flag per UnixFS spec#11230
lidel wants to merge 1 commit intomasterfrom
fix/validate-hamt-fanout-cli

Conversation

@lidel
Copy link
Member

@lidel lidel commented Mar 10, 2026

the CLI flag bypassed the config validation in ValidateImportConfig, allowing spec-noncompliant values (e.g. 3 or 999999) to be silently accepted. validation now happens in the options layer, covering both CLI and programmatic API usage.

the CLI flag bypassed the config validation in ValidateImportConfig,
allowing spec-noncompliant values (e.g. 3 or 999999) to be silently
accepted. validation now happens in the options layer, covering both
CLI and programmatic API usage.
@lidel lidel added the skip/changelog This change does NOT require a changelog entry label Mar 10, 2026
@lidel lidel marked this pull request as ready for review March 10, 2026 17:34
@lidel lidel requested a review from a team as a code owner March 10, 2026 17:34
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the phrasing multiple of 8 AND power of 2 sounds weird. (all powers of 2 beyond 8 are multiples of 8).

IMO "power of 2, between 8 and 1024" is clearer for users.

cmds.IntOption(maxFileLinksOptionName, "Limit the maximum number of links in UnixFS file nodes to this value. WARNING: experimental. Default: Import.UnixFSFileMaxLinks"),
cmds.IntOption(maxDirectoryLinksOptionName, "Limit the maximum number of links in UnixFS basic directory nodes to this value. WARNING: experimental, Import.UnixFSHAMTDirectorySizeThreshold is safer. Default: Import.UnixFSDirectoryMaxLinks"),
cmds.IntOption(maxHAMTFanoutOptionName, "Limit the maximum number of links of a UnixFS HAMT directory node to this (power of 2, multiple of 8). WARNING: experimental, Import.UnixFSHAMTDirectorySizeThreshold is safer. Default: Import.UnixFSHAMTDirectoryMaxFanout"),
cmds.IntOption(maxHAMTFanoutOptionName, "Limit the maximum number of links of a UnixFS HAMT directory node to this (power of 2, multiple of 8, at most 1024). WARNING: experimental, Import.UnixFSHAMTDirectorySizeThreshold is safer. Default: Import.UnixFSHAMTDirectoryMaxFanout"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cmds.IntOption(maxHAMTFanoutOptionName, "Limit the maximum number of links of a UnixFS HAMT directory node to this (power of 2, multiple of 8, at most 1024). WARNING: experimental, Import.UnixFSHAMTDirectorySizeThreshold is safer. Default: Import.UnixFSHAMTDirectoryMaxFanout"),
cmds.IntOption(maxHAMTFanoutOptionName, "Limit the maximum number of links of a UnixFS HAMT directory node to this (power of 2, between 8 and 1024). WARNING: experimental, Import.UnixFSHAMTDirectorySizeThreshold is safer. Default: Import.UnixFSHAMTDirectoryMaxFanout"),

Comment on lines +236 to +237
// Per the UnixFS spec, the value must be a power of 2, a multiple of 8
// (for byte-aligned bitfields), and at most 1024.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Per the UnixFS spec, the value must be a power of 2, a multiple of 8
// (for byte-aligned bitfields), and at most 1024.
// Per the UnixFS spec, the value must be a power of 2, minimum 8
// (for byte-aligned bitfields), and maximum 1024.

func (unixfsOpts) MaxHAMTFanout(n int) UnixfsAddOption {
return func(settings *UnixfsAddSettings) error {
if n < 8 || n&(n-1) != 0 || n > 1024 {
return fmt.Errorf("HAMT fanout must be a power of 2, a multiple of 8, and at most 1024 (got %d)", n)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("HAMT fanout must be a power of 2, a multiple of 8, and at most 1024 (got %d)", n)
return fmt.Errorf("HAMT fanout must be a power of 2, between 8 and 1024 (got %d)", n)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip/changelog This change does NOT require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants