Skip to content

Enable HTTP persistence when using Session with Invoke-WebRequest and Invoke-RestMethod #19173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stevenebutler wants to merge 28 commits into from
Closed

Enable HTTP persistence when using Session with Invoke-WebRequest and Invoke-RestMethod #19173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a7f576e
Enable HTTP persistence when using Session
stevenebutler Feb 18, 2023
c5addae
Address codefactor issues
stevenebutler Feb 18, 2023
ac534ef
Merge branch 'PowerShell:master' into 12764-support-persistent-http-c…
stevenebutler Feb 18, 2023
c722d2c
Address review comments by @CarloToso
stevenebutler Feb 18, 2023
08e96a1
Make -NoProxy and -Proxy switches flip each other's states
stevenebutler Feb 19, 2023
9dda627
Address codefactor issues
stevenebutler Feb 19, 2023
7797a91
Address @iSazanov review comments and suggestions
stevenebutler Feb 25, 2023
e104a79
Address codefactor issues
stevenebutler Feb 25, 2023
882be3c
Address more codefactor issues
stevenebutler Feb 25, 2023
f9e4e65
Tidy up comment
stevenebutler Feb 25, 2023
5f1c8d8
Merge branch 'PowerShell:master' into 12764-support-persistent-http-c…
stevenebutler Feb 25, 2023
bdc1a2d
Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Web…
stevenebutler Feb 25, 2023
49f2d8f
Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Web…
stevenebutler Feb 25, 2023
6b9578c
Address review comments
stevenebutler Feb 25, 2023
f448308
Remove unnecessary MaximumRedirection property
stevenebutler Feb 25, 2023
1c122b8
Simplify logic around whether httpClient was disposed
stevenebutler Feb 25, 2023
646aa79
Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Web…
stevenebutler Feb 25, 2023
1c7c0fd
Implements IDisposable to dispose WebSession
stevenebutler Feb 25, 2023
b69fe72
Move TimeoutSec handling to WebRequestSession
stevenebutler Feb 25, 2023
18eb46e
Clean up variable naming in persistence test cases
stevenebutler Feb 26, 2023
6bb5283
Combine if expressions in dispose
stevenebutler Feb 26, 2023
7ae5471
Remove indentation
stevenebutler Feb 27, 2023
b7a5aff
Merge indentation change from master
stevenebutler Feb 27, 2023
2aee3d8
Apply suggestions from code review
stevenebutler Feb 28, 2023
d747ea2
Address review comments by @iSazonov
stevenebutler Feb 28, 2023
237aaf7
codefactor fix
stevenebutler Mar 1, 2023
14487d4
Make parameter name clearer as per review suggestion
stevenebutler Mar 1, 2023
9f73991
Rename function parameter to suppressHttpClientRedirects
stevenebutler Mar 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 107 additions & 82 deletions ...owerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public enum WebSslProtocol
/// <summary>
/// Base class for Invoke-RestMethod and Invoke-WebRequest commands.
/// </summary>
public abstract class WebRequestPSCmdlet : PSCmdlet
public abstract class WebRequestPSCmdlet : PSCmdlet, IDisposable
{
#region Fields

Expand All @@ -107,6 +107,13 @@ public abstract class WebRequestPSCmdlet : PSCmdlet
/// </summary>
internal int _maximumFollowRelLink = int.MaxValue;

/// <summary>
/// Maximum number of Redirects to follow, initialized from WebSession.MaximumRedirection,
/// and decremented as redirects occur so that the setting in WebSession is not altered
/// between persistent requests.
/// </summary>
internal int _maximumRedirection;

/// <summary>
/// Parse Rel Links.
/// </summary>
Expand All @@ -127,6 +134,12 @@ public abstract class WebRequestPSCmdlet : PSCmdlet
/// </summary>
private bool _resumeSuccess = false;

/// <summary>
/// True if the Dispose() method has already been called to cleanup Disposable fields,
/// as per IDisposable pattern (https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose).
/// </summary>
private bool _disposed = false;

#endregion Fields

#region Virtual Properties
Expand Down Expand Up @@ -517,8 +530,7 @@ protected override void ProcessRecord()

bool handleRedirect = keepAuthorizationOnRedirect || AllowInsecureRedirect || PreserveHttpMethodOnRedirect;

using HttpClient client = GetHttpClient(handleRedirect);

HttpClient client = GetHttpClient(handleRedirect);
int followedRelLink = 0;
Uri uri = Uri;
do
Expand Down Expand Up @@ -549,6 +561,8 @@ protected override void ProcessRecord()

WriteVerbose(reqVerboseMsg);

_maximumRedirection = WebSession.MaximumRedirection;

using HttpResponseMessage response = GetResponse(client, request, handleRedirect);

string contentType = ContentHelper.GetContentType(response);
Expand Down Expand Up @@ -578,15 +592,15 @@ protected override void ProcessRecord()
// Disable writing to the OutFile.
OutFile = null;
}

// Detect insecure redirection
if (!AllowInsecureRedirect && response.RequestMessage.RequestUri.Scheme == "https" && response.Headers.Location?.Scheme == "http")
{
ErrorRecord er = new(new InvalidOperationException(), "InsecureRedirection", ErrorCategory.InvalidOperation, request);
er.ErrorDetails = new ErrorDetails(WebCmdletStrings.InsecureRedirection);
ThrowTerminatingError(er);
}

if (ShouldCheckHttpStatus && !_isSuccess)
{
string message = string.Format(
Expand Down Expand Up @@ -633,7 +647,7 @@ protected override void ProcessRecord()
// Errors with redirection counts of greater than 0 are handled automatically by .NET, but are
// impossible to detect programmatically when we hit this limit. By handling this ourselves
// (and still writing out the result), users can debug actual HTTP redirect problems.
if (WebSession.MaximumRedirection == 0 && IsRedirectCode(response.StatusCode))
if (_maximumRedirection == 0 && IsRedirectCode(response.StatusCode))
{
ErrorRecord er = new(new InvalidOperationException(), "MaximumRedirectExceeded", ErrorCategory.InvalidOperation, request);
er.ErrorDetails = new ErrorDetails(WebCmdletStrings.MaximumRedirectionCountExceeded);
Expand Down Expand Up @@ -682,6 +696,33 @@ protected override void ProcessRecord()
/// </summary>
protected override void StopProcessing() => _cancelToken?.Cancel();

/// <summary>
/// Disposes the associated WebSession if it is not being used as part of a persistent session.
/// </summary>
/// <param name="disposing">True when called from Dispose() and false when called from finalizer.</param>
protected virtual void Dispose(bool disposing)
{
if (!_disposed)
{
if (disposing && !IsPersistentSession())
{
WebSession?.Dispose();
WebSession = null;
}

_disposed = true;
}
}

/// <summary>
/// Disposes the associated WebSession if it is not being used as part of a persistent session.
/// </summary>
public void Dispose()
{
Dispose(disposing: true);
GC.SuppressFinalize(this);
}

#endregion Overrides

#region Virtual Methods
Expand Down Expand Up @@ -894,29 +935,55 @@ internal virtual void PrepareSession()
WebSession.UserAgent = UserAgent;
}

if (Proxy is not null)
// Proxy and NoProxy parameters are mutually exclusive.
// If NoProxy is provided, WebSession will turn off the proxy
// and if Proxy is provided NoProxy will be turned off.
if (NoProxy.IsPresent)
{
WebProxy webProxy = new(Proxy);
webProxy.BypassProxyOnLocal = false;
if (ProxyCredential is not null)
{
webProxy.Credentials = ProxyCredential.GetNetworkCredential();
}
else if (ProxyUseDefaultCredentials)
WebSession.NoProxy = true;
}
else
{
if (Proxy is not null)
{
// If both ProxyCredential and ProxyUseDefaultCredentials are passed,
// UseDefaultCredentials will overwrite the supplied credentials.
webProxy.UseDefaultCredentials = true;
WebProxy webProxy = new(Proxy);
webProxy.BypassProxyOnLocal = false;
if (ProxyCredential is not null)
{
webProxy.Credentials = ProxyCredential.GetNetworkCredential();
}
else if (ProxyUseDefaultCredentials)
{
// If both ProxyCredential and ProxyUseDefaultCredentials are passed,
// UseDefaultCredentials will overwrite the supplied credentials.
webProxy.UseDefaultCredentials = true;
Copy link
Collaborator

@iSazonov iSazonov Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is not correct since the code is in else branch.

Copy link
Contributor Author

@stevenebutler stevenebutler Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. However this is what was there before, which makes me wonder if it's a bug, or just a bad comment?

Should we fix the comment or the code?

Copy link
Collaborator

@iSazonov iSazonov Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CarloToso Could you please look this?

}

// We don't want to update the WebSession unless the proxies are different
// as that will require us to create a new HttpClientHandler and lose connection
// persistence.
if (!webProxy.Equals(WebSession.Proxy))
{
WebSession.Proxy = webProxy;
}
}
}

WebSession.Proxy = webProxy;
if (MyInvocation.BoundParameters.ContainsKey(nameof(SslProtocol)))
{
// SslProtocol parameter is a value type so we only want to switch back to the default
// if it is explicitly provided on the command line. Otherwise we keep the value in
// the WebSession from any previous invocation.
WebSession.SslProtocol = SslProtocol;
}

if (MaximumRedirection > -1)
{
WebSession.MaximumRedirection = MaximumRedirection;
}

WebSession.SkipCertificateCheck = SkipCertificateCheck.IsPresent;

// Store the other supplied headers
if (Headers is not null)
{
Expand All @@ -928,7 +995,7 @@ internal virtual void PrepareSession()
// We silently ignore header if value is null.
if (value is not null)
{
// Add the header value (or overwrite it if already present)
// Add the header value (or overwrite it if already present).
WebSession.Headers[key] = value.ToString();
}
}
Expand All @@ -941,63 +1008,20 @@ internal virtual void PrepareSession()
// Only set retry interval if retry count is set.
WebSession.RetryIntervalInSeconds = RetryIntervalSec;
}

WebSession.TimeoutSec = TimeoutSec;
}

internal virtual HttpClient GetHttpClient(bool handleRedirect)
{
HttpClientHandler handler = new();
handler.CookieContainer = WebSession.Cookies;
handler.AutomaticDecompression = DecompressionMethods.All;

// Set the credentials used by this request
if (WebSession.UseDefaultCredentials)
{
// The UseDefaultCredentials flag overrides other supplied credentials
handler.UseDefaultCredentials = true;
}
else if (WebSession.Credentials is not null)
{
handler.Credentials = WebSession.Credentials;
}

if (NoProxy)
{
handler.UseProxy = false;
}
else if (WebSession.Proxy is not null)
{
handler.Proxy = WebSession.Proxy;
}

if (WebSession.Certificates is not null)
{
handler.ClientCertificates.AddRange(WebSession.Certificates);
}
HttpClient client = WebSession.GetHttpClient(handleRedirect, out bool clientWasReset);

if (SkipCertificateCheck)
if (clientWasReset)
{
handler.ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator;
handler.ClientCertificateOptions = ClientCertificateOption.Manual;
WriteVerbose(WebCmdletStrings.WebSessionConnectionRecreated);
}

// This indicates GetResponse will handle redirects.
if (handleRedirect || WebSession.MaximumRedirection == 0)
{
handler.AllowAutoRedirect = false;
}
else if (WebSession.MaximumRedirection > 0)
{
handler.MaxAutomaticRedirections = WebSession.MaximumRedirection;
}

handler.SslProtocols = (SslProtocols)SslProtocol;

HttpClient httpClient = new(handler);

// Check timeout setting (in seconds instead of milliseconds as in HttpWebRequest)
httpClient.Timeout = TimeoutSec is 0 ? TimeSpan.FromMilliseconds(Timeout.Infinite) : new TimeSpan(0, 0, TimeoutSec);

return httpClient;
return client;
}

internal virtual HttpRequestMessage GetRequest(Uri uri)
Expand Down Expand Up @@ -1230,17 +1254,17 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
response = client.SendAsync(currentRequest, HttpCompletionOption.ResponseHeadersRead, _cancelToken.Token).GetAwaiter().GetResult();

if (handleRedirect
&& WebSession.MaximumRedirection is not 0
&& _maximumRedirection is not 0
&& IsRedirectCode(response.StatusCode)
&& response.Headers.Location is not null)
{
_cancelToken.Cancel();
_cancelToken = null;

// If explicit count was provided, reduce it for this redirection.
if (WebSession.MaximumRedirection > 0)
if (_maximumRedirection > 0)
{
WebSession.MaximumRedirection--;
_maximumRedirection--;
}

// For selected redirects, GET must be used with the redirected Location.
Expand Down Expand Up @@ -1286,7 +1310,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
requestWithoutRange.Version,
requestWithoutRange.Method,
requestContentLength);

WriteVerbose(reqVerboseMsg);

response.Dispose();
Expand All @@ -1303,9 +1327,9 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM

// If the status code is 429 get the retry interval from the Headers.
// Ignore broken header and its value.
if (response.StatusCode is HttpStatusCode.Conflict && response.Headers.TryGetValues(HttpKnownHeaderNames.RetryAfter, out IEnumerable<string> retryAfter))
if (response.StatusCode is HttpStatusCode.Conflict && response.Headers.TryGetValues(HttpKnownHeaderNames.RetryAfter, out IEnumerable<string> retryAfter))
{
try
try
{
IEnumerator<string> enumerator = retryAfter.GetEnumerator();
if (enumerator.MoveNext())
Expand All @@ -1318,7 +1342,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM
// Ignore broken header.
}
}

string retryMessage = string.Format(
CultureInfo.CurrentCulture,
WebCmdletStrings.RetryVerboseMsg,
Expand Down Expand Up @@ -1388,7 +1412,7 @@ private static Uri CheckProtocol(Uri uri)
}

private string QualifyFilePath(string path) => PathUtils.ResolveFilePath(filePath: path, command: this, isLiteralPath: true);

private static string FormatDictionary(IDictionary content)
{
ArgumentNullException.ThrowIfNull(content);
Expand Down Expand Up @@ -1455,6 +1479,8 @@ private void ProcessAuthentication()
}
}

private bool IsPersistentSession() => MyInvocation.BoundParameters.ContainsKey(nameof(WebSession)) || MyInvocation.BoundParameters.ContainsKey(nameof(SessionVariable));

/// <summary>
/// Sets the ContentLength property of the request and writes the specified content to the request's RequestStream.
/// </summary>
Expand Down Expand Up @@ -1485,7 +1511,7 @@ internal void SetRequestContent(HttpRequestMessage request, string content)
{
ArgumentNullException.ThrowIfNull(request);
ArgumentNullException.ThrowIfNull(content);

Encoding encoding = null;
if (ContentType is not null)
{
Expand Down Expand Up @@ -1565,7 +1591,7 @@ internal void SetRequestContent(HttpRequestMessage request, MultipartFormDataCon
{
ArgumentNullException.ThrowIfNull(request);
ArgumentNullException.ThrowIfNull(multipartContent);

// Content headers will be set by MultipartFormDataContent which will throw unless we clear them first
WebSession.ContentHeaders.Clear();

Expand Down Expand Up @@ -1680,7 +1706,6 @@ private void AddMultipartContent(object fieldName, object fieldValue, MultipartF
private static StringContent GetMultipartStringContent(object fieldName, object fieldValue)
{
ContentDispositionHeaderValue contentDisposition = new("form-data");

// .NET does not enclose field names in quotes, however, modern browsers and curl do.
contentDisposition.Name = "\"" + LanguagePrimitives.ConvertTo<string>(fieldName) + "\"";

Expand Down Expand Up @@ -1735,7 +1760,8 @@ private static string FormatErrorMessage(string error, string contentType)
XmlDocument doc = new();
doc.LoadXml(error);

XmlWriterSettings settings = new XmlWriterSettings {
XmlWriterSettings settings = new XmlWriterSettings
{
Indent = true,
NewLineOnAttributes = true,
OmitXmlDeclaration = true
Expand Down Expand Up @@ -1766,7 +1792,6 @@ private static string FormatErrorMessage(string error, string contentType)
{
// Ignore errors
}

if (string.IsNullOrEmpty(formattedError))
{
// Remove HTML tags making it easier to read
Expand Down
Loading