--structure silently ignores a template and mirrors with the default layout - #1418
Merged
Conversation
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
enabled auto-merge (squash)
August 24, 2026 22:16
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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-Nreads a glued value as a preset number and a detached one as a user template, and the alias table classed--structureasparam, 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 newparamnclass rather than a reuse ofparam1because #1379 tried that, and it turned--structure=1into a template literally named1, collapsing the mirror onto one colliding filename, also at exit 0.paramnglues what-Ncan 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 clusterparamused to glue still glues, so--structure=1L0remains 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.%thas 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=flatis a typo rather than a template and is now refused, along withOFF,none,default,yesand-1, which used to mirror the whole site as./flatand./flat-2.--user-structurestill takes such a value verbatim.The nine-digit bound exists because a longer run does not read portably.
sscanf("%d")pastINT_MAXis undefined and glibc answers differently per width: on LP64 the value truncates negative, which is "userdef" with an empty template and crashesurl_savename, while on ILP32 scanf clamps toINT_MAX, a preset whose sub-layout flattens the whole mirror onto one file.optreal_findmatches-Nexactly, so the alias table never sees the glued spelling and the engine counts the digits itself, callingatoionly on a run short enough to fit.-N4294967295and--structure=4294967295both abort on master.-N 1is preset 1 now instead of a template named1. That semantics change is deliberate and approved; a template named1has 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.-N2147483647is 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
-NpastINT_MAXas portable, which i386 contradicts. It compared against a path a trailing-slashTMPDIRgave a//the engine normalizes away, which macOS contradicts, fixed intest-timeout.shso every test benefits. And it built its two absolute-path expectations from the POSIX form, whileurl_savenamemaps what Windows forbids in a filename to_on every platform, so a drive-letterTMPDIRreaches disk asD_; those now derive through the same substitution rather than a hardcodedD_.Closes #1380
The same failure family on the
paramclass, which this does not touch, is filed as #1426.