Bug#982716: [Aptitude-devel] Bug#982716: aptitude: FTBFS: tests failed
Hi David,
Axel Beckert wrote:
> > So I guess what is intended here is more like:
> > | char * endptr;
> > | errno = 0;
> > | auto score_tweaks = strtol(action.c_str(), , 10);
> > | if (errno != 0 || *endptr != '\0')
>
> I applied the following patch locally:
>
> --- a/src/generic/apt/aptitude_resolver.cc
> +++ b/src/generic/apt/aptitude_resolver.cc
> @@ -673,7 +673,10 @@
>else
> {
>unsigned long score_tweak = 0;
> - if(!StrToNum(action.c_str(), score_tweak, action.size()))
> + char * endptr;
> + errno = 0;
> + auto score_tweaks = strtol(action.c_str(), , 10);
> + if (errno != 0 || *endptr != '\0')
> {
> // TRANSLATORS: actions ("approve", etc.) are keywords and should
> // not be translated
>
> The initially failing test indeed seems fixed, but now another test
> fails:
Thanks for the hint on being untested and written without copy &
paste. There was indeed a typo in there which was not detected by the
compiler: score_tweak vs score_tweaks (singular vs plural in variable
name). Fixing that and removing the leading "auto" variable
declaration seems the proper fix.
Thanks for the help nevertheless!
Will upload the fix soon.
Regards, Axel
--
,''`. | Axel Beckert , https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
`-| 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE
Bug#982716: [Aptitude-devel] Bug#982716: aptitude: FTBFS: tests failed
On Sat, Feb 13, 2021 at 06:11:03PM +0100, Lucas Nussbaum wrote: > Relevant part (hopefully): […] > > FAIL: cppunit_test […] | aptitude_resolver.cc:680 ERROR - Invalid hint "-143 aptitude <4.3.0": the action "-143" should be "approve", "reject", or a number. The test uses aptitude_resolver::hint::parse in src/generic/apt/aptitude_resolver.cc which in line 676 uses StrToNum to parse the hint which fails with apt >= 2.1.19 as StrToNum is refusing to parse negative numbers now. The data type of StrToNum is unsigned and using strtoull internally which works on an unsigned long long (ull), too, but defines that for negative numbers "the negation of the result of the conversion" is returned… which tends to be unexpected (Negative numbers played a minor role in e.g. CVE-2020-27350 for example). You could convert to using strtoul directly to replicate the old behaviour, with something like | char * endptr; | auto score_tweaks = strtoul(action.c_str(), , 10); | if (*endptr != '\0') (ideally you would check errno for failures of the conversion, but StrToNum wasn't doing that either in the past, so to replicate bugs… it does do a few other things instead, but they are not relevant here aka: it was an odd choice from the start and the only place it is used in aptitude) BUT a bit further down the number is reinterpreted as a signed int which suggests to me that aptitude wasn't actually expecting to get a potentially huge positive value for a negative number, but would in fact prefer to get a negative number if it parsed one and it just didn't matter for this test either way (and negative hints by users are probably not that common, too). So I guess what is intended here is more like: | char * endptr; | errno = 0; | auto score_tweaks = strtol(action.c_str(), , 10); | if (errno != 0 || *endptr != '\0') Note that I have not checked my hypotheses. (The code samples are also typed in my mail client, so I have probably included some typos letting them not even compile.) Sorry for this breaking change this late in the cycle! If its any consolation I am also angry that I not only not managed to finish the fuzzing project in time, but also not managed to salvage the more useful bit in a more timely fashion either. Best regards David Kalnischkies signature.asc Description: PGP signature
Bug#982716: [Aptitude-devel] Bug#982716: aptitude: FTBFS: tests failed
Control: tag -1 + confirm Hi Lucas, thanks for the bug report! Lucas Nussbaum wrote: > Source: aptitude > Version: 0.8.13-2 > Severity: serious […] > During a rebuild of all packages in sid, your package failed to build > on amd64. Indeed, can reproduce it here locally despite it still worked a few weeks ago. Wouldn't have expected this at this stage of the freeze. :-/ Wonder who^Wwhat broke that. Anyway, will investigate. Regards, Axel -- ,''`. | Axel Beckert , https://people.debian.org/~abe/ : :' : | Debian Developer, ftp.ch.debian.org Admin `. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5 `-| 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE
