Skip to content

WebCmdlets check if absolute uri before checking scheme - #19468

Merged
Dongbo Wang (daxian-dbw) merged 7 commits into
PowerShell:masterfrom
CarloToso:absoluteuri
Apr 12, 2023
Merged

Dongbo Wang (daxian-dbw) merged 7 commits into
PowerShell:masterfrom
CarloToso:absoluteuri

Conversation

@CarloToso

Copy link
Copy Markdown
Contributor

PR Summary

  • Check if absolute uri before checking scheme

History:

PR Context

Fix #19461

PR Checklist

@iSazonov

Copy link
Copy Markdown
Collaborator

Please add new test.

$command = "Invoke-WebRequest -Uri '$uri' -SkipCertificateCheck"

$result = ExecuteWebCommand -command $command
$result.Error.FullyQualifiedErrorId | Should -Be "WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeWebRequestCommand"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Before the fix was it "InsecureRedirection" exception? Could you please explain why now it is WebCmdletWebResponseException?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I introduced "InsecureRedirection" exception in #18595, previously it was WebCmdletWebResponseException.
If the uri has no scheme checking for a scheme would throw an error, so we skip the check and we go back to the old error on https to http redirection

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The issue author has confirmed the fix solves the issue for his scenarios: #19461 (comment)

CarloToso please update the comment as we discussed in #19468 (comment) and then I will merge the PR. Thanks!

@CarloToso

Copy link
Copy Markdown
Contributor Author

Dongbo Wang (@daxian-dbw) done

@daxian-dbw
Dongbo Wang (daxian-dbw) enabled auto-merge (squash) April 12, 2023 00:27
@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 3dd943d into PowerShell:master Apr 12, 2023
@iSazonov Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Apr 12, 2023
@ghost

Copy link
Copy Markdown

🎉v7.4.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

@CarloToso
CarloToso deleted the absoluteuri branch April 20, 2023 18:05
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Insecure Proxy Behavior with Invoke-(RestMethod|WebRequest) on 7.4.0-preview2

4 participants