Skip to content

Commit

Permalink
More helpful error message when procedure loading fails due to sandbo…
Browse files Browse the repository at this point in the history 
…xing
  • Loading branch information
@chrisvest
chrisvest committed Jul 10, 2017
1 parent 9bb465b commit 1d5ae24
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 43 deletions.
24 changes: 15 additions & 9 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@

import static java.util.Collections.emptyIterator;
import static java.util.Collections.emptyList;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.procedure_unrestricted;
import static org.neo4j.helpers.collection.Iterators.asRawIterator;

/**
Expand Down Expand Up @@ -266,9 +267,7 @@ private CallableProcedure compileProcedure( Class<?> procDefinition, MethodHandl
}
catch ( ComponentInjectionException e )
{
description = Optional.of( procName.toString() +
" is not available due to having restricted access rights, check configuration." );
log.warn( description.get() );
description = describeAndLogLoadFailure( procName );
ProcedureSignature signature =
new ProcedureSignature( procName, inputSignature, outputMapper.signature(), Mode.DEFAULT,
Optional.empty(), new String[0], description, warning );
Expand All @@ -282,6 +281,17 @@ private CallableProcedure compileProcedure( Class<?> procDefinition, MethodHandl
return new ReflectiveProcedure( signature, constructor, procedureMethod, outputMapper, setters );
}

private Optional<String> describeAndLogLoadFailure( QualifiedName name )
{
String nameStr = name.toString();
Optional<String> description = Optional.of(
nameStr + " is unavailable because it is sandboxed and has dependencies outside of the sandbox. " +
"Sandboxing is controlled by the " + procedure_unrestricted.name() + " setting. " +
"Only unrestrict procedures you can trust with access to database internals." );
log.warn( description.get() );
return description;
}

private CallableUserFunction compileFunction( Class<?> procDefinition, MethodHandle constructor, Method method,
QualifiedName procName )
throws ProcedureException, IllegalAccessException
Expand Down Expand Up @@ -312,9 +322,7 @@ private CallableUserFunction compileFunction( Class<?> procDefinition, MethodHan
}
catch ( ComponentInjectionException e )
{
description = Optional.of( procName.toString() +
" is not available due to having restricted access rights, check configuration." );
log.warn( description.get() );
description = describeAndLogLoadFailure( procName );
UserFunctionSignature signature =
new UserFunctionSignature( procName, inputSignature, valueConverter.type(), deprecated,
config.rolesFor( procName.toString() ), description );
Expand Down Expand Up @@ -427,9 +435,7 @@ private CallableUserAggregationFunction compileAggregationFunction( Class<?> def
}
catch ( ComponentInjectionException e )
{
description = Optional.of( funcName.toString() +
" is not available due to having restricted access rights, check configuration." );
log.warn( description.get() );
description = describeAndLogLoadFailure( funcName );
UserFunctionSignature signature =
new UserFunctionSignature( funcName, inputSignature, valueConverter.type(), deprecated,
config.rolesFor( funcName.toString() ), description );
Expand Down
11 changes: 4 additions & 7 deletions community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProcedureJarLoaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.procedure_unrestricted;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.procedure_unrestricted;
import static org.neo4j.helpers.collection.Iterators.asList;
import static org.neo4j.helpers.collection.MapUtil.genericMap;
import static org.neo4j.kernel.api.proc.Neo4jTypes.NTInteger;
import static org.neo4j.kernel.api.proc.ProcedureSignature.procedureSignature;
import static org.neo4j.kernel.api.proc.UserFunctionSignature.functionSignature;
import static org.neo4j.kernel.impl.proc.ResourceInjectionTest.notAvailableMessage;

@SuppressWarnings( "WeakerAccess" )
public class ProcedureJarLoaderTest
Expand Down Expand Up @@ -223,12 +224,8 @@ public void shouldGiveHelpfulLogOnUnsafeRestrictedProcedure() throws Throwable
jarloader.loadProcedures( jar );

// Then
verify( log )
.warn( "org.neo4j.kernel.impl.proc.unsafeProcedure is not available " +
"due to having restricted access rights, check configuration." );
verify( log )
.warn( "org.neo4j.kernel.impl.proc.unsafeFunction" +
" is not available due to having restricted access rights, check configuration." );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.unsafeProcedure" ) );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.unsafeFunction" ) );
}

@Test
Expand Down
50 changes: 23 additions & 27 deletions community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ResourceInjectionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.kernel.impl.proc;

import org.hamcrest.Matcher;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -46,8 +47,10 @@
import static java.util.Collections.singletonList;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.Matchers.allOf;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.argThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

Expand All @@ -61,6 +64,16 @@ public class ResourceInjectionTest

private Log log = mock(Log.class);

public static String notAvailableMessage( String procName )
{
return argThat( notAvailableMessageMatcher( procName ) );
}

private static Matcher<String> notAvailableMessageMatcher( String procName )
{
return allOf( containsString( procName ), containsString( "unavailable" ) );
}

@Before
public void setUp()
{
Expand Down Expand Up @@ -125,9 +138,7 @@ public void shouldFailNicelyWhenUnsafeAPISafeMode() throws Throwable
//When
List<CallableProcedure> procList =
compiler.compileProcedure( ProcedureWithUnsafeAPI.class, Optional.empty(), false );
verify( log )
.warn( "org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.listCoolPeople" ) );

assertThat( procList.size(), equalTo( 1 ) );
try
Expand All @@ -137,9 +148,7 @@ public void shouldFailNicelyWhenUnsafeAPISafeMode() throws Throwable
}
catch ( ProcedureException e )
{
assertThat( e.getMessage(), containsString(
"org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." ) );
assertThat( e.getMessage(), notAvailableMessageMatcher( "org.neo4j.kernel.impl.proc.listCoolPeople" ) );
}
}

Expand Down Expand Up @@ -176,9 +185,7 @@ public void shouldFailNicelyWhenUnsafeAPISafeModeFunction() throws Throwable
//When
List<CallableUserFunction> procList =
compiler.compileFunction( FunctionWithUnsafeAPI.class);
verify( log )
.warn( "org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.listCoolPeople" ) );

assertThat( procList.size(), equalTo( 1 ) );
try
Expand All @@ -188,9 +195,7 @@ public void shouldFailNicelyWhenUnsafeAPISafeModeFunction() throws Throwable
}
catch ( ProcedureException e )
{
assertThat( e.getMessage(), containsString(
"org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." ) );
assertThat( e.getMessage(), notAvailableMessageMatcher( "org.neo4j.kernel.impl.proc.listCoolPeople" ) );
}
}

Expand Down Expand Up @@ -227,9 +232,7 @@ public void shouldFailNicelyWhenUnsafeAPISafeModeAggregationFunction() throws Th
//When
List<CallableUserAggregationFunction> procList =
compiler.compileAggregationFunction( AggregationFunctionWithUnsafeAPI.class);
verify( log )
.warn( "org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.listCoolPeople" ) );

assertThat( procList.size(), equalTo( 1 ) );
try
Expand All @@ -240,9 +243,7 @@ public void shouldFailNicelyWhenUnsafeAPISafeModeAggregationFunction() throws Th
}
catch ( ProcedureException e )
{
assertThat( e.getMessage(), containsString(
"org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." ) );
assertThat( e.getMessage(), notAvailableMessageMatcher( "org.neo4j.kernel.impl.proc.listCoolPeople" ) );
}
}

Expand All @@ -255,15 +256,10 @@ public void shouldFailNicelyWhenAllUsesUnsafeAPI() throws Throwable
compiler.compileAggregationFunction( FunctionsAndProcedureUnsafe.class );
// Then

verify( log )
.warn( "org.neo4j.kernel.impl.proc.safeUserFunctionInUnsafeAPIClass is not " +
"available due to having restricted access rights, check configuration." );
verify( log )
.warn( "org.neo4j.kernel.impl.proc.listCoolPeopleProcedure is not " +
"available due to having restricted access rights, check configuration." );
verify( log )
.warn( "org.neo4j.kernel.impl.proc.listCoolPeople is not " +
"available due to having restricted access rights, check configuration." );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.safeUserFunctionInUnsafeAPIClass" ) );
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.listCoolPeopleProcedure" ) );
// With extra ' ' space at the end to distinguish from procedure form:
verify( log ).warn( notAvailableMessage( "org.neo4j.kernel.impl.proc.listCoolPeople " ) );
}

public static class MyOutputRecord
Expand Down

0 comments on commit 1d5ae24

Please sign in to comment.