-1

I have a simple class that handles socket connections:

public class SimpleConnection {

    // Socket, input and output streams
    protected Socket mSocket;

    protected DataInputStream mIn;

    protected DataOutputStream mOut;

    public boolean createConnection(String ip, int port) {
        SocketAddress socketAddress = new InetSocketAddress(ip, port);
        mSocket = new Socket();
        try {
            mSocket.connect(socketAddress, 3000);
            mIn = new DataInputStream(mSocket.getInputStream());
            mOut = new DataOutputStream(mSocket.getOutputStream());
        } catch (IOException e) {
            return false;
        }

        return true;
    }

    public boolean sendData(byte[] data) {
        try {
            mOut.writeInt(data.length);
            mOut.write(data);
            mOut.flush();
        } catch (Exception e) {
            e.printStackTrace();
            closeSocket();
            return false;
        }
        return true;
    }
}

This has worked until Android N. With Android N, mOut.writeInt(data.length) just sends four zeros instead of the length of data.length. This causes the server to misinterpreted the message and for the whole program to not work.

I was able to "fix" the problem by converting the integer to a byte[4]:

byte[] len = Utilities.intToByteArray(data.length);
mOut.write(len);

intToByteArray is shown here.

My question is: Why isn't writeInt not working anymore on Android N? On other Android versions this code runs just fine.

I use the latest Android Studio with Java 8, gradle 2.1.3 and Android buildtools 24.0.2.

Edit: The receiving part looks like this in Qt:

void readData(QTcpSocket* client_) {
  while (client_->bytesAvailable()) {
    int expected_length_;
    QDataStream s(client_);
    s >> expected_length_;
    qLog(Debug) << expected_length_;

    // Read data with expected_length_
    QBuffer buffer_;
    buffer_.write(client_->read(expected_length_));
  }
}

expected_length_ is 0 where with the fix it is 15. Interestingly, client_->bytesAvailable() is 1 with the writeInt variant on Android N.

I did another test using nc -p 1234 -l 0.0.0.0 | xxd:

▶ nc -p 1234 -l 0.0.0.0 | xxd
00000000: 0000 000f 0815 1001 aa01 0808 8edb 0110  ................
00000010: 0118 00                                  ...

This is the output for both variants... so it seems writeInt() works as expected, but why does it work for Android <= 6 and not for Android 7?!??

Edit2:

After analyzing the traffic I found out that the integer is split in multiple TCP frames. I changed the server code to check if client_->bytesAvailable() >= 4 and only then to read the integer from the socket. This fixed the problem and the writeInt() variant works now too.

But why did the behaviour suddenly change?

Community
  • 1
  • 1
amuttsch
  • 1,254
  • 14
  • 24
  • And you can guarantee that the length of the array wasn't in fact `0` then? Claiming that an internal method is broken is quite an accusation. – Kayaman Sep 11 '16 at 19:30
  • Yes, `data.length` was `15` and not `0`. – amuttsch Sep 11 '16 at 19:37
  • This is impossible to believe. If this method was broken half the JDK wouldn't work. Show your receiving code. – user207421 Sep 11 '16 at 22:16
  • I know exactly zip about Qt but surely `s >> expected_length` does an ASCII conversion? and you still haven't shown the part where you actually read exactly that many bytes, which is where most code of this nature falls down. In Java you would need to use `DataInputStream.readFully()`, or the corresponding loop: otherwise you will get out of sync with the sender. – user207421 Sep 12 '16 at 08:33
  • I edited the server side code. `expected_length_` is 0, therefore the `buffer_` has no data. The thing is, the server code and the client code runs well for months and years. Only with Android N, it suddenly stopped working. Has Android N flushed the data before sending the full four bytes for the int? – amuttsch Sep 12 '16 at 08:45
  • If it hadn't flushed the data you would have blocked. Again I know diddly-squat about Qt but most `read()` methods aren't guaranteed to fill the buffer, `readFully()` being a major exception. Unless you know that Qt's `read()` method is another exception you've been sitting on this bug for years. – user207421 Sep 12 '16 at 09:45
  • After analyzing the traffic I found out that `writeInt()` flushes the data prematurely. So some frames have only one or two bytes, that's why `bytesAvailable` is only `1`. The server code `QDataStream` reads the int with only one or two byte. After adding a `if (client_->bytesAvailable() < 4) break;` and waiting for more data it works. But I still don't understand why the behaviour changed. – amuttsch Sep 12 '16 at 10:04

1 Answers1

3

After analyzing the traffic I found out that writeInt() flushes the data

It did exactly what you told it to do. DataOutputStream is not buffered, and there is no BufferedOutputStream under it, so writeInt() wrote four bytes to the network.

prematurely.

There is no such thing as 'prematurely' in TCP. TCP makes no guarantees about packetization or segmentation. If you want to control this so-called 'premature flush', use mOut = new DataOutputStream(new BufferedOutputStream(Socket.getOutputStream())); and flush it yourself after writing the data.

So some frames have only one or two bytes, that's why bytesAvailable is only 1.

  • This was your main problem. You are misusing available(). It isn't an end of message indicator. See the Javadoc.
  • You also didn't check to see that you had actually read the four bytes of the length word.
  • As far as I can see you aren't checking to see that you've received all the bytes of data either.

In general you have to loop until you get everything that is expected.

The server code QDataStream reads the int with only one or two byte. After adding if (client_->bytesAvailable() < 4) break; and waiting for more data it works. But I still don't understand why the behaviour changed.

It can change any time. Your code broke because it relied on several invalid assumptions.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • Thank you for your answer, I now understand what I did wrong. I changed `mOut` to add a `BufferedOutputStream` as you suggested as well as checking the lengths properly in the server code. – amuttsch Sep 13 '16 at 09:26
  • Woaah thankss a tonne for the answer! Even I was facing the same problem (the app was working on Marshmallow n not on Nougat). This was really hepful! – Kartik Shenoy Jul 06 '18 at 08:15