Skip to content

Commit

Permalink
Allow empty file blob uploads (#2393)
Browse files Browse the repository at this point in the history 
* Allow empty file blob uploads

* PR feedback

* pr feed back
  • Loading branch information
@ericwolz
ericwolz authored Oct 22, 2022
1 parent 4602822 commit 19b7457
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 59 deletions.
10 changes: 7 additions & 3 deletions iothub_client/src/blob.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include "azure_c_shared_utility/azure_base64.h"
#include "azure_c_shared_utility/shared_util_options.h"

#define HTTP_STATUS_CODE_OK 200
#define IS_HTTP_STATUS_CODE_SUCCESS(x) ((x) >= 100 && (x) < 300)

static const char blockListXmlBegin[] = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n<BlockList>";
static const char blockListXmlEnd[] = "</BlockList>";
static const char blockListUriMarker[] = "&comp=blocklist";
Expand Down Expand Up @@ -105,7 +108,7 @@ BLOB_RESULT Blob_UploadBlock(
LogError("unable to HTTPAPIEX_ExecuteRequest");
result = BLOB_HTTP_ERROR;
}
else if (*httpStatus >= 300)
else if (!IS_HTTP_STATUS_CODE_SUCCESS(*httpStatus))
{
LogError("HTTP status from storage does not indicate success (%d)", (int)*httpStatus);
result = BLOB_OK;
Expand Down Expand Up @@ -152,6 +155,7 @@ static BLOB_RESULT InvokeUserCallbackAndSendBlobs(HTTPAPIEX_HANDLE httpApiExHand
{
uploadOneMoreBlock = 0;
result = BLOB_OK;
*httpStatus = HTTP_STATUS_CODE_OK;
}
else
{
Expand Down Expand Up @@ -195,7 +199,7 @@ static BLOB_RESULT InvokeUserCallbackAndSendBlobs(HTTPAPIEX_HANDLE httpApiExHand
LogError("unable to Blob_UploadBlock. Returned value=%d", result);
isError = 1;
}
else if (*httpStatus >= 300)
else if (!IS_HTTP_STATUS_CODE_SUCCESS(*httpStatus))
{
LogError("unable to Blob_UploadBlock. Returned httpStatus=%u", (unsigned int)*httpStatus);
isError = 1;
Expand Down Expand Up @@ -348,7 +352,7 @@ BLOB_RESULT Blob_UploadMultipleBlocksFromSasUri(const char* SASURI, IOTHUB_CLIEN
{
LogError("Failed in invoking callback/sending blob step");
}
else if (*httpStatus < 300)
else if (IS_HTTP_STATUS_CODE_SUCCESS(*httpStatus))
{
// Per SRS_BLOB_02_026, it possible for us to have a result=BLOB_OK AND a non-success HTTP status code.
// In order to maintain back-compat with existing code, we will return the BLOB_OK to the caller but NOT invoke this final step.
Expand Down
65 changes: 59 additions & 6 deletions iothub_client/tests/blob_ut/blob_ut.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ static void my_gballoc_free(void* s)
#undef malloc
#endif

#define TEST_HTTPAPIEX_HANDLE (HTTPAPIEX_HANDLE)0x4343;
#define HTTP_OK 200
#define HTTP_NOT_FOUND 404

TEST_DEFINE_ENUM_TYPE(HTTPAPI_REQUEST_TYPE, HTTPAPI_REQUEST_TYPE_VALUES);
IMPLEMENT_UMOCK_C_ENUM_TYPE(HTTPAPI_REQUEST_TYPE, HTTPAPI_REQUEST_TYPE_VALUES);

Expand Down Expand Up @@ -129,9 +133,8 @@ static void on_umock_c_error(UMOCK_C_ERROR_CODE error_code)
}

static BUFFER_HANDLE testValidBufferHandle; /*assigned in TEST_SUITE_INITIALIZE*/
static unsigned int httpResponse; /*used as out parameter in every call to Blob_....*/
static const unsigned int TwoHundred = 200;
static const unsigned int FourHundredFour = 404;
static const unsigned int TwoHundred = HTTP_OK;
static const unsigned int FourHundredFour = HTTP_NOT_FOUND;

// Allocate test content during initial test setup only. This buffer is very large,
// which means significant performance degradation on Valgrind tests if we were to
Expand Down Expand Up @@ -337,6 +340,7 @@ TEST_FUNCTION_CLEANUP(Cleanup)
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_with_NULL_SasUri_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;

///act
BLOB_RESULT result = Blob_UploadMultipleBlocksFromSasUri(NULL, FileUpload_GetData_Callback, &context, &httpResponse, testValidBufferHandle, NULL, NULL, NULL);
Expand All @@ -351,6 +355,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_with_NULL_SasUri_fails)
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_with_NULL_getDataCallBack_and_non_NULL_context_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;

///act
BLOB_RESULT result = Blob_UploadMultipleBlocksFromSasUri(TEST_VALID_SASURI_1, NULL, &context, &httpResponse, testValidBufferHandle, NULL, NULL, NULL);
Expand All @@ -365,6 +370,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_with_NULL_getDataCallBack_and_
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_succeeds_when_HTTP_status_code_is_404)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
size_t size = 1;
context.size = size;
Expand Down Expand Up @@ -406,7 +412,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_succeeds_when_HTTP_status_code
STRICT_EXPECTED_CALL(STRING_c_str(IGNORED_PTR_ARG)) /*this is getting the relative path as const char* */
.IgnoreArgument_handle();

int responseCode = 404; /*not found*/
int responseCode = HTTP_NOT_FOUND; /*not found*/
STRICT_EXPECTED_CALL(HTTPAPIEX_ExecuteRequest(IGNORED_PTR_ARG, HTTPAPI_REQUEST_PUT, IGNORED_PTR_ARG, NULL, IGNORED_PTR_ARG, &httpResponse, NULL, testValidBufferHandle))
.IgnoreArgument_handle()
.IgnoreArgument_relativePath()
Expand All @@ -428,6 +434,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_succeeds_when_HTTP_status_code

///assert
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_OK, result);
ASSERT_ARE_EQUAL(int, HTTP_NOT_FOUND, httpResponse);
ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());

///cleanup
Expand All @@ -436,6 +443,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_succeeds_when_HTTP_status_code
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_HTTPAPIEX_ExecuteRequest_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
size_t size = 1;
context.size = size;
Expand Down Expand Up @@ -477,7 +485,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_HTTPAPIEX_ExecuteRe
STRICT_EXPECTED_CALL(STRING_c_str(IGNORED_PTR_ARG)) /*this is getting the relative path as const char* */
.IgnoreArgument_handle();

int responseCode = 200; /*ok*/
int responseCode = HTTP_OK; /*ok*/
STRICT_EXPECTED_CALL(HTTPAPIEX_ExecuteRequest(IGNORED_PTR_ARG, HTTPAPI_REQUEST_PUT, IGNORED_PTR_ARG, NULL, IGNORED_PTR_ARG, &httpResponse, NULL, testValidBufferHandle))
.IgnoreArgument_handle()
.IgnoreArgument_relativePath()
Expand Down Expand Up @@ -509,6 +517,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_HTTPAPIEX_ExecuteRe
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_BUFFER_create_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
context.size = 1;
context.source = &c;
Expand Down Expand Up @@ -540,6 +549,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_BUFFER_create_fails
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_HTTPAPIEX_Create_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
context.size = 1;
context.source = &c;
Expand Down Expand Up @@ -567,6 +577,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_HTTPAPIEX_Create_fa
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_malloc_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
context.size = 1;
context.source = &c;
Expand All @@ -591,6 +602,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_fails_when_malloc_fails)
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_SasUri_is_wrong_fails_1)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
context.size = 1;
context.source = &c;
Expand All @@ -611,6 +623,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_SasUri_is_wrong_fails_1)
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_SasUri_is_wrong_fails_2)
{
///arrange
unsigned int httpResponse = HTTP_OK;
unsigned char c = '3';
context.size = 1;
context.source = &c;
Expand All @@ -630,6 +643,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_SasUri_is_wrong_fails_2)

static void Blob_UploadMultipleBlocksFromSasUri_various_sizes_happy_path_Impl(HTTP_PROXY_OPTIONS *proxyOptions, const char * networkInterface, const char* certificate, size_t* blockSizesToTest, size_t numberBlockSizesToTest)
{
unsigned int httpResponse = HTTP_OK;
for (size_t iSize = 0; iSize < numberBlockSizesToTest; iSize++)
{
umock_c_reset_all_calls();
Expand Down Expand Up @@ -751,6 +765,7 @@ static void Blob_UploadMultipleBlocksFromSasUri_various_sizes_happy_path_Impl(HT
///assert
ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_OK, result);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);
}


Expand Down Expand Up @@ -792,6 +807,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_various_sizes_happy_path)

TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_64MB_unhappy_paths)
{
unsigned int httpResponse = HTTP_OK;
(void)umock_c_negative_tests_init();

umock_c_reset_all_calls();
Expand Down Expand Up @@ -916,6 +932,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_64MB_unhappy_paths)

///assert
ASSERT_ARE_NOT_EQUAL(BLOB_RESULT, BLOB_OK, result, temp_str);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);
}
}

Expand All @@ -924,8 +941,9 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_64MB_unhappy_paths)

TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_64MB_with_certificate_and_network_interface_unhappy_paths)
{
(void)umock_c_negative_tests_init();
unsigned int httpResponse = HTTP_OK;

(void)umock_c_negative_tests_init();

umock_c_reset_all_calls();
///arrange
Expand Down Expand Up @@ -1052,6 +1070,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_64MB_with_certificate_and_netw

///assert
ASSERT_ARE_NOT_EQUAL(BLOB_RESULT, BLOB_OK, result, temp_str);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);
}
}

Expand All @@ -1061,6 +1080,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_64MB_with_certificate_and_netw
TEST_FUNCTION(Blob_UploadFromSasUri_when_http_code_is_404_it_immediately_succeeds)
{
///arrange
unsigned int httpResponse = HTTP_OK;
umock_c_reset_all_calls();

memset(testUploadToBlobContent, '3', testUploadToBlobContentMaxSize);
Expand Down Expand Up @@ -1130,11 +1150,13 @@ TEST_FUNCTION(Blob_UploadFromSasUri_when_http_code_is_404_it_immediately_succeed
///assert
ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_OK, result);
ASSERT_ARE_EQUAL(int, HTTP_NOT_FOUND, httpResponse);
}

TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockSize_too_big_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = BLOCK_SIZE + 1;
Expand All @@ -1147,6 +1169,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockSize_too_big_fails)

///assert
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_INVALID_ARG, result);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);

///cleanup
gballoc_free(fakeContext.fakeData);
Expand All @@ -1155,6 +1178,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockSize_too_big_fails)
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockSize_is_4MB_succeeds)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = BLOCK_SIZE;
Expand All @@ -1167,6 +1191,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockSize_is_4MB_succeeds

///assert
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_OK, result);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);

///cleanup
gballoc_free(fakeContext.fakeData);
Expand All @@ -1175,6 +1200,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockSize_is_4MB_succeeds
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockCount_is_maximum_succeeds)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = 1;
Expand All @@ -1187,6 +1213,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockCount_is_maximum_suc

///assert
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_OK, result);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);

///cleanup
gballoc_free(fakeContext.fakeData);
Expand All @@ -1195,6 +1222,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockCount_is_maximum_suc
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockCount_is_one_over_maximum_fails)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = 1;
Expand All @@ -1207,6 +1235,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockCount_is_one_over_ma

///assert
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_INVALID_ARG, result);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);

///cleanup
gballoc_free(fakeContext.fakeData);
Expand All @@ -1215,6 +1244,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_when_blockCount_is_one_over_ma
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_returns_BLOB_ABORTED_when_callback_aborts_immediately)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = 1;
Expand All @@ -1235,6 +1265,7 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_returns_BLOB_ABORTED_when_call
TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_returns_BLOB_ABORTED_when_callback_aborts_after_5_blocks)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = 1;
Expand All @@ -1252,4 +1283,26 @@ TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_returns_BLOB_ABORTED_when_call
gballoc_free(fakeContext.fakeData);
}

TEST_FUNCTION(Blob_UploadMultipleBlocksFromSasUri_with_empty_payload)
{
///arrange
unsigned int httpResponse = HTTP_OK;
BLOB_UPLOAD_CONTEXT_FAKE fakeContext;
fakeContext.blockSent = 0;
fakeContext.blockSize = 1;
fakeContext.blocksCount = 0;
fakeContext.fakeData = NULL;
fakeContext.abortOnBlockNumber = 5;

///act
BLOB_RESULT result = Blob_UploadMultipleBlocksFromSasUri("https://h.h/something?a=b", FileUpload_GetFakeData_Callback, &fakeContext, &httpResponse, testValidBufferHandle, NULL, NULL, NULL);

///assert
ASSERT_ARE_EQUAL(BLOB_RESULT, BLOB_OK, result);
ASSERT_ARE_EQUAL(int, HTTP_OK, httpResponse);

///cleanup
gballoc_free(fakeContext.fakeData);
}

END_TEST_SUITE(blob_ut);
Loading

0 comments on commit 19b7457

Please sign in to comment.