Re: [PATCH 2/2] block/curl: add support for S3 presigned URLs

[Markus Armbruster <armbru@redhat.com>]

Antoine Damhet <adamhet@scaleway.com> writes:

> S3 presigned URLs are signed for a specific HTTP method (typically GET
> for our use cases). The curl block driver currently issues a HEAD
> request to discover the backend features and the file size, which fails
> with 403.
>
> Add a 'force-range' option that skips the HEAD request and instead
> issues a minimal GET request (querying 1 byte from the server) to
> extract the file size from the 'Content-Range' response header. To
> achieve this the 'curl_header_cb' is redesigned to generically parse
> HTTP headers.
>
> $ $QEMU -drive driver=http,\
>              'url=https://s3.example.com/some.img?X-Amz-Security-Token=XXX',
>              force-range=true
>
> Enabling the 'force-range' option without the backend supporting it is
> undefined behavior and untested

"Undefined behavior" suggests it could do anything, even destroy data.
I hope that's not the case.  What is the case?

What is "the backend"?  The web server specified with @url?

>                                 but the libcurl should ignore the body
> and stop reading after the HTTP headers then we would fail with the
> expected `Server does not support 'range' (byte ranges).` error.
>
> Signed-off-by: Antoine Damhet <adamhet@scaleway.com>
> ---

[...]

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b82af7425614..ff018c2d6bfb 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4582,12 +4582,17 @@
>  # @cookie-secret: ID of a QCryptoSecret object providing the cookie
>  #     data in a secure way.  See @cookie for the format.  (since 2.10)
>  #
> +# @force-range: Don't issue a HEAD HTTP request to discover if the
> +#     backend supports range requests and rely only on GET requests.
> +#     This is especially useful for S3 presigned URLs.  (since 11.0)

Unlike the commit message, this doesn't mention the need for "the
backend" (whatever that may be) supporting it.

> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsCurlHttp',
>    'base': 'BlockdevOptionsCurlBase',
>    'data': { '*cookie': 'str',
> -            '*cookie-secret': 'str'} }
> +            '*cookie-secret': 'str',
> +            '*force-range': 'bool'} }
>  
>  ##
>  # @BlockdevOptionsCurlHttps:
> @@ -4605,13 +4610,18 @@
>  # @cookie-secret: ID of a QCryptoSecret object providing the cookie
>  #     data in a secure way.  See @cookie for the format.  (since 2.10)
>  #
> +# @force-range: Don't issue a HEAD HTTP request to discover if the
> +#     backend supports range requests and rely only on GET requests.
> +#     This is especially useful for S3 presigned URLs.  (since 11.0)
> +#
>  # Since: 2.9
>  ##

@force-range is is duplicated between BlockdevOptionsCurlHttp and
BlockdevOptionsCurlHttps.  @cookie and @cookie-secret is already
duplicated before the patch.  Time to factor out a common base type?

>  { 'struct': 'BlockdevOptionsCurlHttps',
>    'base': 'BlockdevOptionsCurlBase',
>    'data': { '*cookie': 'str',
>              '*sslverify': 'bool',
> -            '*cookie-secret': 'str'} }
> +            '*cookie-secret': 'str',
> +            '*force-range': 'bool'} }
>  
>  ##
>  # @BlockdevOptionsCurlFtp: