Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3172 closed Bug (fixed)

transmission overwrites settings.json

Reported by: geirha Owned by: kklimonda
Priority: Normal Milestone: 1.93
Component: Daemon Version: 1.92
Severity: Normal Keywords:


This breaks the transmission-daemon deb package which sets the config dir to /var/lib/transmission-damon/info/, but settings.json at /etc/transmisison-daemon/settings.json, with a symlink /var/lib/transmission-damon/info/settings.json -> /etc/transmisison-daemon/settings.json.

When transmission-daemon writes the config on SIGHUP or exit, it writes it to a temporary file, then renames it to /var/lib/transmission-damon/info/settings.json, overwriting the symlink, so subsequent changes to /etc/transmission-daemon/settings.json will no longer be read.

Possible fixes I can think of:

  1. Leave it how it is and have downstream deal with it; alter the package to turn the symlink around, having the symlink under /etc/, pointing to /var/...
  1. Don't write to a tempfile then rename, but write directly to the file.
  1. Do a readlink first, and overwrite that file instead, however, what to do if there are several levels of symlinks?

Attachments (2)

readlink.diff (1.3 KB) - added by charles 13 years ago.
untested diff
follow_symlinks.patch (934 bytes) - added by kklimonda 13 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 13 years ago by charles

  • Resolution set to invalid
  • Status changed from new to closed

Possible fix number 2 is not an option, since it would reopen the settings.json-gets-lost-when-the-disk-is-full bug that the tmpfile code attempts to solve.

Probably option number 3 is the way to go. We could keep drilling down until EINVAL is reached. This would be bad if there are cyclic symbolic links, but I don't think think there's a Right Thing to do in a pathological case like that anyway.

Changed 13 years ago by charles

untested diff

comment:2 Changed 13 years ago by charles

  • Resolution invalid deleted
  • Status changed from closed to reopened

Whoops, I didn't mean to close this.

comment:3 Changed 13 years ago by charles

geirha: could you take a look at the diff and see if it's what you had in mind? Could you give it a test to see if it works, both for the gui, but also for the deb script you're using?

comment:4 Changed 13 years ago by charles

Background on why we're now using mkstemp() + rename() for writing the json and bencoded files to disk: to solve #2505, where (for example) settings.json gets lost on shutdown when the disk is full.

comment:5 Changed 13 years ago by geirha

And option number 3 was not as easy as readlink(2) in a loop since it won't do any magic with regards to relative symlinks. There is, however, realpath(3) which seems to do all the necessary magic, and appears to be portable enough.

There's another issue with option 3 though, which is that once the "real" file is found, we'll need write access to its containing directory if we want to overwrite it. And in the case of the ubuntu (and likely debian) package, /etc/transmission-daemon/ is only writable to root, so it still requires a downstream change.

Changed 13 years ago by kklimonda

comment:6 Changed 13 years ago by charles

  • Keywords backport-1.9x added
  • Milestone changed from None Set to 2.00
  • Owner set to kklimonda
  • Status changed from reopened to new

Fixed in trunk for 2.00.

kklimonda: Thanks for the patch!

20:56:36 < CIA-40> charles * r10549 libtransmission/bencode.c: (trunk libT) #3172 "transmission overwrites settings.json" -- fixed with kklimonda's patch for 2.00

comment:7 Changed 13 years ago by charles

  • Resolution set to fixed
  • Status changed from new to closed

comment:8 Changed 13 years ago by charles

  • Keywords backport-1.9x removed
  • Milestone changed from 2.00 to 1.93

backported to 1.9x by r10584

comment:9 Changed 13 years ago by charles

r10549 and r10584 both have a portability error reported by nahkiss in irc and fixed in r10629

Note: See TracTickets for help on using tickets.