Skip to content

Commit

Permalink
[UNDERTOW-2444] Fix RST scenario violation in H2
Browse files Browse the repository at this point in the history
  • Loading branch information
baranowb authored and laurastreet committed Oct 4, 2024
1 parent 7578625 commit 2afc6f1
Showing 1 changed file with 36 additions and 3 deletions.
39 changes: 36 additions & 3 deletions core/src/main/java/io/undertow/protocols/http2/Http2Channel.java
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,7 @@ private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
resetStreamTracker.store(streamId, holder);
if(streamId % 2 == (isClient() ? 1 : 0)) {
sendConcurrentStreamsAtomicUpdater.getAndDecrement(this);
holder.sent = true;
} else {
receiveConcurrentStreamsAtomicUpdater.getAndDecrement(this);
}
Expand All @@ -1239,13 +1240,14 @@ private StreamHolder handleRstStream(int streamId, boolean receivedRst) {
//Server side originated, no input from client other than RST
//this can happen on page refresh when push happens, but client
//still has valid cache entry
//NOTE: this is specific case when its set.
holder.resetByPeer = receivedRst;
} else {
handleRstWindow();
}
}
} else if(receivedRst){
final StreamHolder resetStream = resetStreamTracker.find(streamId);
final StreamHolder resetStream = resetStreamTracker.find(streamId, true);
if(resetStream != null && resetStream.resetByPeer) {
//This means other side reset stream at some point.
//depending on peer or network latency our frames might be late and
Expand Down Expand Up @@ -1378,6 +1380,10 @@ private static final class StreamHolder {
* This flag is set only in case of short lived server push that was reset by remote end.
*/
boolean resetByPeer = false;
/**
* flag indicate whether our side originated. This is done for caching purposes, handlng differs.
*/
boolean sent = false;
Http2StreamSourceChannel sourceChannel;
Http2StreamSinkChannel sinkChannel;

Expand All @@ -1388,6 +1394,13 @@ private static final class StreamHolder {
StreamHolder(Http2StreamSinkChannel sinkChannel) {
this.sinkChannel = sinkChannel;
}

@Override
public String toString() {
return "StreamHolder [sourceClosed=" + sourceClosed + ", sinkClosed=" + sinkClosed + ", resetByPeer=" + resetByPeer
+ ", sent=" + sent + ", sourceChannel=" + sourceChannel + ", sinkChannel=" + sinkChannel + "]";
}

}

// cache that keeps track of streams until they can be evicted @see Http2Channel#RST_STREAM_EVICATION_TIME
Expand All @@ -1403,12 +1416,27 @@ private void store(int streamId, StreamHolder streamHolder) {
streamHolders.put(streamId, streamHolder);
entries.add(new StreamCacheEntry(streamId));
}
private StreamHolder find(int streamId) {

/**
* Method will return only sent
* @param streamId
* @return
*/
private StreamHolder find(final int streamId) {
return find(streamId, false);
}

private StreamHolder find(final int streamId, final boolean all) {
for (Iterator<StreamCacheEntry> iterator = entries.iterator(); iterator.hasNext();) {
StreamCacheEntry entry = iterator.next();
if (entry.shouldEvict()) {
iterator.remove();
StreamHolder holder = streamHolders.remove(entry.streamId);
if(!holder.sent || holder.resetByPeer) {
//if its not our end of chain, its just cached, so we only cache for purpose of
// handling eager RST
continue;
}
AbstractHttp2StreamSourceChannel receiver = holder.sourceChannel;
if(receiver != null) {
IoUtils.safeClose(receiver);
Expand All @@ -1422,7 +1450,12 @@ private StreamHolder find(int streamId) {
}
} else break;
}
return streamHolders.get(streamId);
final StreamHolder holder = streamHolders.get(streamId);
if(holder != null && (!all && !holder.sent)) {
return null;
} else {
return holder;
}
}

private Map<Integer, StreamHolder> getStreamHolders() {
Expand Down

0 comments on commit 2afc6f1

Please sign in to comment.