Bug 1310204

Summary: rpmspec error message contains binary data
Product: [Fedora] Fedora Reporter: Steve <y9t7sypezp>
Component: rpmAssignee: Packaging Maintenance Team <packaging-team-maint>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 23CC: ffesti, jzeleny, lkardos, novyjindrich, packaging-team-maint, pknirsch
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-02-29 09:17:30 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Attachments:
Description Flags
reverse-video.spec test file that sets reverse video on a terminal none

Description Steve 2016-02-19 18:01:23 UTC
Description of problem:
The "Unknown tag:" in this error message is binary data. The error message should contain a hexadecimal representation of the binary data.

$ rpmspec -q --srpm firefox-44.0.1-2.fc23.src.rpm
error: line 1: Unknown tag: ����
error: query of specfile firefox-44.0.1-2.fc23.src.rpm failed, can't parse

Verify with:
$ rpmspec -q --srpm firefox-44.0.1-2.fc23.src.rpm |& hexdump -C

Version-Release number of selected component (if applicable):
rpm-4.13.0-0.rc1.11.fc23.x86_64

See also:
Bug 1308133 - [Doc] rpmspec help does not match man page; man page needs usage examples (was: rpmspec fails to parse source rpm)

Comment 1 Steve 2016-02-22 03:13:39 UTC
(In reply to Steve from comment #0)
> The error message
> should contain a hexadecimal representation of the binary data.

A better idea is to verify first that the file is a text file. The "file" command knows the difference between a text file and an RPM file:

$ file firefox.spec firefox-44.0.1-2.fc23.src.rpm
firefox.spec:                  UTF-8 Unicode text
firefox-44.0.1-2.fc23.src.rpm: RPM v3.0 src

Comment 2 Florian Festi 2016-02-29 09:17:30 UTC
I added some special code to detect when a binary package is passed instead of a spec file.

Comment 3 Steve 2016-02-29 16:15:06 UTC
(In reply to Florian Festi from comment #2)
> I added some special code to detect when a binary package is passed instead
> of a spec file.

Thanks, Florian.[1] Binary data could still be written to the terminal in other cases:
$ rpmspec -q firefox-44.0.1.source.tar.xz
error: line 1: Unknown tag: �7zXZ
error: query of specfile firefox-44.0.1.source.tar.xz failed, can't parse

$ hexdump -C firefox-44.0.1.source.tar.xz | head -1
00000000  fd 37 7a 58 5a 00 00 04  e6 d6 b4 46 02 00 21 01  |.7zXZ......F..!.|

[1] https://github.com/rpm-software-management/rpm/commit/d93c97b60673d725b9eb255008b8a57dfad65fdb

Comment 4 Steve 2016-02-29 16:36:53 UTC
Created attachment 1131610 [details]
reverse-video.spec test file that sets reverse video on a terminal

This sets reverse video on a terminal, because the "rpmspec" command writes binary data to the terminal:

$ rpmspec -q reverse-video.spec 
error: line 1: Unknown tag: 
error: query of specfile reverse-video.spec failed, can't parse

The "rpm" command handles the same file gracefully:

$ rpm -q -p reverse-video.spec
error: reverse-video.spec: not an rpm package (or package manifest)

Contents of the test file:

$ hexdump -C reverse-video.spec
00000000  1b 5b 3f 35 68 00 00 00  00 00 00 00 00 00 00 00  |.[?5h...........|

Comment 5 Steve 2016-02-29 17:34:18 UTC
rpmCharCheck()[1] checks for invalid characters. Is it being called?

[1] https://github.com/rpm-software-management/rpm/blob/d93c97b60673d725b9eb255008b8a57dfad65fdb/build/parsePreamble.c#L613

Comment 6 Florian Festi 2016-02-29 17:48:41 UTC
No, but it also dumps the line unquoted into the terminal...

Comment 7 Steve 2016-02-29 18:40:29 UTC
(In reply to Florian Festi from comment #6)
> No, but it also dumps the line unquoted into the terminal...

Thanks for pointing that out. A "check" function should not be writing any error messages ...

The simplest solution might be to do what the "rpm" command does and not put any string from the file in the error message.