Bug 1251453 - RFE: reject invalid Epoch in versioned deps
Summary: RFE: reject invalid Epoch in versioned deps
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: rpm
Version: 22
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Packaging Maintenance Team
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-08-07 10:46 UTC by Michael Schwendt
Modified: 2015-10-26 19:18 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-10-23 13:26:57 UTC


Attachments (Terms of Use)

Description Michael Schwendt 2015-08-07 10:46:28 UTC
Please consider rejecting invalid Epoch values in versioned dependencies during rpmbuild.

rpmbuild rejects Epoch tags, which don't contain an unsigned number, but rpmbuild doesn't do the same for versioned dependencies.

[...]

For over a month, an unresolvable dependency in a "blktap" package has not been discovered by repoclosure for the daily Rawhide report. As a result, the same broken package has been pushed also to other dist releases.

It turnt out that:

* repoclosure doesn't find an undefined %{epoch} macro in a Requires, because in the repo metadata the bad Epoch is replaced with "epoch=0"

* the repodata tools createrepo_c (and createrepo) parse Epoch values with strtol(), and if not parsable they default to "0"

* rpmbuild doesn't reject bad Epoch values in versioned deps, it only warns about too many ':', for example, one can even specify a negative Epoch:
  -> https://lists.fedoraproject.org/pipermail/devel/2015-August/213208.html

* in package tools such as DNF, such an unresolvable dependency is not detected prior to RPM's transaction check

Comment 1 Michael Schwendt 2015-10-22 20:47:05 UTC
build/parseReqs.c:

   175              EVR = xmalloc((ve-v) + 1);
   176              rstrlcpy(EVR, v, (ve-v) + 1);
   177              if (rpmCharCheck(spec, EVR, ve-v, ".-_+:%{}~")) goto exit;
   178  
   179              /* While ':' and '-' are valid, only one of each is valid. */
   180              if (checkSep(EVR, '-', &emsg) || checkSep(EVR, ':', &emsg))
   181                  goto exit;
   182  

Why does it put the characters "%{}" in the rpmCharCheck whitelist?

At this point in parseReqs.c, spec macros are substituted already.
rpmCharCheck tells "All alphanums are considered sane.", so the whitelist accepts also EVRs which include %{}.

Comment 2 Ľuboš Kardoš 2015-10-23 13:26:57 UTC
Fixed in rpm-4.13.0-0.rc1.6.fc24

Comment 3 Panu Matilainen 2015-10-24 09:15:03 UTC
(In reply to Michael Schwendt from comment #1)
> Why does it put the characters "%{}" in the rpmCharCheck whitelist?
> 
> At this point in parseReqs.c, spec macros are substituted already.
> rpmCharCheck tells "All alphanums are considered sane.", so the whitelist
> accepts also EVRs which include %{}.

For various wrong reasons.

It was stricter at one point but got reverted "for now", some six years ago :) See bug 547997 and https://github.com/rpm-software-management/rpm/commit/507f21f6bb4bf7029a0bca255cfe4aae3361f358 for one of the excuses.

There are various other cases where too where known-bad unexpanded macros are  let through. Can't recall the details anymore (git log might), but one of the situations might've been related to side-effects of recursive spec-parsing (when buildarch is set).

Comment 4 Orion Poplawski 2015-10-26 16:47:20 UTC
This seems to have broken building some python srpms:

Executing command: ['bash', '--login', '-c', '/usr/bin/rpmbuild -bs --target noarch --nodeps /builddir/build/SPECS/scipy.spec'] with env {'LANG': 'en_US.UTF-8', 'TERM': 'vt100', 'SHELL': '/bin/bash', 'PROMPT_COMMAND': 'printf "\x1b]0;<mock-chroot>\x07<mock-chroot>"', 'PATH': '/usr/bin:/bin:/usr/sbin:/sbin', 'HOME': '/builddir', 'HOSTNAME': 'mock'} and shell False
error: line 53: Invalid version (epoch must be unsigned integer): %{epoch}:0.16.1-1.fc24: Provides:       scipy = %{epoch}:0.16.1-1.fc24

This is from the automatically generated Provides form %python_provide.  It does:

        vr = rpm.expand("%{?epoch:%{epoch}:}%{version}-%{release}")

to generate the version request, so perhaps epoch is getting handled inconsistently?

Comment 5 Michael Schwendt 2015-10-26 19:16:09 UTC
Are you sure about that? I see a plain

  Provides:       scipy = %{epoch}:%{version}-%{release}

in the spec file, but no Epoch tag nor a definiton of %epoch. Here's the related commit: http://pkgs.fedoraproject.org/cgit/scipy.git/commit/?id=7283e398871d176675e7f4646676ddf18ede5e8b

Comment 6 Orion Poplawski 2015-10-26 19:18:39 UTC
Ah, good catch.  Thanks.


Note You need to log in before you can comment on or make changes to this bug.