Archisteamfarm: HttpContent gets disposed on redirect

[Vital7]

Bug report

Description

I am trying to make a plugin which sends some MultipartFormDataContent requests, however, when I get a redirect (such as 302) HttpContent gets disposed (by disposing HttpRequestMessage/HttpResponseMessage, if both are disposed - HttpContent gets disposed as well), so any future redirects fail with ObjectDisposedException.

Expected behavior

HttpContent correctly gets to the next redirect without disposing.

Current behavior

HttpContent gets disposed.

Possible solution

Probably dispose request/response later - not sure though, that's why it's an issue, not a pull request.

Steps to reproduce

1. Make UrlPostToHtmlDocument call with data = MultipartFormDataContent to endpoint which gives a redirect (such as OAuth Steam authorization)
2. On the second redirect content gets disposed.

Full log.txt recorded during reproducing the problem

2020-09-11 03:32:17|dotnet-32676|DEBUG|000|InternalRequest() System.Net.Http.HttpRequestException: An error occurred while sending the request.
 ---> System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'System.Net.Http.MultipartFormDataContent'.
   at System.Net.Http.HttpContent.CheckDisposed()
   at System.Net.Http.HttpContent.CopyToAsync(Stream stream, TransportContext context, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnection.SendRequestContentAsync(HttpRequestMessage request, HttpContentWriteStream stream, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnection.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Net.Http.HttpConnection.SendAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithNtConnectionAuthAsync(HttpConnection connection, HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.DecompressionHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.FinishSendAsyncUnbuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
   at ArchiSteamFarm.WebBrowser.InternalRequest[T](Uri requestUri, HttpMethod httpMethod, T data, String referer, HttpCompletionOption httpCompletionOption, Byte maxRedirections, IReadOnlyDictionary`2 additionalHeaders)

[Vital7]

UPD. On 302 status code browsers change request type to GET, should we do it as well?

[JustArchi]

Can you give me a full example call to the WebBrowser that reproduces this issue? I don't want to test it blindly, and this will ensure that it's not userspace doing something wrong.

[Vital7]

Sure:

private async Task<string> ResponseTest() {
    var oauthPage = (await Bot.ArchiWebHandler.UrlPostToHtmlDocumentWithSession(
        ArchiWebHandler.SteamCommunityURL, "/openid/login", new Dictionary<string, string>(6) {
            { "openid.identity", "http://specs.openid.net/auth/2.0/identifier_select" },
            { "openid.claimed_id", "http://specs.openid.net/auth/2.0/identifier_select" },
            { "openid.ns", "http://specs.openid.net/auth/2.0" },
            { "openid.mode", "checkid_setup" },
            { "openid.realm", "https://steamdb.info/" },
            { "openid.return_to", "https://steamdb.info/login/" }
        }, "https://steamdb.info/", session: ArchiWebHandler.ESession.None
    ).ConfigureAwait(false))?.Content;

    if (oauthPage == null) {
        return FormatBotResponse(string.Format(Strings.ErrorIsEmpty, nameof(oauthPage)));
    }

    Dictionary<string, string> oauthParameters = oauthPage.SelectNodes("//form/input").Where(node => !string.IsNullOrEmpty(node.GetAttribute("name"))).ToDictionary(node => node.GetAttribute("name"), node => node.GetAttribute("value"));
    MultipartFormDataContent content = new MultipartFormDataContent();

    foreach ((string key, string value) in oauthParameters) {
        content.Add(new StringContent(value), key);
    }

    var mainPage = await Bot.ArchiWebHandler.WebBrowser.UrlPostToHtmlDocument(ArchiWebHandler.SteamCommunityURL + "/openid/login", content).ConfigureAwait(false);

    if (mainPage?.Content == null) {
        return FormatBotResponse(string.Format(Strings.ErrorIsEmpty, nameof(mainPage)));
    }

    return FormatBotResponse(mainPage.Content.Body.TextContent);
}

[Vital7]

About changing POST to GET:

Note: For historical reasons, a user agent MAY change the request method from POST to GET for the subsequent request. If this behavior is undesired, the 307 (Temporary Redirect) status code can be used instead.

https://tools.ietf.org/html/rfc7231#section-6.4.3

[JustArchi]

Key word above is MAY:

5. MAY   This word, or the adjective "OPTIONAL", mean that an item is
   truly optional.  One vendor may choose to include the item because a
   particular marketplace requires it or because the vendor feels that
   it enhances the product while another vendor may omit the same item.
   An implementation which does not include a particular option MUST be
   prepared to interoperate with another implementation which does
   include the option, though perhaps with reduced functionality. In the
   same vein an implementation which does include a particular option
   MUST be prepared to interoperate with another implementation which
   does not include the option (except, of course, for the feature the
   option provides.)

https://tools.ietf.org/html/rfc2119#section-5

I'll still consider adding it, but not as a bugfix to this issue, but as a potential enhancement ensuring that ASF web browser follows the most popular behaviour. The bug with disposed request will still happen in other redirections codes, and we don't want to hide it instead of fixing it.

[Outzzz]

meet same problem. receive 302 status code and post again

[JustArchi]

I've applied a fix, which will avoid disposed exception when data is provided as HttpContent. This also means that if you're creating a HttpContent yourself then you're in charge of disposing it, so you should wrap your MultipartFormDataContent example above with using. This applies to redirections that do not change POST nature of the request, e.g. 307, but also to timeouts and failures even with the first one.

Additionally, POST resulting in 301 or 302 will be converted to a GET request, just like 303, as per RFC.

[Outzzz]

when the next release available

[JustArchi]

https://github.com/JustArchiNET/ArchiSteamFarm/wiki/Release-cycle

[Outzzz]

in my case third-party api need session and csrf cookies in request header. can you make a public api for that

[JustArchi]

Sure, but create a standalone issue for that, this relates to a bug and is already fixed.