A pure ACK_ENC packet should be non-probing

[nemethf]

Currently, if a packet contains only an ACK_ENC frame, then picoquic_decode_frames treats it as a validating_packet. This behavior is not in line with the RFC, which says:

PATH_CHALLENGE, PATH_RESPONSE, NEW_CONNECTION_ID, and PADDING frames are "probing frames", and all other frames are "non-probing frames". A packet containing only probing frames is a "probing packet", and a packet containing any other frame is a "non-probing packet".

As a result, the server continues to send packets on an old path even if both parties have been validated a new path and the client only sends ACK_ENC frames.

I believe the following simple patch fixes the issue.

diff --git a/picoquic/frames.c b/picoquic/frames.c
index b9e5dd50..fffe8306 100644
--- a/picoquic/frames.c
+++ b/picoquic/frames.c
@@ -5603,8 +5603,8 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const
                 break;
             }
             }
-            is_path_validating_packet &= is_path_validating_frame;
         }
+        is_path_validating_packet &= is_path_validating_frame;
     }
 
     if (bytes != NULL) {

Thank you.

[huitema]

Thanks for looking into this issue. ACK frames, and also Stream frames, are not "path probing" frames. Section 9.1 of RFC 9000, Probing a New Path, only lists PATH_CHALLENGE, PATH_RESPONSE, NEW_CONNECTION_ID, and PADDING frames.

The flag name is_path_validating_frame is wrong. The correct name would be is_path_probing_frame, to match the vocabulary of RFC 9000, and the cumulative would be is_path_probing_packet. The flag in the path context should not be path_x->last_non_validating_pn but path_x->last_non_path_probing_pn. Aligning the vocabulary with RFC 9000 will ease future maintenance.

[huitema]

Thanks again, @nemethf. This is being fixed in PR #1588.