Github user swill commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1493#discussion_r62207167
  
    --- Diff: utils/src/test/java/com/cloud/utils/testcase/NioTest.java ---
    @@ -19,146 +19,208 @@
     
     package com.cloud.utils.testcase;
     
    -import java.nio.channels.ClosedChannelException;
    -import java.util.Random;
    -
    -import junit.framework.TestCase;
    -
    -import org.apache.log4j.Logger;
    -import org.junit.Assert;
    -
    +import com.cloud.utils.concurrency.NamedThreadFactory;
     import com.cloud.utils.exception.NioConnectionException;
     import com.cloud.utils.nio.HandlerFactory;
     import com.cloud.utils.nio.Link;
     import com.cloud.utils.nio.NioClient;
     import com.cloud.utils.nio.NioServer;
     import com.cloud.utils.nio.Task;
     import com.cloud.utils.nio.Task.Type;
    +import org.apache.log4j.Logger;
    +import org.junit.After;
    +import org.junit.Assert;
    +import org.junit.Before;
    +import org.junit.Test;
    +
    +import java.io.IOException;
    +import java.net.InetSocketAddress;
    +import java.nio.channels.ClosedChannelException;
    +import java.nio.channels.Selector;
    +import java.nio.channels.SocketChannel;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Random;
    +import java.util.concurrent.ExecutorService;
    +import java.util.concurrent.Executors;
     
     /**
    - *
    - *
    - *
    - *
    + * NioTest demonstrates that NioServer can function without getting its 
main IO
    + * loop blocked when an aggressive or malicious client connects to the 
server but
    + * fail to participate in SSL handshake. In this test, we run bunch of 
clients
    + * that send a known payload to the server, to which multiple malicious 
clients
    + * also try to connect and hang.
    + * A malicious client could cause denial-of-service if the server's main 
IO loop
    + * along with SSL handshake was blocking. A passing tests shows that 
NioServer
    + * can still function in case of connection load and that the main IO loop 
along
    + * with SSL handshake is non-blocking with some internal timeout mechanism.
      */
     
    -public class NioTest extends TestCase {
    +public class NioTest {
    +
    +    private static final Logger LOGGER = Logger.getLogger(NioTest.class);
    +
    +    // Test should fail in due time instead of looping forever
    +    private static final int TESTTIMEOUT = 300000;
     
    -    private static final Logger s_logger = Logger.getLogger(NioTest.class);
    +    final private int totalTestCount = 5;
    +    private int completedTestCount = 0;
     
    -    private NioServer _server;
    -    private NioClient _client;
    +    private NioServer server;
    +    private List<NioClient> clients = new ArrayList<>();
    +    private List<NioClient> maliciousClients = new ArrayList<>();
     
    -    private Link _clientLink;
    +    private ExecutorService clientExecutor = 
Executors.newFixedThreadPool(totalTestCount, new 
NamedThreadFactory("NioClientHandler"));;
    +    private ExecutorService maliciousExecutor = 
Executors.newFixedThreadPool(5*totalTestCount, new 
NamedThreadFactory("MaliciousNioClientHandler"));;
     
    -    private int _testCount;
    -    private int _completedCount;
    +    private Random randomGenerator = new Random();
    +    private byte[] testBytes;
     
         private boolean isTestsDone() {
             boolean result;
             synchronized (this) {
    -            result = _testCount == _completedCount;
    +            result = totalTestCount == completedTestCount;
    --- End diff --
    
    Isn't this wrong?  
    
    Shouldn't it be: 
    ```
    result = (totalTestCount -1) == completedTestCount;
    ```
    
    You are are only launching `totalTestCount` tests `0 to totalTestCount-1`.  
`completedTestCount` is also `0` based, so when they all complete it should max 
out at `totalTestCount-1`.
    
    Can you clarify?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to