Bug #8919

tails_persistence_enabled? has buggy use of single-quotes and #{...}

Added by anonym 2015-02-19 08:45:47 . Updated 2015-02-24 22:40:36 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Tails_1.3
Start date:
2015-02-19
Due date:
% Done:

100%

Feature Branch:
bugfix/8919-fix-tails_persistence_enabled
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

The function tails_persistence_enabled? in features/step_definitions/usb.rb has this line:

@vm.execute('. #{persistence_state_file} && ' +


The single-quotes should make it so that #{persistence_state_file} is interpreted literally, not evaluated etc. We want double-quotes here.

How can this function possibly work? It seems to do so since it uses both its true and false case in various steps. However, my reading is that it should always return false since . #{persistence_state_file} should evaluate to false in the shell.

Subtasks

History

#1 Updated by Tails 2015-02-20 11:24:43

  • Status changed from Confirmed to In Progress

Applied in changeset commit:b1f0d0b9c0cca1a8bf2733ff6e726f342b3b0f38.

#2 Updated by anonym 2015-02-20 11:28:45

  • Assignee deleted (anonym)
  • Priority changed from Elevated to Normal
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/8919-fix-tails_persistence_enabled

I ran a specially crafted scenario with --debug, and when tails_persistence_enabled? was run I indeed saw this:

calling as root: . #{persistence_state_file} && test "$TAILS_PERSISTENCE_ENABLED" = true
call returned: [0, "", ""]

At first I thought it surprising that this returned 0, since $TAILS_PERSISTENCE_ENABLED should expand to the empty string, which isn’t equal to true. However… note that we introduce a #, so after the . we have a comment! Since we run the remote shell commands in dash, . will return success, unlike in bash where it will fail and complain about the missing argument. Fun stuff!

So, let’s analyze if this may have messed up all our previous test runs. Here’s the full function:

def tails_persistence_enabled?
  persistence_state_file = "/var/lib/live/config/tails.persistence"
  return @vm.execute("test -e '#{persistence_state_file}'").success? &&
         @vm.execute('. #{persistence_state_file} && ' +
                     'test "$TAILS_PERSISTENCE_ENABLED" = true').success?
end


Since the second @vm.execute always will be true, it all boils down to the first one, so this function has just checked for the existence of the persistence_state_file. In general, that should be enough, and the second @vm.execute was just intended as an extra safe-guard, so we’re fine. Lowering priority because of this.

I’ve pushed a (very simple) fix.

#3 Updated by intrigeri 2015-02-20 15:17:08

  • Assignee set to intrigeri

#4 Updated by intrigeri 2015-02-20 15:33:27

  • % Done changed from 50 to 60

Code review now passes, with commit:7cec2299173ba0930834392c06448e9d8c1b4c7c on top (ACK’d by anonym privately).

#5 Updated by Tails 2015-02-20 21:34:32

  • Status changed from In Progress to Fix committed
  • % Done changed from 60 to 100

Applied in changeset commit:102c3b528e902b325c0799681983bc3d70de35fd.

#6 Updated by intrigeri 2015-02-20 21:35:54

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Pass

#7 Updated by BitingBird 2015-02-24 22:40:36

  • Status changed from Fix committed to Resolved

#8 Updated by intrigeri 2015-03-21 11:15:39

  • blocked by #8538 added