Skip to content

Commit

Permalink
Return proper error code when unable to convert a relative path to an…
Browse files Browse the repository at this point in the history 
… absolute path.

Suggested by Yogesh Sharma.
  • Loading branch information
@dwsteele
dwsteele committed Jan 4, 2017
1 parent c8dfc67 commit 4ff2714
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 29 deletions.
13 changes: 13 additions & 0 deletions doc/xml/release.xml
View file
Expand Up @@ -102,6 +102,11 @@
<contributor-id type="github">cmwshang</contributor-id>
</contributor>

<contributor id="sharma.yogesh">
<contributor-name-display>Yogesh Sharma</contributor-name-display>
<contributor-id type="github">sharmay</contributor-id>
</contributor>

<contributor id="smith.greg">
<contributor-name-display>Greg Smith</contributor-name-display>
<contributor-id type="github">gregscds</contributor-id>
Expand Down Expand Up @@ -173,6 +178,14 @@
<release-item>
<p>Refactor <code>File</code> and <code>BackupCommon</code> modules to improve test coverage.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-ideator id="sharma.yogesh"/>
</release-item-contributor-list>

<p>Return proper error code when unable to convert a relative path to an absolute path.</p>
</release-item>
</release-refactor-list>
</release-core-list>

Expand Down
32 changes: 4 additions & 28 deletions lib/pgBackRest/Archive.pm
View file
Expand Up @@ -342,20 +342,8 @@ sub get
protocolGet(BACKUP)
);

# If the destination file path is not absolute then it is relative to the db data path
if (index($strDestinationFile, '/',) != 0)
{
if (!optionTest(OPTION_DB_PATH))
{
confess &log(
ERROR, "option db-path must be set when relative xlog paths are used\n" .
"HINT: Are \%f and \%p swapped when passed to archive-get?\n" .
'HINT: PostgreSQL generally passes an absolute path to archive-push but in some environments does not, in which' .
" case the 'db-path' option must be specified (this is rare).");
}

$strDestinationFile = optionGet(OPTION_DB_PATH) . "/${strDestinationFile}";
}
# Construct absolute path to the WAL file when it is relative
$strDestinationFile = walPath($strDestinationFile, optionGet(OPTION_DB_PATH, false), commandGet());

# Get the wal segment filename
my $strArchiveId = $self->getCheck($oFile);
Expand Down Expand Up @@ -710,20 +698,8 @@ sub push

lockStopTest();

# If the source file path is not absolute then it is relative to the data path
if (index($strSourceFile, '/') != 0)
{
if (!optionTest(OPTION_DB_PATH))
{
confess &log(
ERROR, "option 'db-path' must be set when relative xlog paths are used\n" .
"HINT: Is \%f passed to archive-push instead of \%p?\n" .
'HINT: PostgreSQL generally passes an absolute path to archive-push but in some environments does not, in which' .
" case the 'db-path' option must be specified (this is rare).");
}

$strSourceFile = optionGet(OPTION_DB_PATH) . "/${strSourceFile}";
}
# Construct absolute path to the WAL file when it is relative
$strSourceFile = walPath($strSourceFile, optionGet(OPTION_DB_PATH, false), commandGet());

# Get the destination file
my $strDestinationFile = basename($strSourceFile);
Expand Down
51 changes: 50 additions & 1 deletion lib/pgBackRest/ArchiveCommon.pm
View file
Expand Up @@ -11,8 +11,9 @@ use Exporter qw(import);
our @EXPORT = qw();

use pgBackRest::DbVersion;
use pgBackRest::Common::Exception;
use pgBackRest::Common::Log;

use pgBackRest::Config::Config;

####################################################################################################################################
# RegEx constants
Expand Down Expand Up @@ -135,4 +136,52 @@ sub lsnFileRange

push @EXPORT, qw(lsnFileRange);

####################################################################################################################################
# walPath
#
# Generates the location of the pg_xlog directory using a relative xlog path and the supplied db path.
####################################################################################################################################
sub walPath
{
# Assign function parameters, defaults, and log debug info
my
(
$strOperation,
$strWalFile,
$strDbPath,
$strCommand,
) =
logDebugParam
(
__PACKAGE__ . 'walPath', \@_,
{name => 'strWalFile', trace => true},
{name => 'strDbPath', trace => true, required => false},
{name => 'strCommand', trace => true},
);

if (index($strWalFile, '/') != 0)
{
if (!defined($strDbPath))
{
confess &log(ERROR,
"option 'db-path' must be specified when relative xlog paths are used\n" .
"HINT: Is \%f passed to ${strCommand} instead of \%p?\n" .
"HINT: PostgreSQL may pass relative paths even with \%p depending on the environment.",
ERROR_OPTION_REQUIRED);
}

$strWalFile = "${strDbPath}/${strWalFile}";
}

# Return from function and log return values if any
return logDebugReturn
(
$strOperation,
{name => 'strWalFile', value => $strWalFile, trace => true}
);

}

push @EXPORT, qw(walPath);

1;
56 changes: 56 additions & 0 deletions test/lib/pgBackRestTest/Archive/ArchiveUnitTest.pm
View file
@@ -0,0 +1,56 @@
####################################################################################################################################
# ArchiveUnitTest.pm - Tests for ArchiveCommon module
####################################################################################################################################
package pgBackRestTest::Archive::ArchiveUnitTest;
use parent 'pgBackRestTest::Full::FullCommonTest';

####################################################################################################################################
# Perl includes
####################################################################################################################################
use strict;
use warnings FATAL => qw(all);
use Carp qw(confess);

use File::Basename qw(dirname);

use pgBackRest::ArchiveCommon;
use pgBackRest::Common::Exception;
use pgBackRest::Common::Log;
use pgBackRest::Config::Config;

####################################################################################################################################
# run
####################################################################################################################################
sub run
{
my $self = shift;

# Increment the run, log, and decide whether this unit test should be run
if (!$self->begin('unit')) {return}

# Unit tests for walPath()
#-----------------------------------------------------------------------------------------------------------------------
{
my $strDbPath = '/db';
my $strWalFileRelative = 'pg_xlog/000000010000000100000001';
my $strWalFileAbsolute = "${strDbPath}/${strWalFileRelative}";

# Error is thrown if the wal file is relative and there is no db path
$self->testException(
sub {walPath($strWalFileRelative, undef, CMD_ARCHIVE_GET)}, ERROR_OPTION_REQUIRED,
"option '" . OPTION_DB_PATH . "' must be specified when relative xlog paths are used\n" .
"HINT: Is \%f passed to " . CMD_ARCHIVE_GET . " instead of \%p?\n" .
"HINT: PostgreSQL may pass relative paths even with \%p depending on the environment.");

# Relative path is contructed
$self->testResult(sub {walPath($strWalFileRelative, $strDbPath, CMD_ARCHIVE_PUSH)}, $strWalFileAbsolute);

# Path is not relative and db-path is still specified
$self->testResult(sub {walPath($strWalFileAbsolute, $strDbPath, CMD_ARCHIVE_PUSH)}, $strWalFileAbsolute);

# Path is not relative and db-path is undef
$self->testResult(sub {walPath($strWalFileAbsolute, $strDbPath, CMD_ARCHIVE_PUSH)}, $strWalFileAbsolute);
}
}

1;
6 changes: 6 additions & 0 deletions test/lib/pgBackRestTest/Common/DefineTest.pm
View file
Expand Up @@ -167,6 +167,12 @@ my $oTestDef =

&TESTDEF_TEST =>
[
{
&TESTDEF_TEST_NAME => 'unit',
&TESTDEF_TEST_TOTAL => 1,
&TESTDEF_TEST_INDIVIDUAL => false,
&TESTDEF_EXPECT => false,
},
{
&TESTDEF_TEST_NAME => 'push',
&TESTDEF_TEST_TOTAL => 8
Expand Down

0 comments on commit 4ff2714

Please sign in to comment.