Bug 196435 - Prevent @sample keyword from removing user symlinks
Summary: Prevent @sample keyword from removing user symlinks
Status: Closed Feedback Timeout
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-02 12:05 UTC by Jan Beich
Modified: 2016-04-13 11:31 UTC (History)
1 user (show)

See Also:


Attachments
v1 (811 bytes, patch)
2015-01-02 12:05 UTC, Jan Beich
no flags Details | Diff
v2 (870 bytes, patch)
2015-01-02 15:34 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2015-01-02 12:05:05 UTC
Created attachment 151209 [details]
v1

If .conf file listed as @sample is replaced by user with a symlink the content may be lost on upgrade because:

- post-install ignores existing non-regular files
- post-deinstall treats symlink as a regular file

An example of such usage is security/ca_root_nss with only one .sample file copied/edited by user while the rest point to it.

  /etc/ssl/cert.pem (regular file, user-modified)
  /usr/local/etc/ssl/cert.pem (symlink to /etc/ssl/cert.pem)
  /usr/local/openssl/cert.pem (symlink to /etc/ssl/cert.pem)
Comment 1 Mathieu Arnold freebsd_committer freebsd_triage 2015-01-02 14:07:05 UTC
Then don't use -e, use -f "${target_file}" -o -L "${target_file}", I'd rather have an error when it's a directory, or a device, or something strange.

Also, for cmp drop the -h, we want symlinks to be followed, otherwise, it'll compare the content of the file with the path of the sample file, which, in most cases, won't be the same ;-)
Comment 2 Jan Beich freebsd_committer freebsd_triage 2015-01-02 15:34:14 UTC
Created attachment 151216 [details]
v2

(In reply to Mathieu Arnold from comment #1)
> Also, for cmp drop the -h, we want symlinks to be followed, otherwise, it'll
> compare the content of the file with the path of the sample file, which, in
> most cases, won't be the same ;-)

Indeed, I've traded one issue for another:

  - symlink target_file vs. regular sample_file (no filetype check)
  - symlink target_file vs. symlink sample_file (no symlink path check)
  - regular target_file vs. regular sample_file
  - regular target_file vs. symlink sample_file

after

  - symlink target_file vs. regular sample_file (path vs. content)
  - symlink target_file vs. symlink sample_file
  - regular target_file vs. regular sample_file
  - regular target_file vs. symlink sample_file (path vs. content)

Let's try to assume target_file cannot be a symlink if unmodified by user or bug 196432 remains closed.
Comment 3 Mathieu Arnold freebsd_committer freebsd_triage 2015-01-02 16:21:21 UTC
(In reply to Jan Beich from comment #2)
> Created attachment 151216 [details]
> v2
> 
> (In reply to Mathieu Arnold from comment #1)
> > Also, for cmp drop the -h, we want symlinks to be followed, otherwise, it'll
> > compare the content of the file with the path of the sample file, which, in
> > most cases, won't be the same ;-)
> 
> Indeed, I've traded one issue for another:
> 
>   - symlink target_file vs. regular sample_file (no filetype check)
>   - symlink target_file vs. symlink sample_file (no symlink path check)
>   - regular target_file vs. regular sample_file
>   - regular target_file vs. symlink sample_file
> 
> after
> 
>   - symlink target_file vs. regular sample_file (path vs. content)
>   - symlink target_file vs. symlink sample_file
>   - regular target_file vs. regular sample_file
>   - regular target_file vs. symlink sample_file (path vs. content)
> 
> Let's try to assume target_file cannot be a symlink if unmodified by user or
> bug 196432 remains closed.

But, hum, target_file can never be a symlink, it will always be a regular file.
Comment 4 Jan Beich freebsd_committer freebsd_triage 2015-01-02 16:53:32 UTC
This bug is about user-modified target_file. How the file was modified isn't limited to content but includes differnt filetypes as well such as being replaced by a symlink. It usually happens after package install but before deinstall.

Here're steps to reproduce:

  1/ install a port with @sample in pkg-plist
  2/ replace target_file with a symlink to sample_file
  3/ deinstall the port
  4/ notice target_file now a symlink disappeared as well

or

  1/ find a port with @sample in pkg-plist
  2/ create matching empty config but without .sample suffix
  3/ swap it with a symlink
  4/ install the port
  5/ notice symlink destination is no longer empty
Comment 5 Mathieu Arnold freebsd_committer freebsd_triage 2015-01-02 16:58:16 UTC
(In reply to Jan Beich from comment #4)
> This bug is about user-modified target_file. How the file was modified isn't
> limited to content but includes differnt filetypes as well such as being
> replaced by a symlink. It usually happens after package install but before
> deinstall.
> 
> Here're steps to reproduce:
> 
>   1/ install a port with @sample in pkg-plist
>   2/ replace target_file with a symlink to sample_file
>   3/ deinstall the port
>   4/ notice target_file now a symlink disappeared as well
> 
> or
> 
>   1/ find a port with @sample in pkg-plist
>   2/ create matching empty config but without .sample suffix
>   3/ swap it with a symlink
>   4/ install the port
>   5/ notice symlink destination is no longer empty

Well, yes, I'm ok with that, people do stupid things, people get themselves shot in the foot.  As in, don't replace the regular file that has been installed by a symlink, it's stupid. :-)
Comment 6 Mathieu Arnold freebsd_committer freebsd_triage 2015-01-02 17:07:25 UTC
but I do see your point.  In any case, you should replace the ! [ -L "${target_file}" ] by [ -f "${target_file}" ] no need to get things overly complicated.
Comment 7 Jan Beich freebsd_committer freebsd_triage 2015-01-02 17:36:42 UTC
|test -f| isn't reliable

  $ ln -s foo bar
  $ [ -f bar ]; echo $?
  1
  $ touch foo
  $ [ -f bar ]; echo $?
  0

while test(1) mangpage says:

  If file is a symbolic link, test will fully dereference it and then
  evaluate the expression against the file referenced, except for the -h
  and -L primaries.
Comment 8 Mathieu Arnold freebsd_committer freebsd_triage 2016-04-13 11:31:10 UTC
If this is still a problem, please reopen the PR.