refactor pem #605
refactor pem #605
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #605 +/- ##
==========================================
- Coverage 79.91% 79.72% -0.19%
==========================================
Files 28 27 -1
Lines 5854 5893 +39
==========================================
+ Hits 4678 4698 +20
- Misses 1176 1195 +19
☔ View full report in Codecov by Sentry. |
include/aws/io/pem.h
Outdated
| * This code is slow, and it allocates, so please try | ||
| * not to call this in the middle of something that needs to be fast or resource sensitive. | ||
| */ | ||
| AWS_IO_API int aws_read_and_decode_pem_file_to_object_list( |
There was a problem hiding this comment.
debatable: would it be simpler if these functions were alternate array-list initializers? I can't imagine this function ever being used without the user needing to initialize the list first. Like:
int aws_array_list_init_from_pem_file_contents(struct aws_array_list *list, struct aws_allocator, struct aws_byte_cursor pem_contents);
int aws_array_list_init_from_pem_file_path(struct aws_array_list *list, struct aws_allocator, const char *pem_path);
Or call it "pem_objects" instead of "array_list" (I like this idea best)?
int aws_pem_objects_init_from_file_contents(struct aws_array_list *pem_objects, struct aws_allocator, struct aws_byte_cursor pem_contents);
int aws_pem_objects_init_from_file_path(struct aws_array_list *pem_objects, struct aws_allocator, const char *pem_path);
There was a problem hiding this comment.
Current names were mostly based on existing names for internal function with a modification to refer to elements as pem objects instead of cert_or_key.
I like your suggestion of just naming it pem_objects_init. next commit switches to that.
I think original code wanted array to be initialized outside of the call because the caller might have better context on how many elements are there in the pem file. But in practice the answer to that is either probably just one or probably many, so initializing outside was not super helpful. Moved array init inside of the function.
Co-authored-by: Michael Graeb <[email protected]>
Co-authored-by: Michael Graeb <[email protected]>
include/aws/io/io.h
Outdated
| @@ -252,6 +252,8 @@ enum aws_io_errors { | |||
|
|
|||
| AWS_IO_TLS_ERROR_READ_FAILURE, | |||
|
|
|||
| AWS_ERROR_PEM_MALFORMED_OBJECT, | |||
There was a problem hiding this comment.
Do you expect other systems to use a different error if the objects themselves are all valid, but the "document" itself is malformed due to missing required objects?
If we fear people will re-use this error-code for bad documents, give it a more generic name like AWS_ERROR_PEM_MALFORMED. But if you want distinct error-codes to be created for malformed documents of a specific type, then this is good
There was a problem hiding this comment.
let me just make it generic AWS_ERROR_PEM_MALFORMED. i dont think its worth distinguishing that its individual objects that are malformed
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.