Add -SkipHeaderValidation switch to Invoke-WebRequest and Invoke-RestMethod to support adding headers without validating the header value. - #4085
Conversation
|
You need to rebase and only include the commits for this PR |
|
Rebased. |
There was a problem hiding this comment.
Looks like you accidentally added whitespace
There was a problem hiding this comment.
Will fix
There was a problem hiding this comment.
I updated the file and removed all trailing whitespace per the using-vscode.md to get the file to a 'clean' state.
note that when I diff my version against powershell/master, the only whitespace difference I see is at line 172 where trailing whitespace was removed. I don't see any of the other whitespace differences that the online diff viewer is reporting.
There was a problem hiding this comment.
Local and online diff sometimes surprise us.
Here may be a real difference in 'rn' and ``n`.
There was a problem hiding this comment.
Use the ShouldBeErrorId pattern https://github.com/PowerShell/PowerShell/search?utf8=%E2%9C%93&q=ShouldBeErrorId&type=
There was a problem hiding this comment.
I can't use ShouldBeErrorId for those statements; it expects a ScriptBlock to execute and throw an exception.
There was a problem hiding this comment.
Use the ShouldBeErrorId pattern
There was a problem hiding this comment.
ShouldBeErrorId does not work here; it expects a script block and then catches the exception to verify the error id.
There was a problem hiding this comment.
"echo" doesn't seem to be necessary as "response" basically does the same thing. I would suggest updating "response" if you want json (perhaps based on "contenttype"
There was a problem hiding this comment.
Changing "response" to use JSON for the output breaks many tests. I will update the comment for echo but won't change response at this time.
There was a problem hiding this comment.
I'm not suggesting always return json. "response" takes a contenttype argument you can use.
There was a problem hiding this comment.
I think it makes more sense to have a separate 'echo' test. The returned data for response would be very different; it currently doesn't echo the request, and using contentype to control that doesn't make sense to me.
|
Travis Plunk (@TravisEz13) please merge this PR. |
|
Dan Travison (@dantraMSFT) Please resolve merge conflicts |
…Method to support adding headers without validating the header value.
|
Travis Plunk (@TravisEz13): Conflicts resolved. |
|
Dan Travison (@dantraMSFT) You used an "email address" to commit which is not associated with your GitHub account. Please add the association. |
PR has change
|
Steve Lee (@SteveL-MSFT), please reapprove this PR. |
|
Travis Plunk (@TravisEz13): Can you look at this? Steve is out until next week. |
| $headers = @{"If-Match" = "*"} | ||
| $response = ExecuteRequestWithCustomHeaders -Uri "http://localhost:8080/PowerShell?test=echo" -headers $headers | ||
|
|
||
| $response.Error | Should BeNullOrEmpty |
There was a problem hiding this comment.
We can remove this - next line makes the check.
There was a problem hiding this comment.
The check is intentional. Checking for $result.Error provides a better failure id if the underlying request fails versus the NullReferenceException that would occur in the next statement.
There was a problem hiding this comment.
I believe NullReferenceException would occur only if we call a method but we call a property.
There was a problem hiding this comment.
You're right but the end result does not change. If the request does not complete successfully, I want to know that. Testing the content doesn't help me diagnose that and, in fact, obfuscates it.
There was a problem hiding this comment.
Thanks for clarify!
| $headers = @{"If-Match" = "12345"} | ||
| $response = ExecuteRequestWithCustomHeaders -Uri "http://localhost:8080/PowerShell?test=echo" -headers $headers | ||
|
|
||
| $response.Error | Should Not BeNullOrEmpty |
There was a problem hiding this comment.
The check is intentional. Checking for $result.Error provides a better failure id if the underlying request fails versus the NullReferenceException that would occur in the next statement.
| $headers = @{"If-Match" = "12345"} | ||
| $response = ExecuteRequestWithCustomHeaders -Uri "http://localhost:8080/PowerShell?test=echo" -headers $headers -SkipHeaderValidation | ||
|
|
||
| $response.Error | Should BeNullOrEmpty |
There was a problem hiding this comment.
The check is intentional. Checking for $result.Error provides a better failure id if the underlying request fails versus the NullReferenceException that would occur in the next statement.
| $headers = @{"If-Match" = "*"} | ||
| $response = ExecuteRequestWithCustomHeaders -Uri "http://localhost:8081/PowerShell?test=echo" -headers $headers -Cmdlet "Invoke-RestMethod" | ||
|
|
||
| $response.Error | Should BeNullOrEmpty |
| $headers = @{"If-Match" = "12345"} | ||
| $response = ExecuteRequestWithCustomHeaders -Uri "http://localhost:8081/PowerShell?test=echo" -headers $headers -Cmdlet "Invoke-RestMethod" | ||
|
|
||
| $response.Error | Should Not BeNullOrEmpty |
There was a problem hiding this comment.
The check is intentional. Checking for $result.Error provides a better failure id if the underlying request fails versus the NullReferenceException that would occur in the next statement.
| $headers = @{"If-Match" = "12345"} | ||
| $response = ExecuteRequestWithCustomHeaders -Uri "http://localhost:8081/PowerShell?test=echo" -headers $headers -SkipHeaderValidation -Cmdlet "Invoke-RestMethod" | ||
|
|
||
| $response.Error | Should BeNullOrEmpty |
There was a problem hiding this comment.
The check is intentional. Checking for $result.Error provides a better failure id if the underlying request fails versus the NullReferenceException that would occur in the next statement.
|
Thanks everyone |
…Method to support adding headers without validating the header value. (PowerShell#4085) * Add -SkipHeaderValidation switch to Invoke-WebRequest and Invoke-RestMethod to support adding headers without validating the header value. * Fix whitespace
Fixes #2895
Some sites require header values that do not conform to strict validation in the CoreCLR's HttpRequestMessage.Headers collection causing calls to Invoke-WebRequest and Invoke-RestMethod to fail with a FormatException.
This change adds a -SkipHeaderValidation switch that allows the headers to be added without validation.
See PR MicrosoftDocs/PowerShell-Docs#1387 for the associated doc change.