Project

General

Profile

Bug #8919

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

Added by anonym almost 5 years ago. Updated almost 5 years ago.

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

100%

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

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.

Associated revisions

Revision 34bfd5cf (diff)
Added by Tails developers almost 5 years ago

Use double quotes so #{...} is actually evaluated and not taken literally.

See the ticket, #8919, for details.

Will-fix: #8919

Revision 714917d9
Added by Tails developers almost 5 years ago

Merge branch 'bugfix/8919-fix-tails_persistence_enabled' into testing

Fix-committed: #8919

History

#1 Updated by Tails almost 5 years ago

  • Status changed from Confirmed to In Progress

Applied in changeset commit:b1f0d0b9c0cca1a8bf2733ff6e726f342b3b0f38.

#2 Updated by anonym almost 5 years ago

  • 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 almost 5 years ago

  • Assignee set to intrigeri

#4 Updated by intrigeri almost 5 years ago

  • % Done changed from 50 to 60

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

#5 Updated by Tails almost 5 years ago

  • Status changed from In Progress to 11
  • % Done changed from 60 to 100

Applied in changeset commit:102c3b528e902b325c0799681983bc3d70de35fd.

#6 Updated by intrigeri almost 5 years ago

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

#7 Updated by BitingBird almost 5 years ago

  • Status changed from 11 to Resolved

Also available in: Atom PDF