You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by rg...@apache.org on 2015/06/11 20:09:20 UTC
svn commit: r1684956 - in /zookeeper/trunk: CHANGES.txt
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java
Author: rgs
Date: Thu Jun 11 18:09:19 2015
New Revision: 1684956
URL: http://svn.apache.org/r1684956
Log:
ZOOKEEPER-2213: Empty path in Set crashes server and prevents restart
(Hongchao Deng via rgs)
Modified:
zookeeper/trunk/CHANGES.txt
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
zookeeper/trunk/src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java
Modified: zookeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1684956&r1=1684955&r2=1684956&view=diff
==============================================================================
--- zookeeper/trunk/CHANGES.txt (original)
+++ zookeeper/trunk/CHANGES.txt Thu Jun 11 18:09:19 2015
@@ -125,6 +125,9 @@ BUGFIXES:
ZOOKEEPER-2201: Network issues can cause cluster to hang due to near-deadlock
(Donny Nadolny via rgs)
+ ZOOKEEPER-2213: Empty path in Set crashes server and prevents restart
+ (Hongchao Deng via rgs)
+
IMPROVEMENTS:
ZOOKEEPER-1660 Documentation for Dynamic Reconfiguration (Reed Wanderman-Milne via shralex)
Modified: zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java?rev=1684956&r1=1684955&r2=1684956&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java (original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java Thu Jun 11 18:09:19 2015
@@ -384,13 +384,7 @@ public class PrepRequestProcessor extend
if (createMode.isSequential()) {
path = path + String.format(Locale.ENGLISH, "%010d", parentCVersion);
}
- try {
- PathUtils.validatePath(path);
- } catch(IllegalArgumentException ie) {
- LOG.info("Invalid path " + path + " with session 0x" +
- Long.toHexString(request.sessionId));
- throw new KeeperException.BadArgumentsException(path);
- }
+ validatePath(path, request.sessionId);
try {
if (getRecordForPath(path) != null) {
throw new KeeperException.NodeExistsException(path);
@@ -465,6 +459,7 @@ public class PrepRequestProcessor extend
if(deserialize)
ByteBufferInputStream.byteBuffer2Record(request.request, setDataRequest);
path = setDataRequest.getPath();
+ validatePath(path, request.sessionId);
nodeRecord = getRecordForPath(path);
checkACL(zks, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo);
int newVersion = checkAndIncVersion(nodeRecord.stat.getVersion(), setDataRequest.getVersion(), path);
@@ -595,6 +590,7 @@ public class PrepRequestProcessor extend
if(deserialize)
ByteBufferInputStream.byteBuffer2Record(request.request, setAclRequest);
path = setAclRequest.getPath();
+ validatePath(path, request.sessionId);
List<ACL> listACL = fixupACL(path, request.authInfo, setAclRequest.getAcl());
nodeRecord = getRecordForPath(path);
checkACL(zks, nodeRecord.acl, ZooDefs.Perms.ADMIN, request.authInfo);
@@ -650,6 +646,7 @@ public class PrepRequestProcessor extend
if(deserialize)
ByteBufferInputStream.byteBuffer2Record(request.request, checkVersionRequest);
path = checkVersionRequest.getPath();
+ validatePath(path, request.sessionId);
nodeRecord = getRecordForPath(path);
checkACL(zks, nodeRecord.acl, ZooDefs.Perms.READ, request.authInfo);
request.setTxn(new CheckVersionTxn(path, checkAndIncVersion(nodeRecord.stat.getVersion(),
@@ -661,6 +658,16 @@ public class PrepRequestProcessor extend
}
}
+ private void validatePath(String path, long sessionId) throws BadArgumentsException {
+ try {
+ PathUtils.validatePath(path);
+ } catch(IllegalArgumentException ie) {
+ LOG.info("Invalid path {} with session 0x{}, reason: {}",
+ path, Long.toHexString(sessionId), ie.getMessage());
+ throw new BadArgumentsException(path);
+ }
+ }
+
private String getParentPathAndValidate(String path)
throws BadArgumentsException {
int lastSlash = path.lastIndexOf('/');
Modified: zookeeper/trunk/src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java
URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java?rev=1684956&r1=1684955&r2=1684956&view=diff
==============================================================================
--- zookeeper/trunk/src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java (original)
+++ zookeeper/trunk/src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java Thu Jun 11 18:09:19 2015
@@ -29,6 +29,7 @@ import org.apache.zookeeper.PortAssignme
import org.apache.zookeeper.ZooDefs.Ids;
import org.apache.zookeeper.ZooDefs.OpCode;
import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.proto.SetDataRequest;
import org.apache.zookeeper.server.ZooKeeperServer.ChangeRecord;
import org.apache.zookeeper.test.ClientBase;
import org.apache.zookeeper.txn.ErrorTxn;
@@ -50,6 +51,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
public class PrepRequestProcessorTest extends ClientBase {
private static final Logger LOG = LoggerFactory.getLogger(PrepRequestProcessorTest.class);
@@ -96,34 +98,29 @@ public class PrepRequestProcessorTest ex
Assert.assertEquals("Request should have marshalling error", new ErrorTxn(KeeperException.Code.MARSHALLINGERROR.intValue()),
outcome.getTxn());
- Assert.assertTrue("request hasn't been processed in chain",
- pLatch.await(5, java.util.concurrent.TimeUnit.SECONDS));
+ Assert.assertTrue("request hasn't been processed in chain", pLatch.await(5, TimeUnit.SECONDS));
}
- private Request createMultiRequest(List<Op> ops) throws IOException {
- Record record = new MultiTransactionRecord(ops);
-
+ private Request createRequest(Record record, int opCode) throws IOException {
// encoding
ByteArrayOutputStream baos = new ByteArrayOutputStream();
BinaryOutputArchive boa = BinaryOutputArchive.getArchive(baos);
record.serialize(boa, "request");
baos.close();
-
// Id
List<Id> ids = Arrays.asList(Ids.ANYONE_ID_UNSAFE);
-
- return new Request(null, 1l, 0, OpCode.multi, ByteBuffer.wrap(baos.toByteArray()), ids);
+ return new Request(null, 1l, 0, opCode, ByteBuffer.wrap(baos.toByteArray()), ids);
}
private void process(List<Op> ops) throws Exception {
pLatch = new CountDownLatch(1);
processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
- Request req = createMultiRequest(ops);
+ Record record = new MultiTransactionRecord(ops);
+ Request req = createRequest(record, OpCode.multi);
processor.pRequest(req);
- Assert.assertTrue("request hasn't been processed in chain",
- pLatch.await(5, java.util.concurrent.TimeUnit.SECONDS));
+ Assert.assertTrue("request hasn't been processed in chain", pLatch.await(5, TimeUnit.SECONDS));
}
/**
@@ -171,9 +168,6 @@ public class PrepRequestProcessorTest ex
zks.getZKDatabase().dataTree.createNode("/foo", new byte[0], Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
zks.getZKDatabase().dataTree.createNode("/foo/bar", new byte[0], Ids.OPEN_ACL_UNSAFE, 0, 0, 0, 0);
- pLatch = new CountDownLatch(1);
- processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
-
Assert.assertNull(zks.outstandingChangesForPath.get("/foo"));
// multi record:
@@ -187,6 +181,23 @@ public class PrepRequestProcessorTest ex
Assert.assertNull(zks.outstandingChangesForPath.get("/foo"));
}
+ /**
+ * It tests that PrepRequestProcessor will return BadArgument KeeperException
+ * if the request path (if it exists) is not valid, e.g. empty string.
+ */
+ @Test
+ public void testInvalidPath() throws Exception {
+ pLatch = new CountDownLatch(1);
+ processor = new PrepRequestProcessor(zks, new MyRequestProcessor());
+
+ SetDataRequest record = new SetDataRequest("", new byte[0], -1);
+ Request req = createRequest(record, OpCode.setData);
+ processor.pRequest(req);
+ pLatch.await();
+ Assert.assertEquals(outcome.getHdr().getType(), OpCode.error);
+ Assert.assertEquals(outcome.getException().code(), KeeperException.Code.BADARGUMENTS);
+ }
+
private class MyRequestProcessor implements RequestProcessor {
@Override
public void processRequest(Request request) {