Skip to content

Commit 264280c

Browse files
committed
Drop non-functional TCP window size checks entirely
Previously we checked the window size, but never incremented or decremented the count of unacked data, so it was a) confusing and b) open to the risk that something would increment the count, thereby blocking the connection entirely. Now, we just ignore it, effectively using an infinite window size. If the device is overloaded, we assume our upstream connections will be applying their own backpressure anyway. Seems to have worked fine so far, might be worth investigating if we do see issues on some devices under load.
1 parent 98e3583 commit 264280c

File tree

3 files changed

+22
-70
lines changed

3 files changed

+22
-70
lines changed

app/src/main/java/tech/httptoolkit/android/vpn/Session.java

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,6 @@ public class Session {
5656

5757
//the next ack to send to client
5858
private long sendNext = 0;
59-
private int sendWindow = 0; //window = windowsize x windowscale
60-
private int sendWindowSize = 0;
61-
private int sendWindowScale = 0;
62-
63-
//track how many byte of data has been sent since last ACK from client
64-
private volatile int sendAmountSinceLastAck = 0;
6559

6660
//sent by client during SYN inside tcp options
6761
private int maxSegmentSize = 0;
@@ -130,27 +124,6 @@ public class Session {
130124
this.sessionCloser = sessionCloser;
131125
}
132126

133-
/**
134-
* decrease value of sendAmountSinceLastAck so that client's window is not full
135-
* @param amount Amount
136-
*/
137-
synchronized void decreaseAmountSentSinceLastAck(long amount){
138-
sendAmountSinceLastAck -= amount;
139-
if(sendAmountSinceLastAck < 0){
140-
Log.e(TAG, "Amount data to be decreased is over than its window.");
141-
sendAmountSinceLastAck = 0;
142-
}
143-
}
144-
145-
/**
146-
* determine if client's receiving window is full or not.
147-
* @return boolean
148-
*/
149-
public boolean isClientWindowFull(){
150-
return (sendWindow > 0 && sendAmountSinceLastAck >= sendWindow) ||
151-
(sendWindow == 0 && sendAmountSinceLastAck > 65535);
152-
}
153-
154127
/**
155128
* append more data
156129
* @param data Data
@@ -244,10 +217,6 @@ public void setSendNext(long sendNext) {
244217
this.sendNext = sendNext;
245218
}
246219

247-
int getSendWindow() {
248-
return sendWindow;
249-
}
250-
251220
public int getMaxSegmentSize() {
252221
return maxSegmentSize;
253222
}
@@ -272,16 +241,6 @@ public int getSourcePort() {
272241
return sourcePort;
273242
}
274243

275-
void setSendWindowSizeAndScale(int sendWindowSize, int sendWindowScale) {
276-
this.sendWindowSize = sendWindowSize;
277-
this.sendWindowScale = sendWindowScale;
278-
this.sendWindow = sendWindowSize * sendWindowScale;
279-
}
280-
281-
int getSendWindowScale() {
282-
return sendWindowScale;
283-
}
284-
285244
void setAcked(boolean isacked) {
286245
this.isacked = isacked;
287246
}

app/src/main/java/tech/httptoolkit/android/vpn/SessionHandler.java

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,7 @@ private void handleTCPPacket(ByteBuffer clientPacketData, IPv4Header ipHeader) t
139139
int destinationPort = tcpheader.getDestinationPort();
140140

141141
if(tcpheader.isSYN()) {
142-
//3-way handshake + create new session
143-
//set windows size and scale, set reply time in options
142+
// 3-way handshake + create new session
144143
replySynAck(ipHeader,tcpheader);
145144
} else if(tcpheader.isACK()) {
146145
String key = Session.getSessionKey(destinationIP, destinationPort, sourceIP, sourcePort);
@@ -196,7 +195,7 @@ private void handleTCPPacket(ByteBuffer clientPacketData, IPv4Header ipHeader) t
196195
resetConnection(ipHeader, tcpheader);
197196
}
198197

199-
if (!session.isClientWindowFull() && !session.isAbortingConnection()) {
198+
if (!session.isAbortingConnection()) {
200199
manager.keepSessionAlive(session);
201200
}
202201
}
@@ -323,23 +322,24 @@ private void sendAckForDisorder(IPv4Header ipHeader, TCPHeader tcpheader, int ac
323322
}
324323

325324
/**
326-
* acknowledge a packet and adjust the receiving window to avoid congestion.
325+
* acknowledge a packet.
327326
* @param tcpHeader TCP Header
328327
* @param session Session
329328
*/
330329
private void acceptAck(TCPHeader tcpHeader, Session session){
331330
boolean isCorrupted = PacketUtil.isPacketCorrupted(tcpHeader);
331+
332332
session.setPacketCorrupted(isCorrupted);
333-
if(isCorrupted){
333+
if (isCorrupted) {
334334
Log.e(TAG,"prev packet was corrupted, last ack# " + tcpHeader.getAckNumber());
335335
}
336-
if(tcpHeader.getAckNumber() > session.getSendUnack() ||
337-
tcpHeader.getAckNumber() == session.getSendNext()){
336+
337+
if (
338+
tcpHeader.getAckNumber() > session.getSendUnack() ||
339+
tcpHeader.getAckNumber() == session.getSendNext()
340+
) {
338341
session.setAcked(true);
339342

340-
if(tcpHeader.getWindowSize() > 0){
341-
session.setSendWindowSizeAndScale(tcpHeader.getWindowSize(), session.getSendWindowScale());
342-
}
343343
session.setSendUnack(tcpHeader.getAckNumber());
344344
session.setRecSequence(tcpHeader.getSequenceNumber());
345345
session.setTimestampReplyto(tcpHeader.getTimeStampSender());
@@ -350,6 +350,7 @@ private void acceptAck(TCPHeader tcpHeader, Session session){
350350
session.setAcked(false);
351351
}
352352
}
353+
353354
/**
354355
* set connection as aborting so that background worker will close it.
355356
* @param ip IP
@@ -386,9 +387,6 @@ private void replySynAck(IPv4Header ip, TCPHeader tcp){
386387
return;
387388

388389
synchronized (session) {
389-
int windowScaleFactor = (int) Math.pow(2, tcpheader.getWindowScale());
390-
session.setSendWindowSizeAndScale(tcpheader.getWindowSize(), windowScaleFactor);
391-
Log.d(TAG, "send-window size: " + session.getSendWindow());
392390
session.setMaxSegmentSize(tcpheader.getMaxSegmentSize());
393391
session.setSendUnack(tcpheader.getSequenceNumber());
394392
session.setSendNext(tcpheader.getSequenceNumber() + 1);

app/src/main/java/tech/httptoolkit/android/vpn/socket/SocketChannelReader.java

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,17 @@ private void readTCP(@NonNull Session session) {
9595

9696
try {
9797
do {
98-
if(!session.isClientWindowFull()) {
99-
len = channel.read(buffer);
100-
if(len > 0) { //-1 mean it reach the end of stream
101-
sendToRequester(buffer, len, session);
102-
buffer.clear();
103-
} else if(len == -1) {
104-
Log.d(TAG,"End of data from remote server, will send FIN to client");
105-
Log.d(TAG,"send FIN to: " + session);
106-
sendFin(session);
107-
session.setAbortingConnection(true);
108-
}
109-
} else {
110-
Log.e(TAG,"*** client window is full, now pause for " + session);
111-
break;
98+
len = channel.read(buffer);
99+
if (len > 0) { //-1 mean it reach the end of stream
100+
sendToRequester(buffer, len, session);
101+
buffer.clear();
102+
} else if (len == -1) {
103+
Log.d(TAG,"End of data from remote server, will send FIN to client");
104+
Log.d(TAG,"send FIN to: " + session);
105+
sendFin(session);
106+
session.setAbortingConnection(true);
112107
}
113-
} while(len > 0);
108+
} while (len > 0);
114109
}catch(NotYetConnectedException e){
115110
Log.e(TAG,"socket not connected");
116111
}catch(ClosedByInterruptException e){
@@ -148,7 +143,7 @@ private void sendToRequester(ByteBuffer buffer, int dataSize, @NonNull Session s
148143
* @param session Session
149144
*/
150145
private void pushDataToClient(@NonNull Session session){
151-
if(!session.hasReceivedData()){
146+
if (!session.hasReceivedData()) {
152147
//no data to send
153148
Log.d(TAG,"no data for vpn client");
154149
}

0 commit comments

Comments
 (0)