Implementation of range header #1703
Conversation
|
The picoquic demo CLI allows for downloading multiple files. Options like "-H" would apply the same range to all files. Isn't that a problem? |
|
sorry, you're right |
|
The last two commit allow specifying ranges as part of the "scenario" syntax used by picoquicdemo. The h3zero tests verify that this is correctly parsed, the test value being: |
|
@fbussery I have not been able to try the range requests in picoquicdemo -- I would need to implement the server side first, and that may take some time. Can you test this PR against a server that supports ranges? |
|
I did test it quicky. And It is not working. I will test your branch in our env today to check if I have the same status. I run the test in your env this way: picoquicdemo -l log.txt -G -v dash.akamaized.net 443 /akamai/bbb_30fps/bbb_30fps_3840x2160_12000k/bbb_30fps_3840x2160_12000k_12.m4v:#bytes=100-20000 |
|
Yes, I tried this locally and got an error. This is visible in qlog:
Error code 268 (0x10c) is "H3_REQUEST_CANCELLED (0x010c): The request or its response (including pushed response) is cancelled". |
|
@fbussery: I added parsing code for the range header, and then used that in a unit test, which works. I tried repeating the test against the akamai server, and it still fails. I think that we are missing a few steps. I read RFC 7233, and there are a few interesting points:
Do we know who is developing the Akamai server? They may be able to help. |
|
Hello, I will investigate further next week I hope. |
|
I tried repeating exactly the command that you used, and it still fails. If I remove the range specification, I do get the content of the whole chunk. I suspect that we are missing some negotiation step. It would be good to compare PCAPs of the successful session with curl and the failing session with picoquic, and look for potential differences. |
|
In fact, I found out how to make it work. so, if we apply index f9b28a94..5d6e055f 100644
--- a/picohttp/h3zero.c
+++ b/picohttp/h3zero.c
@@ -903,10 +903,6 @@ uint8_t * h3zero_create_request_header_frame_ex(uint8_t * bytes, uint8_t * bytes
bytes = h3zero_qpack_code_encode(bytes, bytes_max, 0xC0, 0x3F, H3ZERO_QPACK_SCHEME_HTTPS);
/* Path: doc_name. Use literal plus reference format */
bytes = h3zero_qpack_literal_plus_ref_encode(bytes, bytes_max, H3ZERO_QPACK_CODE_PATH, path, path_length);
- /*Optional: range. Use literal plus reference format */
- if (range_length > 0) {
- bytes = h3zero_qpack_literal_plus_ref_encode(bytes, bytes_max, H3ZERO_QPACK_RANGE, (uint8_t const *)range, range_length);
- }
/*Authority: host. Use literal plus reference format */
if (host != NULL) {
bytes = h3zero_qpack_literal_plus_ref_encode(bytes, bytes_max, H3ZERO_QPACK_AUTHORITY, (uint8_t const *)host, strlen(host));
@@ -915,6 +911,10 @@ uint8_t * h3zero_create_request_header_frame_ex(uint8_t * bytes, uint8_t * bytes
if (ua_string != NULL) {
bytes = h3zero_qpack_literal_plus_ref_encode(bytes, bytes_max, H3ZERO_QPACK_USER_AGENT, (uint8_t const*)ua_string, strlen(ua_string));
}
+ /*Optional: range. Use literal plus reference format */
+ if (range_length > 0) {
+ bytes = h3zero_qpack_literal_plus_ref_encode(bytes, bytes_max, H3ZERO_QPACK_RANGE, (uint8_t const *)range, range_length);
+ }
return bytes;
}This works |
|
Good find, but wow! I can't find that any part of RFC 7233 or 7232... |
|
I moved the code around a bit. The "range" header does not have to come after the UA header, but it must be sent after the "host" header. That makes more sense: the document is specified by the combination of path and host, thus both may be needed before a subset of the document can be specified. |
Adding an option to encode a range in GET and POST requests.
This is motivated by issue #1702, but the implementation differs somewhat: the new "range" and "range_length" arguments are only added to the "extended" methods
h3zero_create_request_header_frame_exandh3zero_create_post_header_frame_ex, so as to minimize the churn in existing projects. No support in the "connect" method, because range does not make sense there. Or does it?This implementation is insufficient. At a minimum, we need unit tests for the frame creation APIs. But it would be better to have a way to exercise the option. Maybe allow "document" arguments in the "picoquicdemo" app to include a range, as in:
Then, split the argument into path and range components.
But really, for a complete implementation, we should make sure that the demo server supports the range request, and only returns the requested range. At list for requests, maybe for "post" too, but we need to understand what "range" means for a post.