fsprojects / AzureStorageTypeProvider

An F# Azure Type Provider which can be used to explore Blob, Table and Queue Azure Storage assets and easily apply CRUD operations on them.
http://fsprojects.github.io/AzureStorageTypeProvider/
The Unlicense
84 stars 34 forks source link

Badly formatted byte or string array causes exception on dequeue #78

Closed garrardkitchen closed 8 years ago

garrardkitchen commented 8 years ago

Hi,

On a FormatException, how would I go about deleting that particular message so it no longer gets Dequeued?

Context

If the message format was invalid, this line:

let dequeuedMessage = (queue.Dequeue() |> Async.RunSynchronously).Value 

would raise an error and I'd not be able to get hold of the Id to subsequently delete the message.

Warmest regards,

Garrard.

isaacabraham commented 8 years ago

Hmmm. Under what circumstances is this FormatException raised? You should normally be able to get to the Queue Message (if not necessarily the body) - you could then get the ID to delete the message.

It seems that this isn't the case though, so sounds like an error in the mapping from the Azure SDK to the TP domain. Can you send the full stack trace through as this might be a bug?

In the interim you can always get a hold of the CloudQueueClient (using the AsCloudQueue method) which falls back to the Azure SDK.

garrardkitchen commented 8 years ago

Hi Isaac,

I've just entered garbage via the Azure Storage Explorer to explorer an edge case. I know one would normally use an API call but I wanted to test what would go wrong and how I would have to code defensively for this eventuality.

Here's the stack trace:

   at System.Convert.FromBase64_Decode(Char* startInputPtr, Int32 inputLength, Byte* startDestPtr, Int32 destLength)
   at System.Convert.FromBase64CharPtr(Char* inputPtr, Int32 inputLength)
   at System.Convert.FromBase64String(String s)
   at Microsoft.WindowsAzure.Storage.Queue.CloudQueueMessage.get_AsBytes() in c:\Program Files (x86)\Jenkins\workspace\release_dotnet_master\Lib\Common\Queue\CloudQueueMessage.Common.cs:line 146
   at FSharp.Azure.StorageTypeProvider.Queue.Factory.toProvidedQueueMessage(CloudQueueMessage message)
   at <StartupCode$FSharp-Azure-StorageTypeProvider>.$ProvidedQueueTypes.Dequeue@79-1.Invoke(CloudQueueMessage _arg1)
   at Microsoft.FSharp.Control.AsyncBuilderImpl.args@835-1.Invoke(a a)
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.FSharp.Control.AsyncBuilderImpl.commit[a](Result`1 res)
   at Microsoft.FSharp.Control.CancellationTokenOps.RunSynchronously[a](CancellationToken token, FSharpAsync`1 computation, FSharpOption`1 timeout)
   at Microsoft.FSharp.Control.FSharpAsync.RunSynchronously[T](FSharpAsync`1 computation, FSharpOption`1 timeout, FSharpOption`1 cancellationToken)
   at AzureLibrary.Queue.Listener(IActorRef worker, Commands message) in F:\code\SAaaS\AzureLibrary\Queue.fs:line 30

Thanks for the advice on using AsCloudQueue method. I will look into this.

Warmest regards,

Garrard.

isaacabraham commented 8 years ago

OK, I've identified where I think this occurs (https://github.com/fsprojects/AzureStorageTypeProvider/blob/master/src/FSharp.Azure.StorageTypeProvider/Queue/ProvidedQueueTypes.fs#L44)

Basically the properties AsBytes and AsString need to be lazily evaluated - currently they are being eagerly set whenever a message is dequeued. I'll push out a fix.

isaacabraham commented 8 years ago

@garrardkitchen I think I have a fix, but can't repro - can you give me a string queue value to reproduce this?

garrardkitchen commented 8 years ago

I appreciate your rapid resolution to this. Thanks for the added information too.

I'm currently switching between two envs at the moment, windows & mac. The mac version of Azure Storage Explorer converts entered text to Base64 but the windows version doesn't. If it wasn't for this then I'd never had bothered you. <- This is well timed, just finished typing this paragraph and saw your entry from a few moments ago! Any text entered, e.g. 'qqwdwwef' into the Azure Storage Explorer (Windows 10 edition) will trigger the FormatException.

Warmest regards,

Garrard.

isaacabraham commented 8 years ago

@garrardkitchen is it possible to repro this somehow by putting e.g. a byte array that can't become a string (or vice versa) via the API? I'd love to have a reproduceable test case for this.

isaacabraham commented 8 years ago

FYI - my intention for a "fix" is to make both AsString and AsBytes lazy fields - I'll submit a PR shortly - have a look and let me know your thoughts.

garrardkitchen commented 8 years ago

Hi @isaacabraham,

I never got it to fail via the API, just by entering garbage via the Azure Storage Explorer then attempting the dequeue via your TP. I've a meeting first thing but I will look at enqueuing a byte array that can't become a string (or vice versa) after that.

Warmest regards,

Garrard.

isaacabraham commented 8 years ago

Can't repro this from latest Storage Explorer on Win 10 (version 0.8.2) :-(

garrardkitchen commented 8 years ago

Hi @isaacabraham,

I've just installed version 0.8.2 on windows (was using 0.8.0) and like you, no issue. With versions leading up to 0.8.2 I had to encode message to Base64 myself and paste into Azure Storage Explorer.

Warmest regards,

Garrard.

isaacabraham commented 8 years ago

OK - so I'm not entirely sure whether to go ahead with this fix or not. On the one hand, perhaps these properties should be lazily evaluated as that's what the underlying SDK does. On the other hand, it's a breaking change.

isaacabraham commented 8 years ago

I'm going to add this - I suppose what you've proved is that someone could conceivably use another API, or go straight to the REST API, and put bad data in. We should never be in a position where we've dequeued a message but can't safely get to the ID of it etc.

isaacabraham commented 8 years ago

@garrardkitchen Do you still have the older version of the storage explorer - or can somehow reproduce this manually? If so, you could at least build the TP from source (off the pull request branch) and prove that it works correctly?

WilliamOckham commented 8 years ago

I caused this problem for myself by adding messages using the .net SDK and using the REST API, and finally changing CloudQueue.EncodeMessage to False, and then dequeuing the messages. Unfortunately, to unblock myself, I cleared the queue. I'll see if I can put together a repro.

garrardkitchen commented 8 years ago

Hi @isaacabraham, do you still need me to locate old version and test against build?

Warmest regards,

Garrard

isaacabraham commented 8 years ago

@garrardkitchen if it's a pain to get hold of the data, never mind - I'll merge in the PR anyway. It would have been nice if I could have somehow reproed this before putting in the fix, but it's not essential.

garrardkitchen commented 8 years ago

Hi @isaacabraham, leave it with me. I'll get back to you a little later.

Warmest regards,

Garrard.

WilliamOckham commented 8 years ago

I have a repro case for this, if you are still interested. All that is required is to add a message to the queue using REST API

isaacabraham commented 8 years ago

Yes please - especially if that works for the emulator.


From: WilliamOckhammailto:notifications@github.com Sent: ‎23/‎07/‎2016 18:57 To: fsprojects/AzureStorageTypeProvidermailto:AzureStorageTypeProvider@noreply.github.com Cc: Isaac Abrahammailto:isaac.abraham@gmail.com; State changemailto:state_change@noreply.github.com Subject: Re: [fsprojects/AzureStorageTypeProvider] Badly formatted byte or string array causes exception on dequeue (#78)

I have a repro case for this, if you are still interested. All that is required is to add a message to the queue using REST API


You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/fsprojects/AzureStorageTypeProvider/issues/78#issuecomment-234728517

WilliamOckham commented 8 years ago

I actually ran in to this error on a real project. Here's the minimal possible code that works in the emulator:

open FSharp.Azure.StorageTypeProvider
open System.Net
open System.Text

type Local = AzureTypeProvider<"DevStorageAccount","">

let sas = Local.Queues.myqueue.GenerateSharedAccessSignature(System.TimeSpan.FromDays(7.0))

//Create a broken message to send to the queue. The queue system expects "Test" to be a Base64 encoded string.
let requestBytes = Encoding.UTF8.GetBytes(@"<QueueMessage>
<MessageText>Test</MessageText>
</QueueMessage>")

let length = requestBytes.Length
//This block of code sets up an HTTP call to the Azure Storage REST API
let req = WebRequest.Create("http://127.0.0.1:10001/devstoreaccount1/myqueue/messages" + sas + "&visibilityTimeout=30"  ) :?> HttpWebRequest
req.Method <- "POST"
req.ContentLength <- int64 (length)
req.GetRequestStream().Write(requestBytes, 0, length)
//This executes the HTTP call and successfully puts the message on the queue. There's no need to handle the response.
use resp = req.GetResponse() :?> HttpWebResponse

// Before the fix, the line below would error:
let msg = Local.Queues.myqueue.Dequeue() |> Async.RunSynchronously