Skip to content

--structure silently ignores a template and mirrors with the default layout - #1418

Merged
xroche merged 5 commits into
masterfrom
fix-1380-structure-template
Aug 25, 2026
Merged

--structure silently ignores a template and mirrors with the default layout#1418
xroche merged 5 commits into
masterfrom
fix-1380-structure-template

Conversation

@xroche

@xroche xroche commented Aug 24, 2026

Copy link
Copy Markdown
Owner

-N reads a glued value as a preset number and a detached one as a user template, and the alias table classed --structure as param, which always glues. --structure "%h%p/%n%q.%t" therefore became -N%h%p/%n%q.%t, which the engine never consumes: the token fell through to the URL list and the crawl ran with the default structure at exit 0. It is a new paramn class rather than a reuse of param1 because #1379 tried that, and it turned --structure=1 into a template literally named 1, collapsing the mirror onto one colliding filename, also at exit 0.

paramn glues what -N can actually read glued: on/off, and a digit run of at most nine digits whose remainder holds no path separator and no extension dot. Every preset and every cluster param used to glue still glues, so --structure=1L0 remains preset 1 plus -L0, while a template reaches the engine detached even when it opens with a digit. The short arm uses a stricter test, a bare digit run and nothing else, because -N 2col/%n.%t has to stay a template. Anything left over has to carry a %: a value without one maps every URL onto a single local name, so --structure=flat is a typo rather than a template and is now refused, along with OFF, none, default, yes and -1, which used to mirror the whole site as ./flat and ./flat-2. --user-structure still takes such a value verbatim.

The nine-digit bound exists because a longer run does not read portably. sscanf("%d") past INT_MAX is undefined and glibc answers differently per width: on LP64 the value truncates negative, which is "userdef" with an empty template and crashes url_savename, while on ILP32 scanf clamps to INT_MAX, a preset whose sub-layout flattens the whole mirror onto one file. optreal_find matches -N exactly, so the alias table never sees the glued spelling and the engine counts the digits itself, calling atoi only on a run short enough to fit. -N4294967295 and --structure=4294967295 both abort on master.

-N 1 is preset 1 now instead of a template named 1. That semantics change is deliberate and approved; a template named 1 has no practical use. --user-structure, the spelling that always worked, was documented nowhere and now reaches the help text, and from there the man page, plus guide.html and cmdguide.html.

Test 359 asserts the whole mirrored file set on disk for every spelling rather than just the wanted path, so a value that fell through to the URL list shows up as an extra file. Each assertion was proven red against a mutant of the guard it covers, including a first-character-only predicate, an unbounded digit run, and an engine still reading the run with sscanf. -N2147483647 is what pins that last one on a 64-bit host: it is in range for an int and out of range for -N, so it fails the same way the i386 leg did.

Three of the test's own assumptions were POSIX-only and each showed up on a different leg. It read -N past INT_MAX as portable, which i386 contradicts. It compared against a path a trailing-slash TMPDIR gave a // the engine normalizes away, which macOS contradicts, fixed in test-timeout.sh so every test benefits. And it built its two absolute-path expectations from the POSIX form, while url_savename maps what Windows forbids in a filename to _ on every platform, so a drive-letter TMPDIR reaches disk as D_; those now derive through the same substitution rather than a hardcoded D_.

Closes #1380

The same failure family on the param class, which this does not touch, is filed as #1426.

xroche and others added 3 commits August 24, 2026 22:41
The alias table classed -N as "param", which glues the value onto the short
form. The engine reads a glued -N value as a preset number and only a detached
one as a user template, so --structure "%h%p/%n%q.%t" was never consumed: the
token fell through to the URL list, the mirror ran with the default structure
and exited 0.

Add a "paramn" class that glues a bare digit run and detaches anything else.
The presets keep gluing, a template now reaches the engine, and -N 1 means
preset 1 rather than a user template literally named 1.

--user-structure, the spelling that always worked, was in no documentation;
it now appears in the help text, the man page, guide.html and cmdguide.html.

Closes #1380

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
The first predicate detached every value that was not pure digits, which
regressed three things "param" had always glued: a preset trailed by more short
options (--structure=1L0 collapsed the mirror onto a file named 1L0 and dropped
-L0), the on/off values every param row takes, and an overlong digit run, where
gluing sent sscanf("%d") past INT_MAX onto -1 and crashed url_savename.

paramn now glues on/off, and a digit run of at most nine digits whose remainder
carries no path separator and no extension dot. An option cluster keeps gluing,
and a template that opens with a digit still detaches. The short arm keeps the
strict all-digits test, since a detached -N template may legitimately start
with a digit.

--structure=4294967295 aborts on master; both spellings now mirror normally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
optreal_find matches -N exactly, so the glued spelling never reaches the alias
table and PARAMN_MAX_DIGITS could not guard it: -N4294967295 still wrapped
sscanf("%d") onto -1, which is "userdef" with an empty template, and crashed
url_savename. Only the detached form means userdef, so the engine clamps a
negative savename_type back to the default. Test 359 asserted that value did
not abort while it still did, which is why the assertion is now proven red
against an unclamped engine.

Naming the off value by hand also left its neighbours in: --structure=OFF and
=flat, =none, =default, =yes exited 0 and wrote the whole mirror as ./flat and
./flat-2, where master errors. A template with no % maps every URL onto one
local name, so --structure now refuses such a value by that property rather
than by enumeration, and --user-structure still takes it verbatim.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche and others added 2 commits August 25, 2026 02:05
sscanf("%d") past INT_MAX is undefined, and glibc does not answer the same
way on every width: on LP64 the value truncates negative, so the "< 0" clamp
caught it, but on ILP32 scanf clamps to LONG_MAX == INT_MAX, which is positive
and sailed straight past. -N4294967295 then reached url_savename as preset
2147483647, whose 47 sub-layout flattens the whole mirror onto ./page.html.

Read the digit run directly instead: count the digits, and take atoi() only
when the run is short enough to fit, so nothing depends on an int's width.
The bound is the one the alias table already applies, now shared as
HTS_SAVENAME_PRESET_MAX_DIGITS rather than spelled twice.

-N2147483647 pins the ILP32 half on any host: it is in range for an int and
out of range for -N, so an engine that still parsed with sscanf reds on it.

Also drop the trailing slash macOS leaves on TMPDIR before test-timeout.sh
glues "/ht.$$" onto it. The "//" that produced is a path no test can compare
against what the engine writes, which is how 359 failed there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
…tizer

url_savename maps every character Windows forbids in a filename to '_', and it
does so on every platform, not under an #ifdef. Test 359 built its two
absolute-path expectations straight from the POSIX form, so a drive-letter
TMPDIR made it demand D:/a/... where the engine had written D_/a/... .

Route both through a localname() that applies the same substitution, rather
than hardcoding D_: the expectation is then correct wherever the test runs,
and unchanged on a host whose TMPDIR holds no such character. ':' and '~' are
the two the sanitizer rewrites that a temp path can actually carry, the latter
from an 8.3 name like RUNNE~1.

Win32 stopped at --structure=100, but --structure=0 was the same bug one line
later, through default_layout. Both are covered; every other expectation in the
file is a relative literal with nothing the sanitizer touches, and none collides
with hts_tbdev or hts_tbreserved.

Proven by running the suite with a colon in TMPDIR, which reproduces the Win32
failure on Linux and reds each of the two lines on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
@xroche
xroche merged commit 3188d73 into master Aug 25, 2026
45 of 46 checks passed
@xroche
xroche deleted the fix-1380-structure-template branch August 25, 2026 06:30
xroche added a commit that referenced this pull request Aug 25, 2026
#1418 gave --structure its own "paramn" class, so the -N guard this
branch carried to spare it is gone: the rule is scoped to "param",
which is now exactly the 27 rows the issue lists. Both sides' helpers
sit side by side, and #1418's own refusal drops its dangling help tab
the way the other four syntax errors here do.

362 tests the class boundary where it is real: --structure=1L0 is a
preset trailed by more short options, which the param rule would
refuse, and whose -L0 tail shows on disk. The self-test's --structure
assertions are dropped as duplicates of #1418's own block.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Aug 25, 2026
…cked (#1427)

The `param` class glued a long option's value onto its short form
without looking at it. A value the short form cannot read converts short
or not at all, so the option keeps a value nobody asked for, and the
tail spills into the cluster loop as more short options.
`--long-names=yes` and `--long-names=none` mirror in DOS 8.3 names,
which is what `--long-names=0` means, and `--sockets=8I0` mirrors with
no top `index.html` because the `I0` reached `-I0`. Both exit 0. A
`param` value is now a bounded digit run, the `on`/`off` mapping the
class already had, or one separator with a digit after it where the
parser reads one (`-m` takes `N,N2`, `-%c` a rate with a decimal point).
Anything else draws the refusal the other classes have used since #1195,
across all 27 `param` rows, asserted table-wide in the `optalias`
self-test so a new row is covered the day it is added.

Refusal rather than a warning, because carrying on does not preserve the
run: the value's characters turn other options on, so the warning would
be followed by a wrong mirror under exit 0, which a script cannot see.
Refusal is not a new outcome here either, since an uppercase junk value
already exits 255 on every one of these rows, `OFF` reaching `-O`, with
a message naming an option nobody typed. `--cache=OFF` now says `Option
--cache does not take the value OFF`. Along the way: `--cache`'s help
string was `--retries`'s, and 19 of these rows carry no help string at
all, whose dangling tab read as a truncated message.

`--structure` is out of scope, having its own `paramn` class since
#1418, which this branch is merged onto. Three gaps stay open behind
that class, all pre-existing and all surviving #1418, which keeps
`8I0`-shaped gluing by design: `--structure=I0` is byte-identical to
`-I0`, exit 0 with `index.html` and the two GIFs silently missing;
`--structure=on <URL>` maps to a bare `-N`, which eats the next
argument, so the crawl mirrors nothing and still exits 0; and
`--long-names=` with an empty value is the bare `-L`, which is 8.3, so
the headline symptom stays reachable by that spelling.

Closes #1426

---------

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
xroche added a commit that referenced this pull request Aug 25, 2026
The bound #1418 introduced counts digits, so a zero-padded run falls off it:
-L0000000001 selected ISO9660 where -L1 selects long names, -N0000000001 lost
its preset, and --structure=0000000001 was refused outright. optalias_digits_fit
skips the leading zeros before applying the bound, and the four sites that read
such a run share it.

Test 368 gains the padded spellings, and its template case now carries a
literal tpl/ marker: %h%p/%n%q.%t is the default layout for a file:// URL, so
it passed even with --structure back on the pre-#1418 "param" class.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Aug 25, 2026
Two of the three spellings in #1434 mirrored the wrong thing at exit 0,
both because the value mapped onto the bare short form.

`--structure=on` expanded to a bare `-N`, which reads the next word as a
user template, so `--structure=on <URL>` swallowed the URL and mirrored
nothing. "on" now names the default preset (`-N0`), the layout "off"
already selected; #1418's glue rules are otherwise untouched.

`--long-names=` expanded to a bare `-L`, whose `sscanf` converted
nothing and left the switch to re-map the value already stored, landing
on DOS 8.3 names (`-L1 -L` did the same). Rather than refuse the empty
value I fixed the short form: a bare `-L` now selects L1, the default
the help and man page already star. Refusing would have taken `sockets=`
and the config-file `long-names=` spelling with it, and this way the
empty value means the option's own default. `-L0`, `--long-names=off`
and `--long-names=2` are unchanged.

Rewriting that parse surfaced a third bug, pre-existing and folded in
here: the digit-run bound #1418 added counts digits rather than reading
the value, so a zero-padded run falls off it. `-L0000000001` selected
ISO9660, `-N0000000001` lost its preset, and `--structure=0000000001`
was refused. `optalias_digits_fit` skips the leading zeros first, shared
by the four sites that read such a run.

The 27 `param` rows keep #1427's 16-character cap: its bound is a
magnitude, not a run length, so `optalias_digits_fit` does not fit there
— 9 significant digits would refuse `--advanced-maxlinks=2000000000`,
and it cannot see the second operand of `-m N,N2`. Filed as #1437.

The third spelling in the issue, `--structure=I0`, has been refused
since #1418 landed after the issue was measured, and test 368 pins that.
What survives is `--structure=8I0`: the digits-then-cluster form #1418
glues on purpose, and the same shape as the `--structure=1L0` that
#1427's test pins as accepted. No rule separates the two, so that one
wants a decision rather than a patch.

Closes #1434

---------

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
xroche added a commit that referenced this pull request Aug 25, 2026
`--structure=8I0` glued preset 8 onto `-I0` and mirrored with no index
files at exit 0. Nothing separates that from `--structure=1L0`, which
was accepted on purpose and which the alias table's `param` comment
advertises as cluster mixing. Sorting tails into wanted and unwanted
needs a rule nobody can state, so `--structure` now refuses all of them.

This is a deliberate CLI break rather than a bug fix. `--structure=1L0`,
`=1c8`, `=1%c8`, `=8I0` and any other digits-then-tail value stop
working, in both the `=` and the space-separated spelling; all were
accepted since before #1418. Presets, `on`/`off` and templates are
untouched, and the template test is unchanged: a path separator or an
extension dot still marks one, so `--structure=2col/%n.%t` keeps
working, and `--user-structure` takes a tail-shaped name verbatim.

`-N1L0` still clusters. There the cluster is explicit and it is the
generic short-option scanner walking characters, so refusing it would
change short-option grammar instead of this one option. Test 370 covers
the refusals against mirrored output; 359, 362 and 368 each pinned a
tail case and now pin the refusal, keeping the coverage those cases
carried besides the tail, including `-N1L0` itself.

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--structure takes only the numeric presets, and --user-structure is undocumented

1 participant