Skip to content

Commit 2c845b0

Browse files
graebmgraebm
authored
Revise "Better helper functions for querying header values (#412)" (#414)
`aws_http_headers_get_single()` was clunky to use correctly. It's very easy to mis-use and assume it only raises an error if the header is missing. Let's just continue doing what we were before, what [most HTTP APIs do](https://netty.io/4.1/api/io/netty/handler/codec/http/HttpHeaders.html#get-java.lang.CharSequence-), and return the first header found.
1 parent 3b520ea commit 2c845b0

File tree

6 files changed

+39
-119
lines changed

6 files changed

+39
-119
lines changed

‎include/aws/http/http.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ enum aws_http_errors {
5454
AWS_ERROR_HTTP_STREAM_MANAGER_SHUTTING_DOWN,
5555
AWS_ERROR_HTTP_STREAM_MANAGER_CONNECTION_ACQUIRE_FAILURE,
5656
AWS_ERROR_HTTP_STREAM_MANAGER_UNEXPECTED_HTTP_VERSION,
57-
AWS_ERROR_HTTP_UNEXPECTED_DUPLICATE_HEADER,
5857

5958
AWS_ERROR_HTTP_END_RANGE = AWS_ERROR_ENUM_END_RANGE(AWS_C_HTTP_PACKAGE_ID)
6059
};

‎include/aws/http/request_response.h‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -528,33 +528,17 @@ int aws_http_headers_get_index(
528528
struct aws_http_header *out_header);
529529

530530
/**
531-
* Get the single value for this name.
532-
* AWS_ERROR_HTTP_HEADER_NOT_FOUND is raised if the name is not found.
533-
* AWS_ERROR_HTTP_UNEXPECTED_DUPLICATE_HEADER is raised if the name is found multiple times.
534531
*
535-
* If you expect a comma-separated value, use aws_http_headers_get_comma_separated() instead.
536-
*/
537-
AWS_HTTP_API
538-
int aws_http_headers_get_single(
539-
const struct aws_http_headers *headers,
540-
struct aws_byte_cursor name,
541-
struct aws_byte_cursor *out_value);
542-
543-
/**
544-
* Get the comma-separated value for this name.
545-
* If there are any headers for this name, a new aws_string is returned that you
546-
* are responsible for destroying. If there are multiple headers with this name
547-
* their values are concatenated together with comma-separators.
532+
* Get all values with this name, combined into one new aws_string that you are responsible for destroying.
533+
* If there are multiple headers with this name, their values are appended with comma-separators.
548534
* If there are no headers with this name, NULL is returned and AWS_ERROR_HTTP_HEADER_NOT_FOUND is raised.
549535
*/
550536
AWS_HTTP_API
551-
struct aws_string *aws_http_headers_get_comma_separated(
552-
const struct aws_http_headers *headers,
553-
struct aws_byte_cursor name);
537+
struct aws_string *aws_http_headers_get_all(const struct aws_http_headers *headers, struct aws_byte_cursor name);
554538

555539
/**
556-
* DEPRECATED
557-
* Use aws_http_headers_get_single() or aws_http_headers_get_comma_separated() instead.
540+
* Get the first value for this name, ignoring any additional values.
541+
* AWS_ERROR_HTTP_HEADER_NOT_FOUND is raised if the name is not found.
558542
*/
559543
AWS_HTTP_API
560544
int aws_http_headers_get(

‎source/http.c‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,6 @@ static struct aws_error_info s_errors[] = {
139139
AWS_DEFINE_ERROR_INFO_HTTP(
140140
AWS_ERROR_HTTP_STREAM_MANAGER_UNEXPECTED_HTTP_VERSION,
141141
"Stream acquisition failed because stream manager got an unexpected version of HTTP connection"),
142-
AWS_DEFINE_ERROR_INFO_HTTP(
143-
AWS_ERROR_HTTP_UNEXPECTED_DUPLICATE_HEADER,
144-
"A header name that should occur only once was encountered multiple times."),
145142
};
146143
/* clang-format on */
147144

‎source/request_response.c‎

Lines changed: 20 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -314,58 +314,22 @@ int aws_http_headers_get_index(
314314
return aws_array_list_get_at(&headers->array_list, out_header, index);
315315
}
316316

317-
int aws_http_headers_get_single(
318-
const struct aws_http_headers *headers,
319-
struct aws_byte_cursor name,
320-
struct aws_byte_cursor *out_value) {
321-
322-
AWS_PRECONDITION(headers);
323-
AWS_PRECONDITION(out_value);
324-
AWS_PRECONDITION(aws_byte_cursor_is_valid(&name));
325-
326-
AWS_ZERO_STRUCT(*out_value);
327-
328-
bool found = false;
329-
struct aws_byte_cursor value;
330-
AWS_ZERO_STRUCT(value);
331-
struct aws_http_header *header = NULL;
332-
const size_t count = aws_http_headers_count(headers);
333-
for (size_t i = 0; i < count; ++i) {
334-
aws_array_list_get_at_ptr(&headers->array_list, (void **)&header, i);
335-
AWS_ASSUME(header);
336-
337-
if (aws_http_header_name_eq(header->name, name)) {
338-
if (found) {
339-
return aws_raise_error(AWS_ERROR_HTTP_UNEXPECTED_DUPLICATE_HEADER);
340-
}
341-
value = header->value;
342-
found = true;
343-
}
344-
}
345-
346-
if (found) {
347-
*out_value = value;
348-
return AWS_OP_SUCCESS;
349-
} else {
350-
return aws_raise_error(AWS_ERROR_HTTP_HEADER_NOT_FOUND);
351-
}
352-
}
353-
354-
/* RFC-7230 - 3.2.2
355-
* A recipient MAY combine multiple header fields with the same field name
356-
* into one "field-name: field-value" pair, without changing the semantics
357-
* of the message, by appending each subsequent field value to the combined
358-
* field value in order, separated by a comma */
317+
/* RFC-9110 - 5.3
318+
* A recipient MAY combine multiple field lines within a field section that
319+
* have the same field name into one field line, without changing the semantics
320+
* of the message, by appending each subsequent field line value to the initial
321+
* field line value in order, separated by a comma (",") and optional whitespace
322+
* (OWS, defined in Section 5.6.3). For consistency, use comma SP. */
359323
AWS_HTTP_API
360-
struct aws_string *aws_http_headers_get_comma_separated(
361-
const struct aws_http_headers *headers,
362-
struct aws_byte_cursor name) {
324+
struct aws_string *aws_http_headers_get_all(const struct aws_http_headers *headers, struct aws_byte_cursor name) {
363325

364326
AWS_PRECONDITION(headers);
365327
AWS_PRECONDITION(aws_byte_cursor_is_valid(&name));
366328

367329
struct aws_string *value_str = NULL;
368330

331+
const struct aws_byte_cursor separator = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL(", ");
332+
369333
struct aws_byte_buf value_builder;
370334
aws_byte_buf_init(&value_builder, headers->alloc, 0);
371335
bool found = false;
@@ -377,7 +341,7 @@ struct aws_string *aws_http_headers_get_comma_separated(
377341
if (!found) {
378342
found = true;
379343
} else {
380-
aws_byte_buf_append_byte_dynamic(&value_builder, ',');
344+
aws_byte_buf_append_dynamic(&value_builder, &separator);
381345
}
382346
aws_byte_buf_append_dynamic(&value_builder, &header->value);
383347
}
@@ -419,48 +383,38 @@ int aws_http_headers_get(
419383

420384
bool aws_http_headers_has(const struct aws_http_headers *headers, struct aws_byte_cursor name) {
421385

422-
AWS_PRECONDITION(headers);
423-
AWS_PRECONDITION(aws_byte_cursor_is_valid(&name));
424-
425-
struct aws_http_header *header = NULL;
426-
const size_t count = aws_http_headers_count(headers);
427-
for (size_t i = 0; i < count; ++i) {
428-
aws_array_list_get_at_ptr(&headers->array_list, (void **)&header, i);
429-
AWS_ASSUME(header);
430-
431-
if (aws_http_header_name_eq(header->name, name)) {
432-
return true;
433-
}
386+
struct aws_byte_cursor out_value;
387+
if (aws_http_headers_get(headers, name, &out_value)) {
388+
return false;
434389
}
435-
436-
return false;
390+
return true;
437391
}
438392

439393
int aws_http2_headers_get_request_method(
440394
const struct aws_http_headers *h2_headers,
441395
struct aws_byte_cursor *out_method) {
442-
return aws_http_headers_get_single(h2_headers, aws_http_header_method, out_method);
396+
return aws_http_headers_get(h2_headers, aws_http_header_method, out_method);
443397
}
444398

445399
int aws_http2_headers_get_request_scheme(
446400
const struct aws_http_headers *h2_headers,
447401
struct aws_byte_cursor *out_scheme) {
448-
return aws_http_headers_get_single(h2_headers, aws_http_header_scheme, out_scheme);
402+
return aws_http_headers_get(h2_headers, aws_http_header_scheme, out_scheme);
449403
}
450404

451405
int aws_http2_headers_get_request_authority(
452406
const struct aws_http_headers *h2_headers,
453407
struct aws_byte_cursor *out_authority) {
454-
return aws_http_headers_get_single(h2_headers, aws_http_header_authority, out_authority);
408+
return aws_http_headers_get(h2_headers, aws_http_header_authority, out_authority);
455409
}
456410

457411
int aws_http2_headers_get_request_path(const struct aws_http_headers *h2_headers, struct aws_byte_cursor *out_path) {
458-
return aws_http_headers_get_single(h2_headers, aws_http_header_path, out_path);
412+
return aws_http_headers_get(h2_headers, aws_http_header_path, out_path);
459413
}
460414

461415
int aws_http2_headers_get_response_status(const struct aws_http_headers *h2_headers, int *out_status_code) {
462416
struct aws_byte_cursor status_code_cur;
463-
int return_code = aws_http_headers_get_single(h2_headers, aws_http_header_status, &status_code_cur);
417+
int return_code = aws_http_headers_get(h2_headers, aws_http_header_status, &status_code_cur);
464418
if (return_code == AWS_OP_SUCCESS) {
465419
uint64_t code_val_u64;
466420
if (aws_byte_cursor_utf8_parse_u64(status_code_cur, &code_val_u64)) {
@@ -1006,7 +960,7 @@ struct aws_http_message *aws_http2_message_new_from_http1(
1006960
*/
1007961
struct aws_byte_cursor host_value;
1008962
AWS_ZERO_STRUCT(host_value);
1009-
if (aws_http_headers_get_single(http1_msg->headers, aws_byte_cursor_from_c_str("host"), &host_value) ==
963+
if (aws_http_headers_get(http1_msg->headers, aws_byte_cursor_from_c_str("host"), &host_value) ==
1010964
AWS_OP_SUCCESS) {
1011965
if (aws_http_headers_add(copied_headers, aws_http_header_authority, host_value)) {
1012966
goto error;
@@ -1018,14 +972,6 @@ struct aws_http_message *aws_http2_message_new_from_http1(
1018972
aws_http_header_authority.ptr,
1019973
(int)host_value.len,
1020974
host_value.ptr);
1021-
} else if (aws_last_error() != AWS_ERROR_HTTP_HEADER_NOT_FOUND) {
1022-
AWS_LOGF_ERROR(
1023-
AWS_LS_HTTP_GENERAL,
1024-
"Failed to create HTTP/2 message from HTTP/1 message, ip: %p, due to error with host header: %d %s",
1025-
(void *)http1_msg,
1026-
aws_last_error(),
1027-
aws_error_name(aws_last_error()));
1028-
goto error;
1029975
}
1030976
/* TODO: If the host headers is missing, the target URI could be the other source of the authority information
1031977
*/

‎tests/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ add_test_case(headers_erase_index)
2525
add_test_case(headers_erase)
2626
add_test_case(headers_erase_value)
2727
add_test_case(headers_clear)
28-
add_test_case(headers_get_comma_separated)
28+
add_test_case(headers_get_all)
2929
add_test_case(h2_headers_request_pseudos_get_set)
3030
add_test_case(h2_headers_response_pseudos_get_set)
3131

‎tests/test_message.c‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,7 @@ TEST_CASE(headers_add) {
155155

156156
/* get-by-name (ignore case) */
157157
struct aws_byte_cursor value_get;
158-
ASSERT_SUCCESS(
159-
aws_http_headers_get_single(headers, aws_byte_cursor_from_c_str("host"), &value_get)); /* ignore case */
158+
ASSERT_SUCCESS(aws_http_headers_get(headers, aws_byte_cursor_from_c_str("host"), &value_get)); /* ignore case */
160159
ASSERT_SUCCESS(s_check_value_eq(value_get, "example.com"));
161160

162161
aws_http_headers_release(headers);
@@ -182,16 +181,11 @@ TEST_CASE(headers_add_array) {
182181
ASSERT_SUCCESS(s_check_headers_eq(src_headers[i], get));
183182
}
184183

185-
/* check the DEPRECATED get-by-name returns first one it sees */
184+
/* check that get-by-name returns first one it sees */
186185
struct aws_byte_cursor get;
187186
ASSERT_SUCCESS(aws_http_headers_get(headers, aws_byte_cursor_from_c_str("COOKIE"), &get));
188187
ASSERT_SUCCESS(s_check_value_eq(get, "a=1"));
189188

190-
/* check that get_single() reports an error due to duplicates */
191-
ASSERT_ERROR(
192-
AWS_ERROR_HTTP_UNEXPECTED_DUPLICATE_HEADER,
193-
aws_http_headers_get_single(headers, aws_byte_cursor_from_c_str("COOKIE"), &get));
194-
195189
aws_http_headers_release(headers);
196190
return AWS_OP_SUCCESS;
197191
}
@@ -222,7 +216,7 @@ TEST_CASE(headers_set) {
222216
ASSERT_UINT_EQUALS(1, aws_http_headers_count(headers));
223217

224218
struct aws_byte_cursor value_get;
225-
ASSERT_SUCCESS(aws_http_headers_get_single(headers, aws_byte_cursor_from_c_str("cookie"), &value_get));
219+
ASSERT_SUCCESS(aws_http_headers_get(headers, aws_byte_cursor_from_c_str("cookie"), &value_get));
226220
ASSERT_SUCCESS(s_check_value_eq(value_get, "d=4"));
227221

228222
aws_http_headers_release(headers);
@@ -331,22 +325,22 @@ TEST_CASE(headers_clear) {
331325
return AWS_OP_SUCCESS;
332326
}
333327

334-
TEST_CASE(headers_get_comma_separated) {
328+
TEST_CASE(headers_get_all) {
335329
(void)ctx;
336330

337331
struct aws_http_headers *headers = aws_http_headers_new(allocator);
338332

339333
/* Check when no such headers exist */
340334
aws_http_headers_clear(headers);
341335
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("Host"), aws_byte_cursor_from_c_str("example.com"));
342-
ASSERT_NULL(aws_http_headers_get_comma_separated(headers, aws_byte_cursor_from_c_str("X-My-List")));
336+
ASSERT_NULL(aws_http_headers_get_all(headers, aws_byte_cursor_from_c_str("X-My-List")));
343337
ASSERT_INT_EQUALS(AWS_ERROR_HTTP_HEADER_NOT_FOUND, aws_last_error());
344338

345339
/* Check getting a single value */
346340
aws_http_headers_clear(headers);
347341
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("Host"), aws_byte_cursor_from_c_str("example.com"));
348342
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-My-List"), aws_byte_cursor_from_c_str("A"));
349-
struct aws_string *value = aws_http_headers_get_comma_separated(headers, aws_byte_cursor_from_c_str("X-My-List"));
343+
struct aws_string *value = aws_http_headers_get_all(headers, aws_byte_cursor_from_c_str("X-My-List"));
350344
ASSERT_NOT_NULL(value);
351345
ASSERT_TRUE(aws_string_eq_c_str(value, "A"));
352346
aws_string_destroy(value);
@@ -355,7 +349,7 @@ TEST_CASE(headers_get_comma_separated) {
355349
aws_http_headers_clear(headers);
356350
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("Host"), aws_byte_cursor_from_c_str("example.com"));
357351
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-My-List"), aws_byte_cursor_from_c_str(""));
358-
value = aws_http_headers_get_comma_separated(headers, aws_byte_cursor_from_c_str("X-My-List"));
352+
value = aws_http_headers_get_all(headers, aws_byte_cursor_from_c_str("X-My-List"));
359353
ASSERT_NOT_NULL(value);
360354
ASSERT_TRUE(aws_string_eq_c_str(value, ""));
361355
aws_string_destroy(value);
@@ -365,26 +359,26 @@ TEST_CASE(headers_get_comma_separated) {
365359
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("Host"), aws_byte_cursor_from_c_str("example.com"));
366360
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-My-List"), aws_byte_cursor_from_c_str("A"));
367361
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-My-List"), aws_byte_cursor_from_c_str("B"));
368-
value = aws_http_headers_get_comma_separated(headers, aws_byte_cursor_from_c_str("X-My-List"));
362+
value = aws_http_headers_get_all(headers, aws_byte_cursor_from_c_str("X-My-List"));
369363
ASSERT_NOT_NULL(value);
370-
ASSERT_TRUE(aws_string_eq_c_str(value, "A,B"));
364+
ASSERT_TRUE(aws_string_eq_c_str(value, "A, B"));
371365
aws_string_destroy(value);
372366

373367
/* Check more edge cases */
374368
aws_http_headers_clear(headers);
375369
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("Host"), aws_byte_cursor_from_c_str("example.com"));
376370
/* some fields have single entry, some fields have multiple entries */
377-
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-My-List"), aws_byte_cursor_from_c_str("A,B"));
371+
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-My-List"), aws_byte_cursor_from_c_str("A, B"));
378372
/* preserve whitespace within middle of value. also name is different case */
379-
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("x-my-list"), aws_byte_cursor_from_c_str("C, D"));
373+
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("x-my-list"), aws_byte_cursor_from_c_str("C,D"));
380374
aws_http_headers_add(
381375
headers, aws_byte_cursor_from_c_str("X-My-List-"), aws_byte_cursor_from_c_str("BAD-EXTRA-DASH"));
382376
/* name is different case, and blank value*/
383377
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("X-MY-LIST"), aws_byte_cursor_from_c_str(""));
384378
aws_http_headers_add(headers, aws_byte_cursor_from_c_str("x-my-list"), aws_byte_cursor_from_c_str("E"));
385-
value = aws_http_headers_get_comma_separated(headers, aws_byte_cursor_from_c_str("X-My-List"));
379+
value = aws_http_headers_get_all(headers, aws_byte_cursor_from_c_str("X-My-List"));
386380
ASSERT_NOT_NULL(value);
387-
ASSERT_TRUE(aws_string_eq_c_str(value, "A,B,C, D,,E"));
381+
ASSERT_TRUE(aws_string_eq_c_str(value, "A, B, C,D, , E"));
388382
aws_string_destroy(value);
389383

390384
/* Done */

0 commit comments

Comments
 (0)