Repository: netty/netty
Issue: Broken support of SCTP_INIT_MAXSTREAMS in *SctpServerChannel
Labels: defect

Netty version: 4.0.12.Final (but the suspected code is the same in the current 4.0 branch).

The SCTP_INIT_MAXSTREAMS property is ignored on NioSctpServerChannel (and judging by the source on OioSctpServerChannel too).

JUnit test to reproduce:

``` java
import com.sun.nio.sctp.SctpServerChannel;
import com.sun.nio.sctp.SctpStandardSocketOptions;
import io.netty.bootstrap.Bootstrap;
import io.netty.bootstrap.ServerBootstrap;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelOption;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.sctp.SctpChannel;
import io.netty.channel.sctp.SctpChannelOption;
import io.netty.channel.sctp.nio.NioSctpChannel;
import io.netty.channel.sctp.nio.NioSctpServerChannel;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.*;

import java.lang.reflect.Method;
import java.net.InetSocketAddress;

public class SctpLimitStreamsTest {

    @Before
    public void setUp() {
        serverBootstrap = new ServerBootstrap();
        serverBootstrap.group(new NioEventLoopGroup(), new NioEventLoopGroup())
                .channel(NioSctpServerChannel.class)
                .option(ChannelOption.SO_REUSEADDR, true)
                .option(SctpChannelOption.SCTP_INIT_MAXSTREAMS, SctpStandardSocketOptions.InitMaxStreams.create(1, 1))
                .localAddress(new InetSocketAddress("localhost", 7766))
                .childHandler(new ChannelHandler() {
                    @Override
                    public void handlerAdded(ChannelHandlerContext ctx) throws Exception {

                    }

                    @Override
                    public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {

                    }

                    @Override
                    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {

                    }
                });

        clientBootstrap = new Bootstrap()
                .group(new NioEventLoopGroup())
                .channel(NioSctpChannel.class)
                .option(SctpChannelOption.SCTP_INIT_MAXSTREAMS, SctpStandardSocketOptions.InitMaxStreams.create(112, 112))
                .handler(new ChannelHandler() {
                    @Override
                    public void handlerAdded(ChannelHandlerContext ctx) throws Exception {

                    }

                    @Override
                    public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {

                    }

                    @Override
                    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {

                    }
                });

    }

    @After
    public void tearDown() {
        serverBootstrap.childGroup().shutdownGracefully();
        clientBootstrap.group().shutdownGracefully();
        serverBootstrap.group().shutdownGracefully().syncUninterruptibly(); // this sync is enough for this test
    }

    @Test
    public void testSctpInitMaxstreamsNetty() throws Exception {
        NioSctpServerChannel serverChannel = (NioSctpServerChannel) serverBootstrap.bind().syncUninterruptibly().channel();
        serverChannel.config().setOption(SctpChannelOption.SCTP_INIT_MAXSTREAMS, SctpStandardSocketOptions.InitMaxStreams.create(1, 1));

        SctpChannel clientChannel = (SctpChannel) clientBootstrap.connect(new InetSocketAddress("localhost", 7766)).syncUninterruptibly()
                .channel();

        System.out.println("maxOutboundStreams:" + clientChannel.association().maxOutboundStreams());
        System.out.println("maxInboundStreams:" + clientChannel.association().maxInboundStreams());
        assertEquals(1, clientChannel.association().maxOutboundStreams());
        assertEquals(1, clientChannel.association().maxInboundStreams());
    }

    @Test
    public void testSctpInitMaxstreamsNioDirect() throws Exception {
        NioSctpServerChannel serverChannel = (NioSctpServerChannel) serverBootstrap.bind().syncUninterruptibly().channel();

        Method getJavaChannel = serverChannel.getClass().getDeclaredMethod("javaChannel");
        getJavaChannel.setAccessible(true);
        SctpServerChannel javaChannel = (SctpServerChannel) getJavaChannel.invoke(serverChannel);
        javaChannel.setOption(SctpStandardSocketOptions.SCTP_INIT_MAXSTREAMS, SctpStandardSocketOptions.InitMaxStreams.create(1, 1));

        SctpChannel clientChannel = (SctpChannel) clientBootstrap.connect(new InetSocketAddress("localhost", 7766)).syncUninterruptibly()
                .channel();

        System.out.println("maxOutboundStreams:" + clientChannel.association().maxOutboundStreams());
        System.out.println("maxInboundStreams:" + clientChannel.association().maxInboundStreams());
        assertEquals(1, clientChannel.association().maxOutboundStreams());
        assertEquals(1, clientChannel.association().maxInboundStreams());
    }

    private ServerBootstrap serverBootstrap;
    private Bootstrap clientBootstrap;
}

```

The effect is that directly setting the SCTP_INIT_MAXSTREAMS on NIO SctpServerChannel in Testcase testSctpInitMaxstreamsNioDirect **succeeds**, 
while doing it using NETTY SctpChannel/ServerBootstrap in Testcase testSctpInitMaxstreamsNetty **fails**.

```
    [junit] Testcase: testSctpInitMaxstreamsNetty took 2.214 sec
    [junit]     FAILED
    [junit] expected:<1> but was:<112>
    [junit] junit.framework.AssertionFailedError: expected:<1> but was:<112>
    [junit]     at SctpLimitStreamsTest.testSctpInitMaxstreamsNetty(SctpLimitStreamsTest.java:90)
```

It seems that the offending code is located in io.netty.channel.sctp.DefaultSctpServerChannelConfig.setOption(...):

``` java

import static com.sun.nio.sctp.SctpStandardSocketOptions.*; 
//                   /\== note that the static import is "com.sun.nio" 

...

@Override
    public <T> boolean setOption(ChannelOption<T> option, T value) {
        validate(option, value);

        if (option == ChannelOption.SO_RCVBUF) {
            setReceiveBufferSize((Integer) value);
        } else if (option == ChannelOption.SO_SNDBUF) {
            setSendBufferSize((Integer) value);
        } else if (option == SCTP_INIT_MAXSTREAMS) {  
// YOU SHALL NOT PASS...
// as in the preceding conditional we are comparing 
// constant from io.netty.channel.ChannelOption WITH constant from 
// com.sun.nio.sctp.SctpStandardSocketOptions
// (option == SCTP_INIT_MAXSTREAMS) is always FALSE
            setInitMaxStreams((InitMaxStreams) value);
        } else {
            return super.setOption(option, value);
        }

        return true;
    }

```

Environment:
java version "1.7.0_72"
Java(TM) SE Runtime Environment (build 1.7.0_72-b14)
Java HotSpot(TM) 64-Bit Server VM (build 24.72-b04, mixed mode)

Linux lwy-dell 3.13.0-63-generic #103-Ubuntu SMP Fri Aug 14 21:42:59 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux


Issue comments:
Do you want to provide a pr to fix it?

I'm on company time - I'll ask (+day of turnaround).

Supposing that I get an OK to fix/re test.
Do you want to:
- have the DefaultSctpServerChannelConfig brought to match DefaultSctpChannelConfig (where the static import is `import static io.netty.channel.sctp.SctpChannelOption.*;`)
- just add ChannelOption to SCTP_INIT_MAXSTREAMS

?

Yes sounds good

I've never got an answer back about it (working on this issue on company time). 
If I where to guess that means NO.
Sorry.

God damn, so that's what "close and comment" does.
Reopening.

@normanmaurer I will do the fix. In which branch I have to open a PR?

4.1 would be best

> Am 22.01.2016 um 18:37 schrieb Jestan Nirojan notifications@github.com:
> 
> @normanmaurer I will do the fix. In which branch I have to open a PR?
> 
> —
> Reply to this email directly or view it on GitHub.

@jestan did you fix this ?

Fixed by https://github.com/netty/netty/pull/5638


Pull request review comments:
nit: newline

fixed


Patch that resolved the issue:
diff --git a/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpChannelConfig.java b/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpChannelConfig.java
index 932e09ca3a9..6b284992e81 100644
--- a/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpChannelConfig.java
+++ b/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpChannelConfig.java
@@ -74,6 +74,9 @@ public <T> T getOption(ChannelOption<T> option) {
         if (option == SCTP_NODELAY) {
             return (T) Boolean.valueOf(isSctpNoDelay());
         }
+        if (option == SCTP_INIT_MAXSTREAMS) {
+            return (T) getInitMaxStreams();
+        }
         return super.getOption(option);
     }
 
diff --git a/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpServerChannelConfig.java b/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpServerChannelConfig.java
index efad6eb722a..2c2e59afa64 100644
--- a/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpServerChannelConfig.java
+++ b/transport-sctp/src/main/java/io/netty/channel/sctp/DefaultSctpServerChannelConfig.java
@@ -16,6 +16,7 @@
 package io.netty.channel.sctp;
 
 import com.sun.nio.sctp.SctpServerChannel;
+import com.sun.nio.sctp.SctpStandardSocketOptions;
 import io.netty.buffer.ByteBufAllocator;
 import io.netty.channel.ChannelException;
 import io.netty.channel.ChannelOption;
@@ -27,8 +28,6 @@
 import java.io.IOException;
 import java.util.Map;
 
-import static com.sun.nio.sctp.SctpStandardSocketOptions.*;
-
 /**
  * The default {@link SctpServerChannelConfig} implementation for SCTP.
  */
@@ -65,6 +64,9 @@ public <T> T getOption(ChannelOption<T> option) {
         if (option == ChannelOption.SO_SNDBUF) {
             return (T) Integer.valueOf(getSendBufferSize());
         }
+        if (option == SctpChannelOption.SCTP_INIT_MAXSTREAMS) {
+            return (T) getInitMaxStreams();
+        }
         return super.getOption(option);
     }
 
@@ -76,8 +78,8 @@ public <T> boolean setOption(ChannelOption<T> option, T value) {
             setReceiveBufferSize((Integer) value);
         } else if (option == ChannelOption.SO_SNDBUF) {
             setSendBufferSize((Integer) value);
-        } else if (option == SCTP_INIT_MAXSTREAMS) {
-            setInitMaxStreams((InitMaxStreams) value);
+        } else if (option == SctpChannelOption.SCTP_INIT_MAXSTREAMS) {
+            setInitMaxStreams((SctpStandardSocketOptions.InitMaxStreams) value);
         } else {
             return super.setOption(option, value);
         }
@@ -88,7 +90,7 @@ public <T> boolean setOption(ChannelOption<T> option, T value) {
     @Override
     public int getSendBufferSize() {
         try {
-            return javaChannel.getOption(SO_SNDBUF);
+            return javaChannel.getOption(SctpStandardSocketOptions.SO_SNDBUF);
         } catch (IOException e) {
             throw new ChannelException(e);
         }
@@ -97,7 +99,7 @@ public int getSendBufferSize() {
     @Override
     public SctpServerChannelConfig setSendBufferSize(int sendBufferSize) {
         try {
-            javaChannel.setOption(SO_SNDBUF, sendBufferSize);
+            javaChannel.setOption(SctpStandardSocketOptions.SO_SNDBUF, sendBufferSize);
         } catch (IOException e) {
             throw new ChannelException(e);
         }
@@ -107,7 +109,7 @@ public SctpServerChannelConfig setSendBufferSize(int sendBufferSize) {
     @Override
     public int getReceiveBufferSize() {
         try {
-            return javaChannel.getOption(SO_RCVBUF);
+            return javaChannel.getOption(SctpStandardSocketOptions.SO_RCVBUF);
         } catch (IOException e) {
             throw new ChannelException(e);
         }
@@ -116,7 +118,7 @@ public int getReceiveBufferSize() {
     @Override
     public SctpServerChannelConfig setReceiveBufferSize(int receiveBufferSize) {
         try {
-            javaChannel.setOption(SO_RCVBUF, receiveBufferSize);
+            javaChannel.setOption(SctpStandardSocketOptions.SO_RCVBUF, receiveBufferSize);
         } catch (IOException e) {
             throw new ChannelException(e);
         }
@@ -124,18 +126,18 @@ public SctpServerChannelConfig setReceiveBufferSize(int receiveBufferSize) {
     }
 
     @Override
-    public InitMaxStreams getInitMaxStreams() {
+    public SctpStandardSocketOptions.InitMaxStreams getInitMaxStreams() {
         try {
-            return javaChannel.getOption(SCTP_INIT_MAXSTREAMS);
+            return javaChannel.getOption(SctpStandardSocketOptions.SCTP_INIT_MAXSTREAMS);
         } catch (IOException e) {
             throw new ChannelException(e);
         }
     }
 
     @Override
-    public SctpServerChannelConfig setInitMaxStreams(InitMaxStreams initMaxStreams) {
+    public SctpServerChannelConfig setInitMaxStreams(SctpStandardSocketOptions.InitMaxStreams initMaxStreams) {
         try {
-            javaChannel.setOption(SCTP_INIT_MAXSTREAMS, initMaxStreams);
+            javaChannel.setOption(SctpStandardSocketOptions.SCTP_INIT_MAXSTREAMS, initMaxStreams);
         } catch (IOException e) {
             throw new ChannelException(e);
         }


Test patch:
diff --git a/transport-sctp/src/main/test/io/netty/channel/sctp/SctpLimitStreamsTest.java b/transport-sctp/src/main/test/io/netty/channel/sctp/SctpLimitStreamsTest.java
new file mode 100644
index 00000000000..3d5abf4c344
--- /dev/null
+++ b/transport-sctp/src/main/test/io/netty/channel/sctp/SctpLimitStreamsTest.java
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel.sctp;
+
+import com.sun.nio.sctp.SctpStandardSocketOptions;
+import io.netty.bootstrap.Bootstrap;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.ChannelOption;
+import io.netty.channel.EventLoopGroup;
+import org.junit.Test;
+import java.net.InetSocketAddress;
+
+import static org.junit.Assert.*;
+
+public abstract class SctpLimitStreamsTest {
+
+    @Test(timeout = 5000)
+    public void testSctpInitMaxstreams() throws Exception {
+        EventLoopGroup loop = newEventLoopGroup();
+        try {
+            ServerBootstrap serverBootstrap = new ServerBootstrap();
+            serverBootstrap.group(loop)
+                    .channel(serverClass())
+                    .option(ChannelOption.SO_REUSEADDR, true)
+                    .option(SctpChannelOption.SCTP_INIT_MAXSTREAMS,
+                            SctpStandardSocketOptions.InitMaxStreams.create(1, 1))
+                    .localAddress(new InetSocketAddress(0))
+                    .childHandler(new ChannelInboundHandlerAdapter());
+
+            Bootstrap clientBootstrap = new Bootstrap()
+                    .group(loop)
+                    .channel(clientClass())
+                    .option(SctpChannelOption.SCTP_INIT_MAXSTREAMS,
+                            SctpStandardSocketOptions.InitMaxStreams.create(112, 112))
+                    .handler(new ChannelInboundHandlerAdapter());
+
+            Channel serverChannel = serverBootstrap.bind()
+                    .syncUninterruptibly().channel();
+            SctpChannel clientChannel = (SctpChannel) clientBootstrap.connect(serverChannel.localAddress())
+                    .syncUninterruptibly().channel();
+            assertEquals(1, clientChannel.association().maxOutboundStreams());
+            assertEquals(1, clientChannel.association().maxInboundStreams());
+            serverChannel.close().syncUninterruptibly();
+            clientChannel.close().syncUninterruptibly();
+        } finally {
+            loop.shutdownGracefully();
+        }
+    }
+
+    protected abstract EventLoopGroup newEventLoopGroup();
+    protected abstract Class<? extends SctpChannel> clientClass();
+    protected abstract Class<? extends SctpServerChannel> serverClass();
+}
diff --git a/transport-sctp/src/main/test/io/netty/channel/sctp/nio/NioSctpLimitStreamsTest.java b/transport-sctp/src/main/test/io/netty/channel/sctp/nio/NioSctpLimitStreamsTest.java
new file mode 100644
index 00000000000..78365ab5072
--- /dev/null
+++ b/transport-sctp/src/main/test/io/netty/channel/sctp/nio/NioSctpLimitStreamsTest.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel.sctp.nio;
+
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.sctp.SctpChannel;
+import io.netty.channel.sctp.SctpLimitStreamsTest;
+import io.netty.channel.sctp.SctpServerChannel;
+
+public class NioSctpLimitStreamsTest extends SctpLimitStreamsTest {
+    @Override
+    protected EventLoopGroup newEventLoopGroup() {
+        return new NioEventLoopGroup();
+    }
+
+    @Override
+    protected Class<? extends SctpChannel> clientClass() {
+        return NioSctpChannel.class;
+    }
+
+    @Override
+    protected Class<? extends SctpServerChannel> serverClass() {
+        return NioSctpServerChannel.class;
+    }
+}
diff --git a/transport-sctp/src/main/test/io/netty/channel/sctp/oio/OioSctpLimitStreamsTest.java b/transport-sctp/src/main/test/io/netty/channel/sctp/oio/OioSctpLimitStreamsTest.java
new file mode 100644
index 00000000000..d30a97d0ba2
--- /dev/null
+++ b/transport-sctp/src/main/test/io/netty/channel/sctp/oio/OioSctpLimitStreamsTest.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel.sctp.oio;
+
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.oio.OioEventLoopGroup;
+import io.netty.channel.sctp.SctpChannel;
+import io.netty.channel.sctp.SctpLimitStreamsTest;
+import io.netty.channel.sctp.SctpServerChannel;
+
+public class OioSctpLimitStreamsTest extends SctpLimitStreamsTest {
+    @Override
+    protected EventLoopGroup newEventLoopGroup() {
+        return new OioEventLoopGroup();
+    }
+
+    @Override
+    protected Class<? extends SctpChannel> clientClass() {
+        return OioSctpChannel.class;
+    }
+
+    @Override
+    protected Class<? extends SctpServerChannel> serverClass() {
+        return OioSctpServerChannel.class;
+    }
+}