Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove a weird special case in path traversal #20

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Bread/Board/GraphViz.pm
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ has 'visitor' => (

sub service_name {
my $service = shift;
return '' unless $service;
return '/' . $service->name
if $service->parent == $service->get_root_container;
return join '/', service_name($service->parent), $service->name;
}

Expand Down
12 changes: 10 additions & 2 deletions lib/Bread/Board/Traversable.pm
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,18 @@ sub _get_container_or_service {
# there must be a better way to do this

if ($c->does('Bread::Board::Service')) {
return $c if $c->name eq $name;
if ($c->name eq $name) {
warn "Traversing into the current service ($name) is deprecated."
. " You should remove the $name component from the path.";
return $c;
}
}
elsif ($c->isa('Bread::Board::Container')) {
return $c if $c->name eq $name;
if ($c->name eq $name) {
warn "Traversing into the current container ($name) is deprecated;"
. " you should remove the $name component from the path";
return $c;
}
return $c->get_sub_container($name) if $c->has_sub_container($name);
return $c->get_service($name) if $c->has_service($name);
}
Expand Down
8 changes: 4 additions & 4 deletions t/011_container_path.t
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ my $c = Bread::Board::Container->new(
#use Bread::Board::Dumper;
#diag(Bread::Board::Dumper->new->dump($c));

my $model = $c->fetch('Application/Model');
my $model = $c->fetch('Model');
isa_ok($model, 'Bread::Board::Container');

is($model->name, 'Model', '... got the right model');

{
my $model2 = $c->fetch('/Application/Model');
my $model2 = $c->fetch('/Model');
isa_ok($model2, 'Bread::Board::Container');

is($model, $model2, '... they are the same thing');
Expand All @@ -65,7 +65,7 @@ isa_ok($dsn, 'Bread::Board::Dependency');
is($dsn->service_path, 'dsn', '... got the right name');

{
my $dsn2 = $c->fetch('/Application/Model/schema/dsn');
my $dsn2 = $c->fetch('/Model/schema/dsn');
isa_ok($dsn2, 'Bread::Board::Dependency');

is($dsn, $dsn2, '... they are the same thing');
Expand All @@ -76,7 +76,7 @@ isa_ok($root, 'Bread::Board::Container');

is($root, $c, '... got the same container');

is($model, $model->fetch('../Application/Model'), '... navigated back to myself');
is($model, $model->fetch('../Model'), '... navigated back to myself');
is($dsn, $model->fetch('../Model/schema/dsn'), '... navigated to dsn');

is($model, $dsn->fetch('../Model'), '... got the model from the dsn');
Expand Down
8 changes: 4 additions & 4 deletions t/025_sugar_w_absolute_path.t
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ my $c = container 'Application' => as {
container 'Controller';
};

my $model = $c->fetch('Application/Model');
my $model = $c->fetch('Model');
isa_ok($model, 'Bread::Board::Container');


is($model->name, 'Model', '... got the right model');

{
my $model2 = $c->fetch('/Application/Model');
my $model2 = $c->fetch('/Model');
isa_ok($model2, 'Bread::Board::Container');

is($model, $model2, '... they are the same thing');
Expand All @@ -50,7 +50,7 @@ isa_ok($dsn, 'Bread::Board::Dependency');
is($dsn->service_path, 'dsn', '... got the right name');

{
my $dsn2 = $c->fetch('/Application/Model/schema/dsn');
my $dsn2 = $c->fetch('/Model/schema/dsn');
isa_ok($dsn2, 'Bread::Board::Dependency');

is($dsn, $dsn2, '... they are the same thing');
Expand All @@ -61,7 +61,7 @@ isa_ok($root, 'Bread::Board::Container');

is($root, $c, '... got the same container');

is($model, $model->fetch('../Application/Model'), '... navigated back to myself');
is($model, $model->fetch('../Model'), '... navigated back to myself');
is($dsn, $model->fetch('../Model/schema/dsn'), '... navigated to dsn');

is($model, $dsn->fetch('../Model'), '... got the model from the dsn');
Expand Down
6 changes: 3 additions & 3 deletions t/052_parameterized_in_hierarchy.t
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ my $db_conn_info = container 'DatabaseConnection' => as {
};
isa_ok($db_conn_info, 'Bread::Board::Container');

my $db = $utils->fetch('Utils/Database');
my $db = $utils->fetch('Database');
isa_ok($db, 'Bread::Board::Container::Parameterized');

isnt(exception {
$utils->fetch('Utils/Database')->fetch('handle');
$utils->fetch('Database')->fetch('handle');
}, undef, '... cannot fetch on a parameterized container');

isnt(exception {
$utils->fetch('Utils/Database/handle');
$utils->fetch('Database/handle');
}, undef, '... cannot fetch within a parameterized container');

my $dbh = $db->create( DBConnInfo => $db_conn_info )->resolve( service => 'handle' );
Expand Down
14 changes: 7 additions & 7 deletions t/100_clone_w_constructor_injection.t
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ $board->add_service(
name => 'test',
class => 'Test::Class',
dependencies => {
dep => Bread::Board::Dependency->new(service_path => '/app/dep'),
dep => Bread::Board::Dependency->new(service_path => '/dep'),
},
)
);
Expand Down Expand Up @@ -60,12 +60,12 @@ isnt($board->get_service('dep'), $board2->get_service('dep'), '... not the same

# test them ...

is($board->fetch('/app/dep')->get(), 1, '... got correct dep');
is($board->fetch('/app/test')->get()->dep, 1, '... test uses dep');
is(refaddr $board->fetch('/app/test')->parent, refaddr $board, '... got the right board');
is($board->fetch('/dep')->get(), 1, '... got correct dep');
is($board->fetch('/test')->get()->dep, 1, '... test uses dep');
is(refaddr $board->fetch('/test')->parent, refaddr $board, '... got the right board');

is($board2->fetch('/app/dep')->get(), 2, '... got correct dep');
is($board2->fetch('/app/test')->get()->dep, 2, '... test uses dep');
is(refaddr $board2->fetch('/app/test')->parent, refaddr $board2, '... got the right board');
is($board2->fetch('/dep')->get(), 2, '... got correct dep');
is($board2->fetch('/test')->get()->dep, 2, '... test uses dep');
is(refaddr $board2->fetch('/test')->parent, refaddr $board2, '... got the right board');

done_testing;
14 changes: 7 additions & 7 deletions t/101_clone_w_setter_injection.t
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ $board->add_service(
name => 'test',
class => 'Test::Class',
dependencies => {
dep => Bread::Board::Dependency->new(service_path => '/app/dep'),
dep => Bread::Board::Dependency->new(service_path => '/dep'),
},
)
);
Expand Down Expand Up @@ -60,12 +60,12 @@ isnt($board->get_service('dep'), $board2->get_service('dep'), '... not the same

# test them ...

is($board->fetch('/app/dep')->get(), 1, '... got correct dep');
is($board->fetch('/app/test')->get()->dep, 1, '... test uses dep');
is(refaddr $board->fetch('/app/test')->parent, refaddr $board, '... got the right board');
is($board->fetch('/dep')->get(), 1, '... got correct dep');
is($board->fetch('/test')->get()->dep, 1, '... test uses dep');
is(refaddr $board->fetch('/test')->parent, refaddr $board, '... got the right board');

is($board2->fetch('/app/dep')->get(), 2, '... got correct dep');
is($board2->fetch('/app/test')->get()->dep, 2, '... test uses dep');
is(refaddr $board2->fetch('/app/test')->parent, refaddr $board2, '... got the right board');
is($board2->fetch('/dep')->get(), 2, '... got correct dep');
is($board2->fetch('/test')->get()->dep, 2, '... test uses dep');
is(refaddr $board2->fetch('/test')->parent, refaddr $board2, '... got the right board');

done_testing;
14 changes: 7 additions & 7 deletions t/102_clone_w_block_injection.t
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ $board->add_service(
Test::Class->new(%{ $s->params });
},
dependencies => {
dep => Bread::Board::Dependency->new(service_path => '/app/dep'),
dep => Bread::Board::Dependency->new(service_path => '/dep'),
},
)
);
Expand Down Expand Up @@ -64,12 +64,12 @@ isnt($board->get_service('dep'), $board2->get_service('dep'), '... not the same

# test them ...

is($board->fetch('/app/dep')->get(), 1, '... got correct dep');
is($board->fetch('/app/test')->get()->dep, 1, '... test uses dep');
is(refaddr $board->fetch('/app/test')->parent, refaddr $board, '... got the right board');
is($board->fetch('/dep')->get(), 1, '... got correct dep');
is($board->fetch('/test')->get()->dep, 1, '... test uses dep');
is(refaddr $board->fetch('/test')->parent, refaddr $board, '... got the right board');

is($board2->fetch('/app/dep')->get(), 2, '... got correct dep');
is($board2->fetch('/app/test')->get()->dep, 2, '... test uses dep');
is(refaddr $board2->fetch('/app/test')->parent, refaddr $board2, '... got the right board');
is($board2->fetch('/dep')->get(), 2, '... got correct dep');
is($board2->fetch('/test')->get()->dep, 2, '... test uses dep');
is(refaddr $board2->fetch('/test')->parent, refaddr $board2, '... got the right board');

done_testing;
8 changes: 4 additions & 4 deletions t/110_clone_w_singleton.t
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ $board->add_service(
name => 'test',
class => 'Test::Class',
dependencies => {
dep => Bread::Board::Dependency->new(service_path => '/app/dep'),
dep => Bread::Board::Dependency->new(service_path => '/dep'),
},
)
);
Expand All @@ -38,7 +38,7 @@ isa_ok($board->get_service('dep'), 'Bread::Board::Literal');

## check the singleton-ness

is($board->fetch('/app/test')->get, $board->fetch('/app/test')->get, '... got the singleton');
is($board->fetch('/test')->get, $board->fetch('/test')->get, '... got the singleton');

# clone ...

Expand All @@ -57,10 +57,10 @@ isnt($board->get_service('dep'), $board2->get_service('dep'), '... not the same

## check the singleton-ness

is($board2->fetch('/app/test')->get, $board2->fetch('/app/test')->get, '... got the singleton');
is($board2->fetch('/test')->get, $board2->fetch('/test')->get, '... got the singleton');

## check the singleton-less-ness

isnt($board->fetch('/app/test')->get, $board2->fetch('/app/test')->get, '... singleton are not shared');
isnt($board->fetch('/test')->get, $board2->fetch('/test')->get, '... singleton are not shared');

done_testing;
2 changes: 1 addition & 1 deletion t/201_log_dispatch_example.t
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ my $c = container 'Logging' => as {
};
};

my $logger = $c->resolve( service => 'Logging/Logger' );
my $logger = $c->resolve( service => 'Logger' );
isa_ok($logger, 'Log::Dispatch');

my $screen = $logger->output('screen');
Expand Down
21 changes: 21 additions & 0 deletions t/302_path_traversal_deprecation.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/env perl
use strict;
use warnings;
use Test::More;

use Bread::Board;

my $c = container 'Foo' => as {
service bar => 'baz';
};

{
my $warning;
local $SIG{__WARN__} = sub { $warning = $_[0] };

my $baz = $c->resolve(service => '/Foo/bar');
is($baz, 'baz');
like($warning, qr/Traversing into the current container \(Foo\) is deprecated; you should remove the Foo component from the path/);
}

done_testing;
18 changes: 9 additions & 9 deletions t/500_graphviz.t
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ sub cmp_edges {
}

is_deeply [ sort cmp_edges map { [$_->{from}, $_->{to}] } $g->edges ], [
['/MyApp/config/config_file' => '/MyApp/name' ],
['/MyApp/config/dsn' => '/MyApp/config/config_file'],
['/MyApp/config/dsn' => '/MyApp/logger'],
['/MyApp/database' => '/MyApp/config/dsn'],
['/MyApp/database' => '/MyApp/logger'],
['/MyApp/pages/login' => '/MyApp/database'],
['/MyApp/pages/login' => '/MyApp/logger'],
['/MyApp/pages/login' => '/MyApp/templates/login'],
['/MyApp/templates/login' => '/MyApp/config/template_dir'],
['/config/config_file' => '/name' ],
['/config/dsn' => '/config/config_file'],
['/config/dsn' => '/logger'],
['/database' => '/config/dsn'],
['/database' => '/logger'],
['/pages/login' => '/database'],
['/pages/login' => '/logger'],
['/pages/login' => '/templates/login'],
['/templates/login' => '/config/template_dir'],
], 'added all the edges';

done_testing;
12 changes: 6 additions & 6 deletions t/lib/graphable.bb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ container 'MyApp' => as {

container 'config' => as {
service 'config_file' => (
dependencies => [ depends_on('/MyApp/name') ],
dependencies => [ depends_on('/name') ],
lifecycle => 'Singleton',
block => sub {},
);
Expand All @@ -27,7 +27,7 @@ container 'MyApp' => as {

service 'dsn' => (
lifecycle => 'Singleton',
dependencies => [ depends_on('/MyApp/logger'), depends_on('config_file') ],
dependencies => [ depends_on('/logger'), depends_on('config_file') ],
block => sub {},
);
};
Expand All @@ -42,7 +42,7 @@ container 'MyApp' => as {

container 'templates' => as {
service 'login' => (
dependencies => [ depends_on('/MyApp/config/template_dir') ],
dependencies => [ depends_on('/config/template_dir') ],
class => 'Template',
block => sub {},
);
Expand All @@ -52,9 +52,9 @@ container 'MyApp' => as {
service 'login' => (
class => 'Page::Login',
dependencies => [
depends_on('/MyApp/templates/login'),
depends_on('/MyApp/database'),
depends_on('/MyApp/logger'),
depends_on('/templates/login'),
depends_on('/database'),
depends_on('/logger'),
],
);
};
Expand Down