Azure-iot-sdk-csharp: Unable to upload file with HTTP proxy

[motias]

• OS, version, SKU and CPU architecture used: Windows 10 Desktop x64
• Application's .NET Target Framework : netcoreapp2.2
• Device: Laptop (Simulated Device)
• SDK version used: Microsoft.Azure.Devices.Client 1.21.0

Description of the issue:

It is not possible to upload file with proxy by use of DeviceClient.UploadToBlobAsync.
I found a similar question in Stack Overflow, but it has not beed resolved.
Azure IoT Hub, File Upload through proxy?

Code sample exhibiting the issue:

var proxy = new WebProxy("http://proxy.example.com:3128");
var deviceClient = DeviceClient.CreateFromConnectionString("ConnectionStringForIoTHub", new ITransportSettings[]
{
    new AmqpTransportSettings(TransportType.Amqp_WebSocket_Only)
    {
        Proxy = proxy
    }
});

// DeviceTwin update works correctly
await deviceClient.GetTwinAsync().ConfigureAwait(false);
await deviceClient.UpdateReportedPropertiesAsync(new TwinCollection { ["status"] = "connected" }).ConfigureAwait(false); 

// File upload does not work correctly
var filePath = "test.log";
var blobName = "test.log";
using (var stream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
{
    await deviceClient.UploadToBlobAsync(blobName, stream).ConfigureAwait(false); 
}

Console log of the issue:

[bikamani]

@motias What's the behavior you are seeing? Any logs/errors?

[motias]

@bikamani Sorry, no errors and logs.
In my network environment, there are two network route. One is proxy-enabled, and the other is not enabled. I tried to upload file using the former route, but the latter is used (and file upload is succeeded).

It seems to me that the following implementation is incomplete.
The code is very simple and CloudBlockBlob instance is not configured to use a proxy.

https://github.com/Azure/azure-iot-sdk-csharp/blob/a0824e20e1f87dae193e3c951f61772fd29796ce/iothub/device/src/Transport/HttpTransportHandler.cs#L162

I think it is necessary to be implemented as follows:

var client = new CloudBlobClient(new Uri(fileUploadResponse.HostName), 
    new StorageCredentials(fileUploadResponse.SasToken), new ProxyInjectionHandler(proxy));
var container = client.GetContainerReference(fileUploadResponse.ContainerName);
var blob = container.GetBlockBlobReference(fileUploadResponse.BlobName);

private class ProxyInjectionHandler : DelegatingHandler
{
    private readonly IWebProxy _proxy;
    private bool _firstCall = true;

    public ProxyInjectionHandler(IWebProxy proxy)
    {
        _proxy = proxy;
    }

    protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        if (_firstCall)
        {
            if (_proxy != null)
            {
                var handler = (HttpClientHandler) InnerHandler;
                handler.Proxy = _proxy;
                handler.UseProxy = true;
            }
            _firstCall = false;
        }

        return base.SendAsync(request, cancellationToken);
    }
}

[randomstring384540]

DeviceClient ignores all proxy settings when you call UploadToBlobAsync. Is there a reason why the Http1TransportSettings passed to the client are completely ignored?

Rigt now it is impossible to set a proxy for file upload in the code.

See: public Task UploadToBlobAsync(String blobName, System.IO.Stream source)
calls this.internalClient.UploadToBlobAsync(blobName, source)

And in InternalClient.UploadToBlobAsync:

// [added] Why are new transport settings are generated here, even though we passed Http1TransportSettings to client ??
var transportSettings = new Http1TransportSettings();

//We need to add the certificate to the fileUpload httpTransport if DeviceAuthenticationWithX509Certificate
if (this.Certificate != null)
{
   transportSettings.ClientCertificate = this.Certificate;
}

httpTransport = new HttpTransportHandler(context, IotHubConnectionString, transportSettings);

return httpTransport.UploadToBlobAsync(blobName, source, cancellationToken);

[randomstring384540]

Any update?

[timtay-microsoft]

Taking a look at this now, it does appear to be a feature gap. I'll try to get this feature added in soon

[az-iot-builder-01]

@motias, @MkTru, thank you for your contribution to our open-sourced project! Please help us improve by filling out this 2-minute customer satisfaction survey

[timtay-microsoft]

Closing this issue because the fix has been checked in. Our next release will contain this fix

[timtay-microsoft]

Re-opening this issue as the fix I checked in only allows for proxy support when getting the blob uri from iothub. The step in file upload that uses the storage SDK still does not support using a proxy

[timtay-microsoft]

@abhipsaMisra pointed out that using the proxy settings configured to the device client may be confusing, especially for users who send telemetry over amqp/mqtt, but unknowingly are forced to use http for file upload operations. We'll look into other ways of configuring proxies for file upload.

Maybe we could take an instance of HttpTransportSettings in the file upload API itself, so that there is no ambiguity on how settings are used during file upload? Or maybe add a fileUploadTransportSettings object to each of the TransportSettings objects so that users can configure the client level settings for normal operations like sending telemetry, and the file upload operations

[abhipsaMisra]

In order to be able to use proxy settings for uploading files, we need to ensure that the supplied proxy settings are honored for both communicating with IoT hub to retrieve the SAS Uri, and for the actual communication with Azure storage. The version of Azure storage that the device client currently takes a dependency on does not support proxies; however, there are updated versions of the storage SDK that do support proxies. We will look into updating our SDK to refer to the latest version of storage SDK.

As for the question on how the proxy is to be supplied for the IoT Hub connection, and for communicating with the storage service; currently we have the transport information hidden from the user for the upload operation. This was probably designed with the assumption that the http client being used underneath will not require any configuration outside of it's default behavior. This assumption is not correct. We could have the file upload operation take in http transport settings, however, the idea of the device client is that once you establish a connection over a particular protocol, the device client will follow it throughout it's lifetime. By that logic, it is not correct to provide overloads for individual operations that take in transport protocol specific settings.
What we could do instead is, call out that file upload operation is only supported over http, and have it honor the http transport settings that are passed into the device client during instantiation.

[timtay-microsoft]

What we could do instead is, call out that file upload operation is only supported over http, and have it honor the http transport settings that are passed into the device client during instantiation.

We may have customers who already use file upload on a device client configured with AMQP or MQTT transport settings, though. Are you suggesting that clients configured with AMQP transport settings can't use file upload, or are you suggesting that clients configured with AMQP transport settings can only use the default HTTP transport settings for file upload?

The latter feels like an artificial restriction considering we could implement this feature such that users can have AMQP telemetry sending through a proxy, and HTTP file uploads going through another proxy. All it would take is a new configuration for file upload

[abhipsaMisra]

Let us take a step back from implementation and take a look at what a device client is supposed to be. It is supposed to be a way for devices to communicate with IoT hub, over their protocol of choice. To that effect, you create a device client with your authentication credentials and the transport protocol settings, and initialize an instance. This instance now carries those settings, and uses it throughout it's lifetime -> it will refresh it's authentication credentials if provided with a token refresher, or will use the supplied x509 certificates; it will route all of it's requests through the configured proxy server etc. At any point that the client gets disconnected, it will try to reestablish connection to the same transport protocol it was connected to previously.

Now coming to file upload specific implementation:

Are you suggesting that clients configured with AMQP transport settings can't use file upload, or are you suggesting that clients configured with AMQP transport settings can only use the default HTTP transport settings for file upload?

• I agree that we would have customers who would have configured the device client instance over AMQP/MQTT, but might also be using file upload feature. This was previously seamless to the user, since the actual implementation detail was hidden (the fact that file upload was always over an HTTP client, and that HTTP client was not customizable). We will need to look at the usage data and customer use-case to figure out how this API is actually being used.

we could implement this feature such that users can have AMQP telemetry sending through a proxy, and HTTP file uploads going through another proxy. All it would take is a new configuration for file upload

• I agree that this can be implemented quite easily, my point is around if it should be implemented this way. If we start taking in transport protocol specific settings as operation overloads, then tomorrow someone might want to implement a scenario where you have telemetry going through AMQP, twin updates coming though HTTP, and commands over MQTT. This goes against the concept of what the device client is supposed to be.

[timtay-microsoft]

File upload is a special enough case that I have no issue with having separate configurations for it and for the rest of the device client. I don't see twin, methods and telemetry in the same fashion. The fact remains that we have to continue to support file upload for clients that are configured with AMQP/MQTT, and that there is no way to give those clients proxy support without another config.

This may go against the abstraction that the device client was intended to be, but until file upload can be done over AMQP/MQTT we have no choice but to expose to users that file upload is always HTTP, regardless of what they configure. And that is the only reason why we would extend special configurations for it.

[timtay-microsoft]

Customers who need to use AMQP/MQTT for twin should not be encouraged to create another client just to upload files. One client per identity very strong rule, and we don't want customers accidentally sending telemetry from one AMQP client and from one HTTP client

[abhipsaMisra]

This is a special case where this operation is restricted to a single transport protocol, but allowing the operation to specify it's own transport specific settings is something I am not in favor of.
How would we handle disconnection-retry for file upload in this scenario? What about scenarios where you want to specify additional security related configurations (SSL certificate restrictions for TLS negotiation etc.). You would now have to maintain that information in multiple locations.

We need to hash out the design for the file upload operation first, before we can implement proxy support.

[timtay-microsoft]

Our team had an internal sync to bounce around these ideas, and the general solution we have come up with looks like this:

1) Deprecate the existing file upload APIs
2) Add new API for getting SAS URI that allows for an optional http client to be provided by the user (allows for proxy support and other configurations)
3) Make users directly use the Azure Storage SDK to perform the upload itself, after getting the SAS URI
4) Add new API for notifying iothub of the file upload status, again taking an option http client.

The result of this would be that users have finer grain control over the file upload flow, including all http options being configurable. It also has the cost/benefit of allowing customers to use whatever azure storage SDK they want, or even their own azure storage SDK if they are so inclined.

We are very curious about customer feedback on this approach though.

@MkTru @motias do you have any opinions you'd like to share here?

[timtay-microsoft]

Re-opening this issue until we actually release it. The fix is checked in, though

[abhipsaMisra]

Microsoft.Azure.Devices.Client v1.27.0 has been released with this fix.