Skip to content

Commit

Permalink
Fix character escaping issues during config generation
Browse files Browse the repository at this point in the history
> This is a re-implementation of microsoft#198 for the v2 branch.

Many years ago, microsoft#24 attempted to properly escape special characters in
a string by creating its own `Get-EscapedJsonValue` function, but that
only escaped `\`, `\t`, `\r`, `\n`.  That doesn't cover all the possible
characters needing to be encoded though.

A user had a `<` character (which was properly JSON-escaped as `\u003c`),
but that didn't get escaped by this logic, and thus caused an issue when
trying to get decoded by `ConvertFrom-Json` later on.

Back then, `Get-EscapedJsonValue` must have seemed like a _good_ idea,
but looking back at this code with more experience, it's not clear to me
why I didn't just use `ConvertTo-Json` directly. Doing that now.

One additional update that needed to be done however was to replace `//`
with `\\`  in `tag` and `notesForCertification`.  This isn't _completely_
correct, but not doing so meant that anything after that would be
improperly flagged as a comment by `Remove-Comment` and then removed.
This was found because the same user had a URL in their
`notesForCertification` and the closing JSON was getting stripped out
incorrectly as soon as it found the `//` in `https://....`.
  • Loading branch information
HowardWolosky committed Aug 14, 2020
1 parent 4d67f87 commit 9db745e
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 57 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
# StoreBroker PowerShell Module
## Changelog

## [2.1.13](https://github.com/Microsoft/StoreBroker/tree/2.1.13) - (2020/08/14)
### Fixes:

- Fixes the logic for config generation when `tag` or `notesForCertification` from Partner Center
had an encoded character or a URL value. The encoded character wasn't getting re-encoded properly
in the generated config, and everything after the `//` in the URL was being treated as a comment.

More Info: [[pr]](https://github.com/Microsoft/StoreBroker/pull/https://github.com/Microsoft/StoreBroker/pull/199) | [[cl]](https://github.com/Microsoft/StoreBroker/commit/...)

Author: [**@HowardWolosky**](https://github.com/HowardWolosky)

## [2.1.9](https://github.com/Microsoft/StoreBroker/tree/2.1.9) - (2020/04/10)
### Fixes:

Expand Down
2 changes: 1 addition & 1 deletion StoreBroker.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
CompanyName = 'Microsoft Corporation'
Copyright = 'Copyright (C) Microsoft Corporation. All rights reserved.'

ModuleVersion = '2.1.12'
ModuleVersion = '2.1.13'
Description = 'Provides command-line access to the Windows Store Submission REST API.'

RootModule = 'StoreBroker/StoreIngestionApi.psm1'
Expand Down
51 changes: 0 additions & 51 deletions StoreBroker/Helpers.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -371,57 +371,6 @@ function Get-SHA512Hash
return [System.BitConverter]::ToString($sha512.ComputeHash($utf8.GetBytes($PlainText))) -replace '-', ''
}

function Get-EscapedJsonValue
{
<#
.SYNOPSIS
Escapes special characters within a string for use within a JSON value.
.DESCRIPTION
Escapes special characters within a string for use within a JSON value.
The Git repo for this module can be found here: http://aka.ms/StoreBroker
.PARAMETER Value
The string that needs to be escaped
.EXAMPLE
Get-EscapedJsonValue -Value 'This is my "quote". Look here: c:\windows\'
Returns back the string 'This is my \"quote\". Look here: c:\\windows\\'
.OUTPUTS
System.String - A string with special characters escaped for use within JSON.
.NOTES
Normalizes newlines and carriage returns to always be \r\n.
#>
[CmdletBinding()]
param(
[Parameter(
Mandatory,
ValueFromPipeline)]
[AllowNull()]
[AllowEmptyString()]
[string] $Value
)

# The syntax of -replace is a bit confusing, so it's worth a note here.
# The first parameter is a regular expression match pattern. The second parameter is a replacement string.
# So, when we try to do "-replace '\\', '\\", that's matching a single backslash (which has to be
# escaped within the match regular expression as a double-backslash), and replacing it with a
# string containing literally two backslashes.
# (And as a reminder, PowerShell's escape character is actually the backtick (`) and not backslash (\).)

# \, ", <tab>
$escaped = $Value -replace '\\', '\\' -replace '"', '\"' -replace '\t', '\t'

# Now normalize actual CR's and LF's with their control codes. We'll ensure all variations are uniformly formatted as \r\n
$escaped = $escaped -replace '\r\n', '\r\n' -replace '\r', '\r\n' -replace '\n', '\r\n'

return $escaped
}

function ConvertTo-Array
{
<#
Expand Down
8 changes: 4 additions & 4 deletions StoreBroker/PackageConfig.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,16 @@ function Get-StoreBrokerConfigFileContentForIapId
$updated = $updated -replace '"lifetime": ".*",', "`"lifetime`": `"$($sub.lifetime)`","
$updated = $updated -replace '"contentType": ".*",', "`"contentType`": `"$($sub.contentType)`","

$tag = Get-EscapedJsonValue -Value $sub.tag
$updated = $updated -replace '"tag": ""', "`"tag`": `"$tag`""
$tag = ConvertTo-Json -InputObject ($sub.tag -replace '//', '\\') # Ensure // doesn't get stripped out as a comment later on.
$updated = $updated -replace '"tag": ""', "`"tag`": $tag"

$keywords = $sub.keywords | ConvertTo-Json -Depth $script:jsonConversionDepth
if ($null -eq $keywords) { $keywords = "[ ]" }
$updated = $updated -replace '(\s+)"keywords": \[.*(\r|\n)+\s*\]', "`$1`"keywords`": $keywords"

# NOTES FOR CERTIFICATION
$notesForCertification = Get-EscapedJsonValue -Value $sub.notesForCertification
$updated = $updated -replace '"notesForCertification": ""', "`"notesForCertification`": `"$notesForCertification`""
$notesForCertification = ConvertTo-Json -InputObject ($sub.notesForCertification -replace '//', '\\') # Ensure // doesn't get stripped out as a comment later on.
$updated = $updated -replace '"notesForCertification": ""', "`"notesForCertification`": $notesForCertification"

return $updated
}
Expand Down
2 changes: 1 addition & 1 deletion StoreBroker/PackageTool.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ function Read-AppPackageBundleMetadata
$capabilities += $appPackageMetadata.capabilities
$targetDeviceFamilies += $appPackageMetadata.targetDeviceFamilies
$targetDeviceFamiliesEx += $appPackageMetadata.targetDeviceFamiliesEx

# Don't overwrite full app data with data from a stub
if ($application.IsStub -and $null -ne $metadata.innerPackages.$($appPackageMetadata.architecture))
{
Expand Down

0 comments on commit 9db745e

Please sign in to comment.