Skip to content

Commit

Permalink
Handle large properly values in manual indexes better
Browse files Browse the repository at this point in the history
* At the moment if you try to add a manual index with a very large property value
  you'll get an overflow exception because the value gets included in the response
  header and exceeds the value configured with Jetty.
* This commit makes that value configurable and fails early if it looks like the
  response header size is going to exceed the configured value. We give 512 bytes
  for any other headers so you might need to set the value a bit higher than you want.
  • Loading branch information
Mark Needham committed Mar 30, 2015
1 parent abf94a0 commit 3af617d
Show file tree
Hide file tree
Showing 17 changed files with 279 additions and 29 deletions.
Expand Up @@ -440,9 +440,9 @@ private void configureWebServer()

private int getMaxThreads()
{
return configurator.configuration()
.get( ServerSettings.webserver_max_threads ) != null ? configurator.configuration()
.get( ServerSettings.webserver_max_threads ) : defaultMaxWebServerThreads();
return configurator.configuration().get( ServerSettings.webserver_max_threads ) != null ?
configurator.configuration().get( ServerSettings.webserver_max_threads ) :
defaultMaxWebServerThreads();
}

private int defaultMaxWebServerThreads()
Expand Down
Expand Up @@ -119,7 +119,7 @@ protected Iterable<ServerModule> createServerModules()
@Override
protected WebServer createWebServer()
{
return new Jetty9WebServer( dependencies.logging());
return new Jetty9WebServer( dependencies.logging(), configurator.configuration());
}

@Override
Expand Down
Expand Up @@ -49,6 +49,14 @@
public interface ServerSettings
{

@Description( "Maximum request header size" )
public static final Setting<Integer> maximum_request_header_size =
setting( "org.neo4j.server.webserver.max.request.header", INTEGER, "20480" );

@Description( "Maximum response header size" )
public static final Setting<Integer> maximum_response_header_size =
setting( "org.neo4j.server.webserver.max.response.header", INTEGER, "20480" );

@Description( "Comma-seperated list of custom security rules for Neo4j to use." )
public static final Setting<List<String>> security_rules = setting( "org.neo4j.server.rest.security_rules",
STRING_LIST, EMPTY );
Expand Down
Expand Up @@ -865,6 +865,8 @@ protected Representation underlyingObjectToObject( Relationship rel )
return new ListRepresentation( RepresentationType.RELATIONSHIP, results );
}



public Pair<IndexedEntityRepresentation, Boolean> getOrCreateIndexedNode(
String indexName, String key, String value, Long nodeOrNull, Map<String, Object> properties )
throws BadInputException, NodeNotFoundException
Expand Down
Expand Up @@ -19,6 +19,8 @@
*/
package org.neo4j.server.rest.web;

import org.apache.commons.configuration.Configuration;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
Expand Down Expand Up @@ -47,6 +49,7 @@
import org.neo4j.graphdb.NotFoundException;
import org.neo4j.helpers.Function;
import org.neo4j.helpers.Pair;
import org.neo4j.server.configuration.ServerSettings;
import org.neo4j.server.rest.domain.EndNodeNotFoundException;
import org.neo4j.server.rest.domain.EvaluationException;
import org.neo4j.server.rest.domain.PropertySettingStrategy;
Expand Down Expand Up @@ -155,6 +158,7 @@ public AmpersandSeparatedCollection( String path )
private static final String HEADER_TRANSACTION = "Transaction";

private final DatabaseActions actions;
private Configuration config;
private final OutputFormat output;
private final InputFormat input;

Expand All @@ -169,11 +173,14 @@ private enum UniqueIndexType
}

public RestfulGraphDatabase( @Context InputFormat input,
@Context OutputFormat output, @Context DatabaseActions actions )
@Context OutputFormat output,
@Context DatabaseActions actions,
@Context Configuration config )
{
this.input = input;
this.output = output;
this.actions = actions;
this.config = config;
}

public OutputFormat getOutputFormat()
Expand Down Expand Up @@ -974,6 +981,9 @@ public Response deleteRelationshipIndex( @PathParam("indexName") String indexNam
public Response addToNodeIndex( @PathParam("indexName") String indexName, @QueryParam("unique") String unique,
@QueryParam("uniqueness") String uniqueness, String postBody )
{
int otherHeaders = 512;
int maximumSizeInBytes = config.getInt( ServerSettings.maximum_response_header_size.name() ) - otherHeaders;

try
{
Map<String, Object> entityBody;
Expand All @@ -983,18 +993,32 @@ public Response addToNodeIndex( @PathParam("indexName") String indexName, @Query
{
case GetOrCreate:
entityBody = input.readMap( postBody, "key", "value" );

String getOrCreateValue = String.valueOf( entityBody.get( "value" ) );
if ( getOrCreateValue.length() > maximumSizeInBytes )
{
return valueTooBig();
}

result = actions.getOrCreateIndexedNode( indexName,
String.valueOf( entityBody.get( "key" ) ),
String.valueOf( entityBody.get( "value" ) ), extractNodeIdOrNull( getStringOrNull(
getOrCreateValue, extractNodeIdOrNull( getStringOrNull(
entityBody, "uri" ) ), getMapOrNull( entityBody, "properties" ) );
return result.other() ? output.created( result.first() ) : output.okIncludeLocation( result.first
() );

case CreateOrFail:
entityBody = input.readMap( postBody, "key", "value" );

String createOrFailValue = String.valueOf( entityBody.get( "value" ) );
if ( createOrFailValue.length() > maximumSizeInBytes )
{
return valueTooBig();
}

result = actions.getOrCreateIndexedNode( indexName,
String.valueOf( entityBody.get( "key" ) ),
String.valueOf( entityBody.get( "value" ) ), extractNodeIdOrNull( getStringOrNull(
createOrFailValue, extractNodeIdOrNull( getStringOrNull(
entityBody, "uri" ) ), getMapOrNull( entityBody, "properties" ) );
if ( result.other() )
{
Expand All @@ -1020,9 +1044,16 @@ public Response addToNodeIndex( @PathParam("indexName") String indexName, @Query

default:
entityBody = input.readMap( postBody, "key", "value", "uri" );
String value = String.valueOf( entityBody.get( "value" ) );

if ( value.length() > maximumSizeInBytes )
{
return valueTooBig();
}

return output.created( actions.addToNodeIndex( indexName,
String.valueOf( entityBody.get( "key" ) ),
String.valueOf( entityBody.get( "value" ) ), extractNodeId( entityBody.get( "uri" )
value, extractNodeId( entityBody.get( "uri" )
.toString() ) ) );

}
Expand All @@ -1045,6 +1076,15 @@ public Response addToNodeIndex( @PathParam("indexName") String indexName, @Query
}
}

private Response valueTooBig()
{
return Response.status( 413 ).entity( String.format(
"The property value provided was too large. The maximum size is currently set to %d bytes. " +
"You can configure this by setting the '%s' property.",
config.getInt(ServerSettings.maximum_response_header_size.name()),
ServerSettings.maximum_response_header_size.name() ) ).build();
}

@POST
@Path(PATH_NAMED_RELATIONSHIP_INDEX)
public Response addToRelationshipIndex( @PathParam("indexName") String indexName,
Expand Down
Expand Up @@ -23,12 +23,22 @@
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.server.*;
import org.eclipse.jetty.util.ssl.SslContextFactory;

import org.neo4j.kernel.configuration.Config;
import org.neo4j.server.web.HttpConnectorFactory;
import org.neo4j.server.web.JettyThreadCalculator;


public class SslSocketConnectorFactory extends HttpConnectorFactory
{
private Config configuration;

public SslSocketConnectorFactory( Config configuration )
{
super(configuration);
this.configuration = configuration;
}

@Override
protected HttpConfiguration createHttpConfig()
{
Expand Down
Expand Up @@ -27,8 +27,19 @@
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;

import org.neo4j.kernel.configuration.Config;
import org.neo4j.server.configuration.ServerSettings;

public class HttpConnectorFactory
{
private Config configuration;

public HttpConnectorFactory( Config config )
{

this.configuration = config;
}

public ConnectionFactory createHttpConnectionFactory()
{
return new HttpConnectionFactory( createHttpConfig() );
Expand All @@ -37,8 +48,8 @@ public ConnectionFactory createHttpConnectionFactory()
protected HttpConfiguration createHttpConfig()
{
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.setRequestHeaderSize( 20 * 1024 );
httpConfig.setResponseHeaderSize( 20 * 1024 );
httpConfig.setRequestHeaderSize( configuration.get( ServerSettings.maximum_request_header_size) );
httpConfig.setResponseHeaderSize( configuration.get( ServerSettings.maximum_response_header_size) );
return httpConfig;
}

Expand Down
Expand Up @@ -59,6 +59,8 @@
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.eclipse.jetty.webapp.WebAppContext;

import org.neo4j.kernel.configuration.Config;

import org.neo4j.kernel.impl.util.StringLogger;
import org.neo4j.kernel.logging.ConsoleLogger;
import org.neo4j.kernel.logging.Logging;
Expand Down Expand Up @@ -126,16 +128,18 @@ public String getPathSpec()
private int jettyMaxThreads;
private boolean httpsEnabled = false;
private KeyStoreInformation httpsCertificateInformation = null;
private final SslSocketConnectorFactory sslSocketFactory = new SslSocketConnectorFactory();
private final HttpConnectorFactory connectorFactory = new HttpConnectorFactory();
private final SslSocketConnectorFactory sslSocketFactory;
private final HttpConnectorFactory connectorFactory;
private File requestLoggingConfiguration;
private final ConsoleLogger console;
private final StringLogger log;

public Jetty9WebServer( Logging logging )
public Jetty9WebServer( Logging logging, Config config )
{
this.console = logging.getConsoleLog( getClass() );
this.log = logging.getMessagesLog( getClass() );
sslSocketFactory = new SslSocketConnectorFactory(config);
connectorFactory = new HttpConnectorFactory(config);
}

@Override
Expand Down
124 changes: 124 additions & 0 deletions community/server/src/test/java/org/neo4j/server/LegacyIndexIT.java
@@ -0,0 +1,124 @@
/**
* Copyright (c) 2002-2015 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.server;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.theories.DataPoints;
import org.junit.experimental.theories.Theories;
import org.junit.experimental.theories.Theory;
import org.junit.runner.RunWith;

import java.io.IOException;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.Random;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;

import org.neo4j.server.configuration.ServerSettings;
import org.neo4j.test.server.ExclusiveServerTestBase;
import org.neo4j.test.server.HTTP;

import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
import static org.neo4j.server.helpers.CommunityServerBuilder.server;
import static org.neo4j.test.server.HTTP.GET;
import static org.neo4j.test.server.HTTP.POST;
import static org.neo4j.test.server.HTTP.RawPayload.quotedJson;

@RunWith(Theories.class)
public class LegacyIndexIT extends ExclusiveServerTestBase
{
private CommunityNeoServer server;

public static @DataPoints String[] candidates = {"", "get_or_create", "create_or_fail"};

@After
public void stopTheServer()
{
server.stop();
}

@Before
public void startServer() throws NoSuchAlgorithmException, KeyManagementException, IOException
{
server = server().withHttpsEnabled()
.withProperty( "remote_shell_enabled", "false" )
.withProperty( "dbms.security.auth_enabled", "false" )
.usingDatabaseDir( folder.cleanDirectory( name.getMethodName() ).getAbsolutePath() )
.build();
}

@Theory
public void shouldRejectIndexValueLargerThanConfiguredSize(String uniqueness) throws Exception
{
//Given
server.getConfiguration().setProperty( ServerSettings.maximum_response_header_size.name(), "5000" );
server.start();

// When
String nodeURI = HTTP.POST( server.baseUri().toString() + "db/data/node").header( "Location" );

Random r = new Random();
String value = "";
for ( int i = 0; i < 6_000; i++ )
{
value += (char)(r.nextInt(26) + 'a');
}
HTTP.Response response =
HTTP.POST( server.baseUri().toString() + "db/data/index/node/favorites?uniqueness=" + uniqueness,
quotedJson( "{ 'value': '" + value + " ', 'uri':'" + nodeURI + "', 'key': 'some-key' }" ) );

// Then
assertThat( response.status(), is( 413 ) );
}

@Theory
public void shouldNotRejectIndexValueThatIsJustSmallerThanConfiguredSize(String uniqueness) throws Exception
{
//Given
server.getConfiguration().setProperty( ServerSettings.maximum_response_header_size.name(), "5000" );
server.start();

// When
String nodeURI = HTTP.POST( server.baseUri().toString() + "db/data/node").header( "Location" );

Random r = new Random();
String value = "";
for ( int i = 0; i < 4_000; i++ )
{
value += (char)(r.nextInt(26) + 'a');
}
HTTP.Response response =
HTTP.POST( server.baseUri().toString() + "db/data/index/node/favorites?uniqueness=" + uniqueness,
quotedJson( "{ 'value': '" + value + " ', 'uri':'" + nodeURI + "', 'key': 'some-key' }" ) );

// Then
assertThat( response.status(), is( 201 ) );
}
}

0 comments on commit 3af617d

Please sign in to comment.