[C++][FS][Azure] Prevent reading or writing directory marker blobs

[Tom-Newton]

Describe the enhancement requested

Child of #18014

Blob storage doesn't really have directories. Instead we use directory marker blobs to implement directories. Even hierarchical namespace storage accounts, that do have built in directories, implement it using directory markers.

Its possible that a user of the AzureFileSystem might accidentally try to read or write a directory marker which could cause confusion. This scenario should return a clear error message.

Hierarchical namespace storage accounts will add hdi_isFolder=true https://github.com/Azure/azure-sdk-for-cpp/blob/12407e8bfcb9bc1aa43b253c1d0ec93bf795ae3b/sdk/storage/azure-storage-files-datalake/src/datalake_utilities.cpp#L86-L90 and on flat namespace accounts the the AzureFileSystem sets is_directory=true. This would make a cheap check so that we can fail fast.

Component(s)

C++

[Tom-Newton]

take

[Tom-Newton]

A couple of discoveries after starting work on this:

1. On hierarchical namespace accounts Azure will block this and return [OperationNotSupportedOnDirectory] 400 The requested operation is not supported on a directory.. This is good apart from it tends to happen during the Flush call in the destructor and the error is not handled gracefully.
2. On flat namespace accounts we ensure that directory marker blobs always have a trailing / and files never have a trailing /. This prevents accidentally reading or writing a directory marker blob but it also allows creating a file and a directory with exactly the same name, which seems to cause significant problems for GetFileInfo. @felipecrv do you have any thoughts on this?

To be more concrete the new tests that fail are https://github.com/Tom-Newton/arrow/pull/5/files#diff-7d2cbf9c6abf1ee980b8cf5a79e222543c17b3d93b12c4c0c9fa123b1bb200b6

[Tom-Newton]

Maybe we should just call GetFileInfo prior to reading or writing instead of just calling GetProperties directly. My only concern is if that reduces performance. I will need to read the code in more detail.

[felipecrv]

Its possible that a user of the AzureFileSystem might accidentally try to read or write a directory marker which could cause confusion.

Can you clarify the sequence that leads to the confusion? I think we should aim for having no confusion if the user writes to Azure Storage only via the arrow::FileSystem API and fail-fast on confusion created by other writers.

For example, I agree we shouldn't allow creation of files that get the same name as a directory (including implied directories [when a marker doesn't exist, but some blob exists with that directory as a path-prefix]).

On flat namespace accounts we ensure that directory marker blobs always have a trailing / and files never have a trailing /. This prevents accidentally reading or writing a directory marker blob but it also allows creating a file and a directory with exactly the same name, which seems to cause significant problems for GetFileInfo. @felipecrv do you have any thoughts on this?

I think we can figure cheap ways (with at most one extra request) to check against this. Preventing file open with paths containing trailing slashes is a good start because it will catch most of the times this would happen in practice.

[felipecrv]

Maybe we should just call GetFileInfo prior to reading or writing instead of just calling GetProperties directly. My only concern is if that reduces performance. I will need to read the code in more detail.

We might need a lower-level helper: we can get information about the existence of both path and path/ with a single list-blobs with / delimiter -- that would cover implied directories as well.

[felipecrv]

Another optimisation: if you're opening dir0/foo and it already exists, there is nothing to validate. It's only when it's not found, that you need to be careful with the creation of dir0/foo in case dir0/foo/ already exists.

[Tom-Newton]

Its possible that a user of the AzureFileSystem might accidentally try to read or write a directory marker which could cause confusion.

Can you clarify the sequence that leads to the confusion? I think we should aim for having no confusion if the user writes to Azure Storage only via the arrow::FileSystem API and fail-fast on confusion created by other writers.

Imagine the user does this

fs->CreateDir("container/directory");
fs->OpenAppendStream("container/directory");

Currently on flat namespace this will create the case of file and directory with the same path. On hierarchical namespace it fails with an un-recoverable error when the ObjectAppendStream gets destructed. This is because the destructor attempts to Flush and subsequently runs into Azure Error: [OperationNotSupportedOnDirectory] 400 The requested operation is not supported on a directory.