Bug 67419 - find-requires sed bash too few backslashes
find-requires sed bash too few backslashes
Status: CLOSED DEFERRED
Product: Red Hat Linux
Classification: Retired
Component: rpm-build (Show other bugs)
7.3
All Linux
medium Severity low
: ---
: ---
Assigned To: Jeff Johnson
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2002-06-24 15:33 EDT by Edward J. Huff
Modified: 2008-05-01 11:38 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2002-06-25 13:52:06 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
patch for /usr/lib/rpm/find-requires (398 bytes, patch)
2002-06-25 07:31 EDT, Edward J. Huff
no flags Details | Diff
bash script demonstrating problem and solution. (2.55 KB, application/octet-stream)
2002-06-25 18:38 EDT, Edward J. Huff
no flags Details
Results of running demo script. (7.33 KB, text/plain)
2002-06-25 18:41 EDT, Edward J. Huff
no flags Details

  None (edit)
Description Edward J. Huff 2002-06-24 15:33:23 EDT
From Bugzilla Helper:
User-Agent: Mozilla/4.78 [en] (X11; U; Linux 2.4.9-31 i586)

Description of problem:
The sed script which is supposed to replace ' and " in file names with \' and \"
instead replaces them with &.  

sed needs to see s/['"]/\\&/g

In order to get bash to send this to sed, you have to put in a vast number of
backslashes (well, 5 to be exact) if the string is enclosed in backquotes and
double quotes.  A much better way is to use $() in place of the backquotes and
single quotes in place of the double quotes.  Then you just need to write the
correct number of backslashes which sed needs.

A patch is enclosed.


Version-Release number of selected component (if applicable):


How reproducible:
Always

Steps to Reproduce:
1. bash
2. set -x -v
3. echo a\'b | (filelist=`sed "s/['\"]/\\\&/g"`; echo $filelist)
	

Actual Results:  bash$ echo a\'b | (filelist=`sed "s/['\"]/\\\&/g"`; echo
$filelist)
echo a\'b | (filelist=`sed "s/['\"]/\\\&/g"`; echo $filelist)
+ echo 'a'\''b'
sed "s/['\"]/\\&/g"
++ sed 's/['\''"]/\&/g'
+ filelist=a&b
+ echo 'a&b'
a&b

Expected Results:  bash $ echo a\'b | (filelist=`sed "s/['\"]/\\\\\&/g"`; echo
$filelist)
echo a\'b | (filelist=`sed "s/['\"]/\\\\\&/g"`; echo $filelist)
+ echo 'a'\''b'
sed "s/['\"]/\\\&/g"
++ sed 's/['\''"]/\\&/g'
+ filelist=a\'b
+ echo 'a\'\''b'
a\'b

Additional info:

bash$ echo a\'b | (filelist=$(sed 's/['\''"]/\\&/g'); echo $filelist)
echo a\'b | (filelist=$(sed 's/['\''"]/\\&/g'); echo $filelist)
+ echo 'a'\''b'
sed 's/['\''"]/\\&/g'
++ sed 's/['\''"]/\\&/g'
+ filelist=a\'b
+ echo 'a\'\''b'
a\'b
Comment 1 Jeff Johnson 2002-06-24 15:54:37 EDT
And what problem are you trying to solve?

Permitting embedded quotes in filenames
passed to find-requires?
Comment 2 Edward J. Huff 2002-06-25 07:31:42 EDT
Created attachment 62484 [details]
patch for /usr/lib/rpm/find-requires
Comment 3 Jeff Johnson 2002-06-25 09:38:14 EDT
iAgain, please, what problem are you trying to solve?
Comment 4 Edward J. Huff 2002-06-25 13:52:02 EDT
Sorry about not replying, I just received the question.

I was reading Maximum RPM, this section:

http://www.rpm.org/max-rpm/s1-rpm-depend-auto-depend.html#S3-RPM-DEPEND-FIND-REQUIRES

I looked at the current find-requires script.  It is now quite a bit bigger. 
One of the changes was the addition of this sed script.  I noticed a bug, so I
reported it.

I would say that the original author of the modification to find-requires was
trying to permit quotes in filenames, but he failed.  Leaving it wrong is like
leaving spelling errors in comments.  Either fix it or take it out.

Where is find-requires called from?  Is its input prefiltered?
What exactly are the rules for these filenames?
Comment 5 Jeff Johnson 2002-06-25 14:09:08 EDT
OK. I doubt that quotes in file names was
the motivatyion, as that's pretty sick.

I supect the motivation was to preserve
spaces in file names, where the need is to
replace and restore quotes correctly, not to
preserve the quote value. I also believe
that the existing script permits spaces
in file names even though, indeed, quotes
within file names are almost certainly broken.

There are no explicit rules for file names supported
by rpm. There can and will be breakage with utf8 in
file names.

find-requires is called for most every package
built by rpm, on stdin is the glob expanded %files
manifest, on stdout is deliverd the detected requirements.

Changing the behavior of find-requires -- since it's
used everywhere -- for the purpose of supporting file
names with embedded quotes is, well, not exactly
a typo in a comment sort of change. YMMV.
Comment 6 Edward J. Huff 2002-06-25 18:31:26 EDT
Well, if you think that find-requires deals with blanks in file names, try the
demo script which will be attached.  find-requires treats a\ b as two files a
and b, and '"a b"' as two files, &a and b&.

In fact, it changes quotes to ampersands and gives results from the wrong file.

The suggested fix does not change the behavior of find-requires except if a file
name contains a single or double quote.  In that case,  it causes find-requires
to examine the proper file, as demonstrated by the test script.

It is true that none of this really matters.  The correct fix is to add a -0
option to find-requires requesting use of null terminated file names like those
used with find -print0 and xargs -0.  But even then there is no reason to leave
it changing quotes to ampersands when -0 is not specified.
Comment 7 Edward J. Huff 2002-06-25 18:38:26 EDT
Created attachment 62624 [details]
bash script demonstrating problem and solution.
Comment 8 Edward J. Huff 2002-06-25 18:41:21 EDT
Created attachment 62625 [details]
Results of running demo script.

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