Skip to content

Commit

Permalink
Fix bug: 416 RangeNotSatisfiable exception is swallowed unintentionally
Browse files Browse the repository at this point in the history
  • Loading branch information
Jinming-Hu authored and vinjiang committed Apr 28, 2020
1 parent d669f5d commit c1fdf3a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
13 changes: 10 additions & 3 deletions Microsoft.WindowsAzure.Storage/src/cloud_blob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,15 @@ namespace azure { namespace storage {
single_blob_download_threshold = protocol::default_single_block_download_threshold;
}

// We use this variable to track if we are trying to download the whole blob or just a range.
// In the former case, no exception should be thrown if the blob is empty.
// In the latter case, exception should be thrown.
// And the behavior should be consistent no matter we're downloading with multiple or single thread.
bool no_throw_on_empty = false;

if (offset >= std::numeric_limits<utility::size64_t>::max())
{
no_throw_on_empty = true;
if (length == 0)
{
offset = 0;
Expand All @@ -690,9 +697,9 @@ namespace azure { namespace storage {
}
catch (storage_exception &e)
{
// For empty blob, swallow the exception and update the attributes.
if (e.result().http_status_code() == web::http::status_codes::RangeNotSatisfiable
&& offset == 0)
// If offset equals to 0 and HTTP status code is 416 RangeNotSatisfiable, then this is an empty blob.
// For empty blob, update the attributes or throw an exception.
if (e.result().http_status_code() == web::http::status_codes::RangeNotSatisfiable && offset == 0 && no_throw_on_empty)
{
return instance->download_attributes_async_impl(condition, options, context, timer_handler->get_cancellation_token(), false, timer_handler);
}
Expand Down
41 changes: 41 additions & 0 deletions Microsoft.WindowsAzure.Storage/tests/cloud_blob_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,47 @@ SUITE(Blob)
CHECK(blob.properties().size() == target_length);
}

TEST_FIXTURE(blob_test_base, range_not_satisfiable_exception)
{
auto blob_name = get_random_string(20);
auto blob = m_container.get_block_blob_reference(blob_name);
blob.upload_text(utility::string_t());

auto blob2 = m_container.get_block_blob_reference(blob_name + _XPLATSTR("2"));
blob2.upload_text(_XPLATSTR("abcd"));

azure::storage::blob_request_options options1;
options1.set_parallelism_factor(1);
options1.set_use_transactional_crc64(false);

azure::storage::blob_request_options options2;
options2.set_parallelism_factor(2);
options2.set_use_transactional_crc64(false);

azure::storage::blob_request_options options3;
options3.set_parallelism_factor(1);
options3.set_use_transactional_crc64(true);

for (const auto& option : { options1, options2, options3 }) {
concurrency::streams::container_buffer<std::vector<uint8_t>> download_buffer;

// download whole blob, no exception
blob.download_to_stream(download_buffer.create_ostream(), azure::storage::access_condition(), option, azure::storage::operation_context());

// download range, should throw
CHECK_THROW(blob.download_range_to_stream(download_buffer.create_ostream(), 0, 100, azure::storage::access_condition(), option, azure::storage::operation_context()), azure::storage::storage_exception);

// download range(max, ...), no exception
blob.download_range_to_stream(download_buffer.create_ostream(), std::numeric_limits<utility::size64_t>::max(), 0, azure::storage::access_condition(), option, azure::storage::operation_context());

// download range(3, very large), no exception
blob2.download_range_to_stream(download_buffer.create_ostream(), 3, 100, azure::storage::access_condition(), option, azure::storage::operation_context());

// download range(4, ...), should throw
CHECK_THROW(blob2.download_range_to_stream(download_buffer.create_ostream(), 4, 100, azure::storage::access_condition(), option, azure::storage::operation_context()), azure::storage::storage_exception);
}
}

TEST_FIXTURE(blob_test_base, read_blob_with_invalid_if_none_match)
{
auto blob_name = get_random_string(20);
Expand Down

0 comments on commit c1fdf3a

Please sign in to comment.