Bug fixes in library and hygiene improvements in test cases #52
Conversation
source/sigv4.c
Outdated
|
|
||
| /* Case when parameter has empty value even though the query parameter entry has the form | ||
| * "<param>=". */ | ||
| else if( ( fieldHasValue ) && ( ( currQueryIndex - startOfFieldOrValue ) == 0U ) ) |
There was a problem hiding this comment.
So this will happen in the case of either <param>=<end of query> or <param>=&. Are either of these valid?
There was a problem hiding this comment.
The API Gateway Tutorial provides an example that supports providing query parameters with no value in both the formats.
The example REST API is: http://petstore-demo-endpoint.execute-api.com/petstore/pets?type=cat&page=2
It can be used successfully with the URI as http://petstore-demo-endpoint.execute-api.com/petstore/pets?type&page and http://petstore-demo-endpoint.execute-api.com/petstore/pets?type=&page=
There was a problem hiding this comment.
Moreover, the AWS SigV4 spec mentions about providing the query parameter name with = character but an empty string for empty values.
For each parameter, append the URI-encoded parameter name, followed by the equals sign character (=), followed by the URI-encoded parameter value. Use an empty string for parameters that have no value.
|
|
||
| if( returnStatus == SigV4Success ) | ||
| { | ||
| pBufLoc += encodedLen; | ||
| remainingLen -= encodedLen; | ||
|
|
||
| /* Encode parameter value if non-empty. Query parameters can have empty values. */ | ||
| if( pCanonicalRequest->pQueryLoc[ paramsIndex ].value.dataLen > 0U ) |
There was a problem hiding this comment.
Why can this be assumed to be true now? Or does writeValueInCanonicalizedQueryString still work with a zero length parameter?
There was a problem hiding this comment.
Or does writeValueInCanonicalizedQueryString still work with a zero length parameter?
Yes, this function adds the = character which is required for canonical query format even with empty values.
* Adding OTA agent files * Added tokens to lexicon.txt and spellings fix * spell-check fix * spell check and doxygen fix * Added back OTA_JsonFileSignatureKey * Support for coreJSON2.0 * Add fileType key support for multi file type support * Fix filetype offset in job param table * Update log strings in ota.c * User buffer initial commit * Remove unused code * Alternative memory allocation support * Implement request timer and self-test timers * Cleanup on ota agent * Update the ota config includes in ota.c and ota_private.h * Fix issue with job parsing flag width * Fix ota agent unit tests build errors * Spell-check and formatting fix * doxygen fix and added 2 error codes * formatting and spell check fix * Complexity reduction fix * fixed commas for debug statements * Remove void * from the json parsin table * Add an error for failing to decode cbor * Remove ota types header from library cmake paths * Fix some compilation issues and warnings * Fix PR feedfback in ota agent * Fix CI checks * Addressed PR feedback * Fix pointer usage in job parsing (aws#76) * Update lexicon and github workflow * Sync fix from development * Format files Co-authored-by: pvyawaha <[email protected]> Co-authored-by: Joshua Yan <[email protected]> Co-authored-by: Tiangang Song <[email protected]>
| pBufLoc += encodedLen; | ||
| remainingLen -= encodedLen; | ||
| } | ||
| assert( pCanonicalRequest->pQueryLoc[ paramsIndex ].value.pData != NULL ); |
There was a problem hiding this comment.
@aggarw13 Since, as you mentioned above that, writeValueInCanonicalizedQueryString can work with empty values, why are we asserting this?
Doesn't it essentially force all query parameters to have non-empty value?
Hygiene Changes
Bugfixes
Fix 2 categories of bugs found from adding signature verification in tests:
<param>=&<param2>OR<param>&<param2>)NULLcharacter.