Documented issues with HttpClient.authenticate

Retries following `HttpClient.authenticate` does not include the
original request payload, this is an issue as POST, PUT, PATCH
request can be retried through this mechanism. This patch advices
of the issue and suggests using `HttpClient.AddCredentials` directly
or setting the `'authorization'` header manually.

Closes https://github.com/dart-lang/sdk/issues/28012

---

It is possible that we should use even stronger language. It is not immediately obvious
to me when the `HttpClient.authenticate` property is useful. But retrying POST, PUT, PATCH
requests without sending the body again is problematic.

Perhaps we should print a warning when requests with a payload a retries. Or maybe even
deprecate the `HttpClient.authenticate` property. I'm new here so please educate me on
the useful use-cases.

Change-Id: Iaddba39c2fbb08fd31dda77bf19b9856bfa947f3
Reviewed-on: https://dart-review.googlesource.com/69161
Reviewed-by: Martin Kustermann <kustermann@google.com>
This commit is contained in:
Jonas Finnemann Jensen
2018-08-13 12:28:54 +00:00
committed by Jonas Jensen
parent b244c0805d
commit a9178a20fc
+9 -2
View File
@@ -1761,9 +1761,16 @@ abstract class HttpClient {
* [addCredentials] before completing the [Future] with the value
* [:true:].
*
* If the [Future] completes with true the request will be retried
* using the updated credentials. Otherwise response processing will
* If the [Future] completes with [:true:] the request will be retried
* using the updated credentials, however, the retried request will not
* carry the original request payload. Otherwise response processing will
* continue normally.
*
* If it is known that the remote server requires authentication for all
* requests, it is advisable to use [addCredentials] directly, or manually
* set the `'authorization'` header on the request to avoid the overhead
* of a failed request, or issues due to missing request payload on retried
* request.
*/
set authenticate(Future<bool> f(Uri url, String scheme, String realm));