powershellwrapper icon indicating copy to clipboard operation
powershellwrapper copied to clipboard

Invoke-RestMethod charset not utf8 by default

Open ecspresso opened this issue 6 years ago • 18 comments

Invoke-RestMethod does not seem to have a charset specified and either PowerShell or ITGlue defaults to ISO-8859-1. (RFC). Where does not matter but the result is special characters does not work.

A fix to this is appending -ContentType 'application/vnd.api+json; charset=utf-8' to every POST.

ecspresso avatar Apr 11 '18 13:04 ecspresso

Looked at your code, just changing ITGlueAPI.psm1 to this would solve it.

$ITGlue_Headers = New-Object "System.Collections.Generic.Dictionary[[String],[String]]"   
$ITGlue_Headers.Add("Content-Type", 'application/vnd.api+json; charset=utf-8') 

Set-Variable -Name "ITGlue_Headers"  -Value $ITGlue_Headers -Scope global


Import-ITGlueModuleSettings

ecspresso avatar Apr 11 '18 15:04 ecspresso

Created pull request.

ecspresso avatar Apr 12 '18 06:04 ecspresso

I've heard back from IT Glue support and this issue has been fixed. I did some testing and my result is the following:

# not working
$headers = @{
    "x-api-key" = "ITG...."
    "Content-Type" = "application/vnd.api+json; charset=utf-8"
}
Invoke-RestMethod -$header headers

# working
$headers = @{
    "x-api-key" = "ITG...."
}
Invoke-RestMethod -$header headers -Content-Type "application/vnd.api+json; charset=utf-8"

Not sure why this is the case, my guess is that PowerShell parses -Content-Type but not -Headers. This means that my second post on this issue would not solve the problem.

We could, instead of adding -Content-Type to all calls, specify a hash variable somewhere, accessible to all functions which contains all static parameters as needed, such as -Content-Type, in the following way:

$parameters = @{
    'Content-Type' = 'application/vnd.api+json; charset=utf-8'
}

This could then be spliced:

Invoke-RestMethod -body $body -uri $URI -method $method @parameters

The upside of this way is that is makes it easier to add anything static in the future.

ecspresso avatar Jan 22 '19 10:01 ecspresso

Here is minified code the code to illustrate to problem. Replace $organization_id something valid, the rest will import if you have wrapper.

$tmp_config = Import-LocalizedData -BaseDirectory "$($env:USERPROFILE)\ITGlueAPI" -FileName "config.psd1"
$API_KEY = [System.Management.Automation.PSCredential]::New('null', $(Get-ITGlueAPIKey)).GetNetworkCredential().Password
$URI = "$(Get-ITGlueBaseURI)/contacts"
# update this
$organization_id = 2504761

# 1 (working)
$body = ConvertTo-Json (@{
    data = @{
        type = 'contacts'
        attributes = @{
            organization_id = $organization_id
            first_name = "Example 1 (working)"
            last_name = "ÉûïÛÅÄÖ"
            notes = "user test"
        }
    }
})

$header_example1 = @{
    "x-api-key" = $API_KEY
}

(Invoke-RestMethod -method 'POST' -uri $URI -headers $header_example1 -body $body -ContentType "application/vnd.api+json; charset=utf-8").data.attributes.name

# 2 (not working)
$body = ConvertTo-Json (@{
    data = @{
        type = 'contacts'
        attributes = @{
            organization_id = $organization_id
            first_name = "Example 2 (not working)"
            last_name = "ÉûïÛÅÄÖ"
            notes = "user test"
        }
    }
})
$header_example2 = @{
    "x-api-key" = $API_KEY
    "Content-Type" = "application/vnd.api+json; charset=utf-8"
}
(Invoke-RestMethod -method 'POST' -uri $URI -headers $header_example2 -body $body).data.attributes.name

# 3 (not working)
$body = ConvertTo-Json (@{
    data = @{
        type = 'contacts'
        attributes = @{
            organization_id = $organization_id
            first_name = "Example 3 (not working)"
            last_name = "ÉûïÛÅÄÖ"
            notes = "user test"
        }
    }
})
$header_example3 = @{
    "x-api-key" = $API_KEY
    "Content-Type" = "application/vnd.api+json"
}
(Invoke-RestMethod -method 'POST' -uri $URI -headers $header_example3 -body $body).data.attributes.name

Edit: fixed the url, forgot "/contacts",

ecspresso avatar Jan 24 '19 12:01 ecspresso

@ecspresso thanks for the follow up on this. The suggestion makes sense as I understand this will provide support for special characters within this module. I'd suggest moving forward with this and creating a PR for it. @CalebAlbers do you have additional thoughts?

adrianwells avatar Jan 28 '19 16:01 adrianwells

I definitely agree - utilizing a global hash with character encoding and related parameters would be good. We can have it initialized in the Module Settings section, much like the $ITGlue_JSON_Conversion_Depth variable.

Thanks for digging in to this, @ecspresso!

CalebAlbers avatar Jan 28 '19 18:01 CalebAlbers

Hey @ecspresso! I just had an interesting discussion with IT Glue QA about this (and I'm about 110% sure you're the partner they were referring to that started the discussion 😉 ).

Any way - I did some troubleshooting on this and came across some potential situations in which appending the ; charset=utf-8 to the Content-Type string would lead to issues. Specifically, ConvertTo-Json function doesn't respect the encoding that is passed to it. That's why you can toss something that is encoded in UTF-8 to it and get garbage out. Now, the ; charset=utf-8 works on Windows, but for some reason the Invoke-RestMethod function on PS Core 6.x for Mac/Linux doesn't accept it as a valid content type. I'm guessing there is some larger issue with .NET Core that is causing the problem.

Any way, the fix is to leave out the charset portion of the Content Type and instead explicitly encode the $body JSON as UTF-8. That works on both Windows and Mac (and I'm assuming Linux, but I haven't actually tested that).

Here's a working example:

[String]$API_KEY = ''
$URI = "https://api.eu.itglue.com/contacts"
[int64]$organization_id = ''

$body = ConvertTo-Json (@{
    $data = @{
        type = 'contacts'
        attributes = @{
            first_name = "Example 4 (working)"
            last_name = "…˚Ô€≈ƒ÷éÅäör"
            notes = "user test"
        }
    }
})

$header_example1 = @{
    "x-api-key" = $API_KEY
    "Content-Type" = "application/vnd.api+json"
}

(Invoke-RestMethod -method 'POST' -uri $URI -headers $header_example1 -body ([System.Text.Encoding]::UTF8.GetBytes($body))).data.attributes.'last-name'

Note the [System.Text.Encoding]::UTF8.GetBytes($body) section in the Invoke-RestMethod call.

Can you test and see if that works for you as well?

As an aside, what do you guys think of wrapping the entire section that decodes the API Key and calls Invoke-RestMethod into a helper function? That way we could change stuff like this more easily in the future. I'm not sure what sort of impact it would have on performance or the ability to troubleshoot effectively. I'd be open to hearing both sides. @adrianwells ?

CalebAlbers avatar Jan 31 '19 21:01 CalebAlbers

@CalebAlbers You would be correct in this assumption. 😀

I tested this on both Debian 9/PowerShell Core and Windows 10/PowerShell 5 (and aside from adding organization_id = $organization_id to attributes) it worked just fine.

As an aside, what do you guys think of wrapping the entire section that decodes the API Key and calls Invoke-RestMethod into a helper function?

👍

ecspresso avatar Feb 05 '19 10:02 ecspresso

@CalebAlbers sounds good to create a helper function to help now with this issue and in the future. I do not believe performance is a major driver at this time (though there is a PR that looks to help with an aspect of that). @ecspresso thanks for testing on different platforms!

adrianwells avatar Feb 05 '19 13:02 adrianwells

Tested on Mac OS X 14, installed PowerShell Core via Brew and it was working there as well.

ecspresso avatar Feb 07 '19 11:02 ecspresso

Do we have an ETA regarding this issue?

alexys95 avatar Mar 15 '19 13:03 alexys95

Is there any decision how or progress on solving this permanently aside from the temporary fix suggested? Who is responsible? Seems like @ecspresso is active on these Issues. Hi :)

Do you welcome PRs if my company should decide to allocate my resources to this problem? Will get back to you on that.

emnhIDRIFT avatar Nov 26 '20 11:11 emnhIDRIFT

Please fix this asap. As a German customer we really need this

thomasbiebl avatar Mar 31 '21 05:03 thomasbiebl

I tested this again and it seems that using -ContentType "application/vnd.api+json; charset=utf-8" is enough if you are using PowerShell or PowerShell ISE. The VS Code documentation has an article related to this issue: https://docs.microsoft.com/en-us/powershell/scripting/dev-cross-plat/vscode/understanding-file-encoding

You're more likely to have encoding problems when you're using characters not in the 7-bit ASCII character set

Common reasons for encoding issues are:

  • Another editor has opened and overwritten the file in a new encoding. This often happens with the ISE.

I have not really investigated this anymore than noticing the problem but PowerShell ISE (and I assume the old PowerShell terminal, not Core), saves the files in one charset and every other editor I have ever used saves in another. As a result, if I save åäö in a file using VS Code and created a contact in IT Glue, it will become åäö because that is how it is encoded in PowerShell's encoding.

In short, specifying the content type via the parameter fixes the charset problem but will still upload whatever PowerShell reads and not your editor of choice.

Thus, I suggest that we add -ContentType "application/vnd.api+json; charset=utf-8" to all PATCH and POST requests. What do you think @CalebAlbers @adrianwells?

ecspresso avatar Jun 30 '21 07:06 ecspresso

In short, specifying the content type via the parameter fixes the charset problem but will still upload whatever PowerShell reads and not your editor of choice.

Thus, I suggest that we add -ContentType "application/vnd.api+json; charset=utf-8" to all PATCH and POST requests. What do you think @CalebAlbers @adrianwells?

I think that's entirely reasonable. We can't protect a file from having its encoding change due to an editor.

Thanks for validating the change! We could add the -ContentType param to all existing functions, but since we're going to be in there changing each function anyway, now would probably be a good time to wrap the current request code into a helper function. Everything seen below could be condensed:

$ITGlue_Headers.Add('x-api-key', (New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList 'N/A', $ITGlue_API_Key).GetNetworkCredential().Password)
$rest_output = Invoke-RestMethod -method 'POST' -uri ($ITGlue_Base_URI + $resource_uri) -headers $ITGlue_Headers `
    -body $body -ErrorAction Stop -ErrorVariable $web_error
[void] ($ITGlue_Headers.Remove('x-api-key')) # Quietly clean up scope so the API key doesn't persist

$data = @{}
$data = $rest_output

I propose wrapping it into an internal helper that looks something like this:

function Invoke-ITGlueRequest {
    Param (
        [Parameter(Mandatory = $true)]
        [String]$resource = '',

        [Parameter(Mandatory = $true)]
        [Microsoft.PowerShell.Commands.WebRequestMethod]$method,

        [Parameter(Mandatory = $false)]
        $body
    )

    ...
}

Doing so would have the benefit of making the resource functions easier to read and write. Thoughts?

CalebAlbers avatar Jun 30 '21 15:06 CalebAlbers

Doing so would have the benefit of making the resource functions easier to read and write. Thoughts?

I like it. It makes future modifications like this easier as well.

ecspresso avatar Jul 02 '21 06:07 ecspresso

The proposed changes with a internal helper function acting as a wrapper sounds good @CalebAlbers and @ecspresso.

adrianwells avatar Jul 02 '21 19:07 adrianwells

Awesome - I'll be out for a long holiday weekend, but I'm going to carve out some time when I get back on Wednesday to take a stab at it.

CalebAlbers avatar Jul 02 '21 19:07 CalebAlbers

Any way - I did some troubleshooting on this and came across some potential situations in which appending the ; charset=utf-8 to the Content-Type string would lead to issues. Specifically, ConvertTo-Json function doesn't respect the encoding that is passed to it. That's why you can toss something that is encoded in UTF-8 to it and get garbage out. Now, the ; charset=utf-8 works on Windows, but for some reason the Invoke-RestMethod function on PS Core 6.x for Mac/Linux doesn't accept it as a valid content type. I'm guessing there is some larger issue with .NET Core that is causing the problem.

I did some digging and discovered that this bug was fixed with powershell/powershell#8742, which was included in PowerShell 6.2 (see the release notes). I think it's safe to say that appending ; charset=utf-8 to the Content-Type of all requests would be an adequate fix for this issue.

I successfully tested this on PowerShell 7.3 on Linux.

davidhaymond avatar Dec 05 '22 18:12 davidhaymond

I did some digging and discovered that this bug was fixed with PowerShell/PowerShell#8742, which was included in PowerShell 6.2 (see the release notes). I think it's safe to say that appending ; charset=utf-8 to the Content-Type of all requests would be an adequate fix for this issue.

I successfully tested this on PowerShell 7.3 on Linux.

The decision then is if we still want to support PowerShell 5.1 or just PowerShell Core.

ecspresso avatar Dec 07 '22 10:12 ecspresso

Ideally, I would like to see support for PowerShell 5.1 and above. As someone who works in the MSP space right now, 99% of our local setups still use PowerShell 5.1. Plus PowerShell 5.1 still comes built-in on all Windows OS's. I don't believe MS has stated yet when PowerShell 7 will replace 5.1 as the new built-in option.

I would be interested in hearing what version of PowerShell others are using the ITGlue module on.

Celerium avatar Dec 07 '22 14:12 Celerium

I agree with supporting Windows PowerShell 5.1 because of the target audience. As a developer no longer in the MSP space, I always use Powershell 7 even on Windows, but I recognize the convenience of not having to install PowerShell 7 on all machines that might need it.

The bug originally mentioned by @CalebAlbers only affected PowerShell 6 and later; now that it has been fixed, appending ; charset=utf-8 to -Content-Type works on all supported versions of PowerShell, so I believe it is the best solution for this issue.

davidhaymond avatar Dec 07 '22 16:12 davidhaymond

I would be interested in hearing what version of PowerShell others are using the ITGlue module on.

PowerShell 5.1 on 97% of our Clients. We got some spots, where we still work on old PowerShell Versions. But out entire monitoring is build on 5.1 - so I wouldn't work on supporting older versions than this. pwsh 7.3 on my system. But only because I switched to macOS some months ago.

TheMonzel avatar Dec 07 '22 20:12 TheMonzel

Yes, keeping support for 5.1 would be best.

I remember this not being a problem from the start, it appeared at a later point in time after this wrapper was created. I wonder (unless I am wrong), if IT Glue did something. In any case, any solution we choose should be tested first.

ecspresso avatar Dec 09 '22 16:12 ecspresso

I've fixed this bug in my fork (872178a).

davidhaymond avatar May 02 '23 23:05 davidhaymond