Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: how to use MessageToMessage encoder on UDP pipeline #9199

Closed
ntherrien opened this issue May 29, 2019 · 11 comments · Fixed by #9204
Closed

Question: how to use MessageToMessage encoder on UDP pipeline #9199

ntherrien opened this issue May 29, 2019 · 11 comments · Fixed by #9204

Comments

@ntherrien
Copy link

ntherrien commented May 29, 2019

Expected behavior

I thought I could encode SIP Messages on UDP the same way I had done for the Decoder:
public class SipUdpClientInitializer extends ChannelInitializer {

@Override
public void initChannel(DatagramChannel ch) throws Exception {
    ChannelPipeline pipeline = ch.pipeline();

    // and then business logic.
    pipeline.addLast(new DatagramPacketEncoder<>(new SipMessageEncoder()));

}

}

Actual behavior

Decoder operations work, but not Encoder. I get:
java.lang.UnsupportedOperationException: unsupported message type: DefaultAddressedEnvelope (expected: DatagramPacket, AddressedEnvelope<ByteBuf, SocketAddress>, ByteBuf)
at io.netty.channel.socket.nio.NioDatagramChannel.filterOutboundMessage(NioDatagramChannel.java:332)

Debugging netty code shows that it expects a ByteBuf in the Envelope, not a SIPMessage and thus fails with the above message due to if statement not having an else.
see NioDatagramChannel.filterOutboundMessage line 322

        if (e.content() instanceof ByteBuf) {

I was expecting the SIPMessage object to be passed to the Encoder that I've provided so it can be converted to a ByteBuf. The encoder never gets called.

Steps to reproduce

Minimal yet complete reproducer code (or URL to code)

public class SipUdpClient {

private Logger LOGGER = LoggerFactory.getLogger(SipUdpClient.class);
private final EventLoopGroup group;
private final Bootstrap b;

public SipUdpClient() {
    group = new NioEventLoopGroup();

    b = new Bootstrap();
    b.group(group)
            .channel(NioDatagramChannel.class)
            .handler(new SipUdpClientInitializer());

}

public void send(InetSocketAddress socketAddress, SIPMessage sipMessage) throws InterruptedException {

    LOGGER.info("Sending to " + socketAddress.getAddress().getHostAddress() + ":" + socketAddress.getPort() + " via UDP");

    final Channel channel = b.connect(socketAddress).sync().channel();
    try {
        channel.writeAndFlush(new DefaultAddressedEnvelope<>(sipMessage, socketAddress)).sync();
    }
    finally {
        channel.close().sync();
    }

}

public void close() {
    group.shutdownGracefully();
}

}

public class SipMessageEncoder extends MessageToMessageEncoder<AddressedEnvelope<SIPMessage, InetSocketAddress>> {

private static Logger logger = LoggerFactory.getLogger(SipMessageEncoder.class);

@Override
protected void encode(ChannelHandlerContext ctx, AddressedEnvelope<SIPMessage, InetSocketAddress> msg, List<Object> out) throws Exception {
    final byte[] udps = msg.content().encodeAsBytes("UDP");
    final ByteBuf byteBuf = Unpooled.wrappedBuffer(udps);
    logger.info("\n---OUTGOING MESSAGE---\n" + new String(udps) + "\n+---END OF OUTGOING MESSAGE---");
    out.add(new DatagramPacket(byteBuf, msg.recipient()));
}

}

Netty version

4.1.35

JVM version (e.g. java -version)

11.0.2

OS version (e.g. uname -a)

Windows 10 1809

@normanmaurer
Copy link
Member

normanmaurer commented May 30, 2019

@ntherrien please show your SipMessageEncoder

@ntherrien
Copy link
Author

@normanmaurer done. Added SipMessageEncoder to question.

@qeesung
Copy link
Contributor

qeesung commented May 31, 2019

@ntherrien Maybe you should add sender information to DatagramPacket, otherwise the message will be filtered out by DatagramPacketEncoder.

@Override
protected void encode(ChannelHandlerContext ctx, AddressedEnvelope<SIPMessage, InetSocketAddress> msg, List<Object> out) throws Exception {
    final byte[] udps = msg.content().encodeAsBytes("UDP");
    final ByteBuf byteBuf = Unpooled.wrappedBuffer(udps);
    logger.info("\n---OUTGOING MESSAGE---\n" + new String(udps) + "\n+---END OF OUTGOING MESSAGE---");
    out.add(new DatagramPacket(byteBuf, msg.recipient(), msg.sender()));
}

@ntherrien
Copy link
Author

@queesung I forgot to indicate that the encode method is never called by the pipeline. The NioDatagramChannel is getting the Envelope object from the application code sending a message out. It seems like the NioDatagramChannel does not support encoders on the pipeline.

The Netty Examples have only one example using UDP and that example does not have any encoder in the pipeline.

That being said, I will try to see if setting a sender in the application code results in the encoder being called.

@ntherrien
Copy link
Author

BTW: when i said "application code" I really meant the SipUdpClient's send method.

@qeesung
Copy link
Contributor

qeesung commented May 31, 2019

@ntherrien Because the message is filtered out by DatagramPacketEncoder, so the encode method is never called by the pipeline. You can check the source code of DatagramPacketEncoder and you will find:

image

@normanmaurer
Copy link
Member

@ntherrien as @ntherrien said... that said I think it should be valid to have a have a null sender. Let me do a PR.

@ntherrien
Copy link
Author

@qeesung thanks for the tip. It gave me an investigation path...

I just dont know how I can obtain a sender address... I am writing client code and wanted to send the UDP packet from an ephemeral port as such:
new DatagramPacket(byteBuf, recipient)

The sender part has always been optional when writing udp code by hand. Why does Netty require a sender? What address am I going to put in there?

Any idea how to fill in this part? Should I put a socket address of 0.0.0.0 to indicate I expect Netty to use any available port?

@normanmaurer
Copy link
Member

@ntherrien yeah just put "something in there", it does not matter.. In generally it should support null and I will fix this.

normanmaurer added a commit that referenced this issue May 31, 2019
Motivation:

It is valid to use null as sender so we should support it when DatagramPacketEncoder checks if it supports the message.

Modifications:

- Add null check
- Add unit test

Result:

Fixes #9199.
@normanmaurer
Copy link
Member

See #9204

@ntherrien
Copy link
Author

I can confirm it now works with the write instruction set to a dummy sender socketaddress:
channel.writeAndFlush(new DefaultAddressedEnvelope<>(sipMessage, socketAddress, new InetSocketAddress(0))).sync();

Encoder gets called and everything else!

Thanks much for your support in this matter, I appreciate it and must say i really love the Netty pipeline system. Makes it easy to break down networking code in its constituents!

Accepting null sender would be a nice improvement as it will more closely match the signature of a DatagramPacket which also treats the sender address as optional.

normanmaurer added a commit that referenced this issue Jun 3, 2019
Motivation:

It is valid to use null as sender so we should support it when DatagramPacketEncoder checks if it supports the message.

Modifications:

- Add null check
- Add unit test

Result:

Fixes #9199.
normanmaurer added a commit that referenced this issue Jun 3, 2019
Motivation:

It is valid to use null as sender so we should support it when DatagramPacketEncoder checks if it supports the message.

Modifications:

- Add null check
- Add unit test

Result:

Fixes #9199.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants