Skip to content
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

Encoding/Decoding AppNotificationBuilder arguments #2805

Merged
merged 15 commits into from
Aug 4, 2022
Merged

Encoding/Decoding AppNotificationBuilder arguments #2805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d200a18
Encode arguments
pmpurifoy Aug 1, 2022
8954795
Argument encode/decode
pmpurifoy Aug 2, 2022
20997db
Merge branch 'feature/WNP_ContentBuilder_Prev1' into user/purifoypaul…
pmpurifoy Aug 2, 2022
af02980
Merge branch 'feature/WNP_ContentBuilder_Prev1' into user/purifoypaul…
loneursid Aug 2, 2022
0a2f8e5
Fix merge issues
pmpurifoy Aug 2, 2022
244713d
Merge branch 'user/purifoypaul/ArgumentDecoding' of https://github.co…
pmpurifoy Aug 2, 2022
5d1c7e0
Add Decode unit test
pmpurifoy Aug 2, 2022
2a3c9be
Use regex_replace instead
pmpurifoy Aug 2, 2022
739fb7c
Address nits
pmpurifoy Aug 2, 2022
4e68b51
Simplify encoding
pmpurifoy Aug 3, 2022
c50d93f
Added XML escape encodings, need to improve Decode efficiency
pmpurifoy Aug 4, 2022
b4fae36
Implement efficient decode
pmpurifoy Aug 4, 2022
20a6047
Merge branch 'feature/WNP_ContentBuilder_Prev1' into user/purifoypaul…
loneursid Aug 4, 2022
c61056c
Update APITests.cpp
pmpurifoy Aug 4, 2022
be909e5
Add unit tests and encode all custom strings
pmpurifoy Aug 4, 2022
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
43 changes: 43 additions & 0 deletions dev/AppNotifications/AppNotificationActivatedEventArgs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#include "pch.h"
#include "AppNotificationActivatedEventArgs.h"
#include "Microsoft.Windows.AppNotifications.AppNotificationActivatedEventArgs.g.cpp"
#include "AppNotificationBuilder/AppNotificationBuilderUtility.h"

namespace winrt::Microsoft::Windows::AppNotifications::implementation
{
winrt::Windows::Foundation::Collections::IMap<hstring, hstring> AppNotificationActivatedEventArgs::DecodeArguments(std::wstring arguments)
{
auto result{ winrt::single_threaded_map<winrt::hstring, winrt::hstring>() };

std::vector<std::wstring> pairs{};
size_t pos{ 0 };

// Separate the key/value pairs by ';' as the delimiter
while ((pos = arguments.find(L';')) != std::wstring::npos)
{
pairs.push_back(arguments.substr(0, pos));
arguments.erase(0, pos + 1);
}

// Need to push back final string
pairs.push_back(arguments);

for (auto pair : pairs)
{
// Get the key/value individual values separated by '='
pos = pair.find(L'=');
Copy link
Member

Choose a reason for hiding this comment

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

You can use tokenizers here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There can only be one '=' per token. Find is simpler/better here since we aren't expecting more than 1 delimiter.

if (pos == std::wstring::npos)
{
result.Insert(Decode(pair).c_str(), L"");
Copy link
Member

Choose a reason for hiding this comment

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

So is it expected to return empty string as the value for a pair? Can we get away with not putting in anything here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Result is a pair of <hstring, hstring. As fas I know there is anything else we can put in this to indicate no value. insert() insists on an hstring won't accept a nullptr.

And since this is part of the IDL, we can really change it to an optional<>

Copy link
Contributor Author

@pmpurifoy pmpurifoy Aug 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think you can use optional in midl 3, but I think it's fine to have an empty "" value. Developers doing AddArgument(L"Key", L""), is valid. The toolkit allows this behavior as well. In the future, we can add AddArgument(key) for a more explicit "I want an empty value", but even then we would store it as { L"key", L"" }.

}
else
{
result.Insert(Decode(pair.substr(0, pos)).c_str(), Decode(pair.substr(pos + 1)).c_str());
}
}
return result;
}
}
13 changes: 9 additions & 4 deletions dev/AppNotifications/AppNotificationActivatedEventArgs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#pragma once
Expand All @@ -10,12 +10,17 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
{
AppNotificationActivatedEventArgs() = default;

AppNotificationActivatedEventArgs(winrt::hstring const& arguments, winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::hstring> const& userInput) : m_arguments(arguments), m_userInput(userInput) {};
winrt::hstring Argument() { return m_arguments; };
AppNotificationActivatedEventArgs(winrt::hstring const& argument, winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::hstring> const& userInput)
: m_argument(argument), m_userInput(userInput), m_arguments(DecodeArguments(argument.c_str())) {};
winrt::hstring Argument() { return m_argument; };
winrt::Windows::Foundation::Collections::IMap<hstring, hstring> UserInput() { return m_userInput; };
winrt::Windows::Foundation::Collections::IMap<hstring, hstring> Arguments() { return m_arguments; };

private:
winrt::hstring m_arguments;
winrt::Windows::Foundation::Collections::IMap<hstring, hstring> DecodeArguments(std::wstring arguments);

winrt::hstring m_argument;
winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::hstring> m_userInput;
winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::hstring> m_arguments{ nullptr };
};
}
20 changes: 10 additions & 10 deletions dev/AppNotifications/AppNotificationBuilder/AppNotificationBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
THROW_HR_IF_MSG(E_INVALIDARG, key.empty(), "You must provide a key when adding an argument");

m_arguments.Insert(key, value);
m_arguments.Insert(EncodeArgument(key.c_str()), EncodeArgument(value.c_str()));
return *this;
}

Expand Down Expand Up @@ -64,7 +64,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
THROW_HR_IF_MSG(E_INVALIDARG, m_textLines.size() >= c_maxTextElements, "Maximum number of text elements added");

m_textLines.push_back(wil::str_printf<std::wstring>(L"<text>%ls</text>", text.c_str()).c_str());
m_textLines.push_back(wil::str_printf<std::wstring>(L"<text>%ls</text>", EncodeXml(text).c_str()).c_str());
return *this;
}

Expand All @@ -73,7 +73,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
THROW_HR_IF_MSG(E_INVALIDARG, m_textLines.size() >= c_maxTextElements, "Maximum number of text elements added");

std::wstring props{ properties.as<winrt::Windows::Foundation::IStringable>().ToString() };
m_textLines.push_back(wil::str_printf<std::wstring>(L"%ls%ls</text>", props.c_str(), text.c_str()).c_str());
m_textLines.push_back(wil::str_printf<std::wstring>(L"%ls%ls</text>", props.c_str(), EncodeXml(text).c_str()).c_str());

if (properties.IncomingCallAlignment())
{
Expand All @@ -84,15 +84,15 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAttributionText(hstring const& text)
{
m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution'>%ls</text>", text.c_str());
m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution'>%ls</text>", EncodeXml(text).c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAttributionText(hstring const& text, hstring const& language)
{
THROW_HR_IF_MSG(E_INVALIDARG, language.empty(), "You must provide a language calling SetAttributionText");

m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution' lang='%ls'>%ls</text>", language.c_str(), text.c_str());
m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution' lang='%ls'>%ls</text>", language.c_str(), EncodeXml(text).c_str());
return *this;
}

Expand Down Expand Up @@ -121,7 +121,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetInlineImage");

std::wstring hintCrop { imageCrop == AppNotificationImageCrop::Circle ? L" hint-crop='circle'" : L"" };
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ls' alt='%ls'%ls/>", imageUri.ToString().c_str(), alternateText.c_str(), hintCrop.c_str());
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ls' alt='%ls'%ls/>", imageUri.ToString().c_str(), EncodeXml(alternateText).c_str(), hintCrop.c_str());

return *this;
}
Expand Down Expand Up @@ -151,7 +151,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetAppLogoOverride");

std::wstring hintCrop{ imageCrop == AppNotificationImageCrop::Circle ? L" hint-crop='circle'" : L"" };
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ls' alt='%ls'%ls/>", imageUri.ToString().c_str(), alternateText.c_str(), hintCrop.c_str());
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ls' alt='%ls'%ls/>", imageUri.ToString().c_str(), EncodeXml(alternateText).c_str(), hintCrop.c_str());

return *this;
}
Expand All @@ -166,7 +166,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetHeroImage");

m_heroImage = wil::str_printf<std::wstring>(L"<image placement='hero' src='%ls' alt='%ls'/>", imageUri.ToString().c_str(), alternateText.c_str());
m_heroImage = wil::str_printf<std::wstring>(L"<image placement='hero' src='%ls' alt='%ls'/>", imageUri.ToString().c_str(), EncodeXml(alternateText).c_str());
return *this;
}

Expand Down Expand Up @@ -213,7 +213,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
THROW_HR_IF_MSG(E_INVALIDARG, id.empty(), "You must provide an id for the TextBox");


m_textBoxList.push_back(wil::str_printf<std::wstring>(L"<input id='%ls' type='text'/>", id.c_str()));
m_textBoxList.push_back(wil::str_printf<std::wstring>(L"<input id='%ls' type='text'/>", EncodeXml(id).c_str()));
return *this;
}

Expand All @@ -222,7 +222,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
ThrowIfMaxInputItemsExceeded();
THROW_HR_IF_MSG(E_INVALIDARG, id.empty(), "You must provide an id for the TextBox");

m_textBoxList.push_back(wil::str_printf<std::wstring>(L"<input id='%ls' type='text' placeHolderContent='%ls' title='%ls'/>", id.c_str(), placeHolderText.c_str(), title.c_str()));
m_textBoxList.push_back(wil::str_printf<std::wstring>(L"<input id='%ls' type='text' placeHolderContent='%ls' title='%ls'/>", EncodeXml(id).c_str(), EncodeXml(placeHolderText).c_str(), EncodeXml(title).c_str()));
return *this;
}

Expand Down
94 changes: 93 additions & 1 deletion dev/AppNotifications/AppNotificationBuilder/AppNotificationBuilderUtility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#pragma once
#include "pch.h"
#include "winrt/Microsoft.Windows.AppNotifications.Builder.h"
#include <algorithm>
#include <regex>
#include <map>
#include <iostream>

constexpr size_t c_maxAppNotificationPayload{ 5120 };
constexpr uint8_t c_maxTextElements{ 3 };
constexpr uint8_t c_maxButtonElements{ 5 };
constexpr size_t c_maxEncodingSize{ 3 };
constexpr uint8_t c_maxTextInputElements{ 5 };
constexpr uint8_t c_maxSelectionElements{ 5 };

Expand All @@ -16,6 +20,24 @@ namespace AppNotificationBuilder
using namespace winrt::Microsoft::Windows::AppNotifications::Builder;
}

inline std::unordered_map<wchar_t, std::wstring> GetXmlEscapeEncodings()
Copy link
Member

Choose a reason for hiding this comment

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

Why return the whole array? Instead, let this API take in a char as an argument and return a string.

Call it
std::wstring GetXmlEscapeEncoding(const wchar_t& charToEncode)

{
static std::unordered_map<wchar_t, std::wstring> encodings = { { L'&', L"&amp;"}, { L'\"', L"&quot;"}, {L'<', L"&lt;"}, {L'>', L"&gt;"}, {L'\'', L"&apos;"}};
return encodings;
}

inline std::unordered_map<wchar_t, std::wstring> GetPercentEncodings()
{
static std::unordered_map<wchar_t, std::wstring> encodings = {{ L'%', L"%25"}, {L';', L"%3B"}, {L'=', L"%3D"} };
Copy link
Member

Choose a reason for hiding this comment

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

Just put this in the first map. Why do we need a second API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the second API because the only use for percent encoding is for arguments. %;= don't need any special encoding for anything else.

return encodings;
}

inline std::unordered_map<std::wstring, wchar_t> GetPercentEncodingsReverse()
{
static std::unordered_map<std::wstring, wchar_t> encodings = { { L"%25", L'%' }, {L"%3B", L';' }, { L"%3D", L'=' } };
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above here. Why return a list when you can just return a char and the API takes in an encodedString

return encodings;
}

inline PCWSTR GetWinSoundEventString(AppNotificationBuilder::AppNotificationSoundEvent soundEvent)
{
switch (soundEvent)
Expand Down Expand Up @@ -72,3 +94,73 @@ inline PCWSTR GetWinSoundEventString(AppNotificationBuilder::AppNotificationSoun
return L"ms-winsoundevent:Notification.Default";
}
}

inline std::wstring EncodeArgument(std::wstring const& value)
{
std::wstring encodedValue{};

auto percentEncodings{ GetPercentEncodings() };
auto xmlEncodings{ GetXmlEscapeEncodings() };
for (auto ch : value)
{
if (percentEncodings.find(ch) != percentEncodings.end())
{
encodedValue.append(percentEncodings[ch]);
}
else if (xmlEncodings.find(ch) != xmlEncodings.end())
{
encodedValue.append(xmlEncodings[ch]);
}
else
{
encodedValue.push_back(ch);
}
}

return encodedValue;
}

inline std::wstring EncodeXml(winrt::hstring const& value)
{
std::wstring encodedValue{};

auto xmlEncodings{ GetXmlEscapeEncodings() };
for (auto ch : value)
{
if (xmlEncodings.find(ch) != xmlEncodings.end())
{
encodedValue.append(xmlEncodings[ch]);
}
else
{
encodedValue.push_back(ch);
}
}

return encodedValue;
}

// Decoding process based off the Windows Community Toolkit:
// https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/rel/7.1.0/Microsoft.Toolkit.Uwp.Notifications/Toasts/ToastArguments.cs#L389inline
inline std::wstring Decode(std::wstring const& value)
{
std::wstring result{};
auto percentEncodings{ GetPercentEncodingsReverse() };

// Need to unescape special characters
for (size_t index = 0; index < value.size();)
{
std::wstring curr{ value.substr(index, c_maxEncodingSize) };
if (percentEncodings.find(curr) != percentEncodings.end())
{
result.push_back(percentEncodings[curr]);
index += c_maxEncodingSize;
}
else
{
result.push_back(value.at(index));
index++;
}
}
return result;
}
7 changes: 4 additions & 3 deletions dev/AppNotifications/AppNotificationBuilder/AppNotificationButton.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "AppNotificationButton.h"
#include "Microsoft.Windows.AppNotifications.Builder.AppNotificationButton.g.cpp"
#include <IsWindowsVersion.h>
#include "AppNotificationBuilderUtility.h"

namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
Expand All @@ -25,7 +26,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
THROW_HR_IF_MSG(E_INVALIDARG, key.empty(), "You must provide a key when adding an argument");
THROW_HR_IF_MSG(E_INVALIDARG, m_protocolUri, "You cannot add an argument after calling SetInvokeUri");

m_arguments.Insert(key, value);
m_arguments.Insert(EncodeArgument(key.c_str()), EncodeArgument(value.c_str()));
return *this;
}

Expand All @@ -37,7 +38,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationButton AppNotificationButton::SetToolTip(winrt::hstring const& value)
{
m_toolTip = value;
m_toolTip = EncodeXml(value.c_str()).c_str();
return *this;
}

Expand All @@ -55,7 +56,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationButton AppNotificationButton::SetInputId(winrt::hstring const& value)
{
m_inputId = value;
m_inputId = EncodeXml(value.c_str()).c_str();
return *this;
}

Expand Down
9 changes: 5 additions & 4 deletions dev/AppNotifications/AppNotificationBuilder/AppNotificationComboBox.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,25 @@

namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
AppNotificationComboBox::AppNotificationComboBox(hstring const& id) : m_id(id)
AppNotificationComboBox::AppNotificationComboBox(hstring const& id)
{
THROW_HR_IF_MSG(E_INVALIDARG, id.empty(), "You must provide an id for the ComboBox");
m_id = EncodeXml(id).c_str();
};

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationComboBox AppNotificationComboBox::AddItem(winrt::hstring const& id, winrt::hstring const& content)
{
THROW_HR_IF_MSG(E_INVALIDARG, m_items.Size() >= c_maxSelectionElements, "Maximum number of items added");
THROW_HR_IF_MSG(E_INVALIDARG, id.empty(), "You must provide an id for the item");

m_items.Insert(id, content);
m_items.Insert(EncodeXml(id).c_str(), EncodeXml(content).c_str());

return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationComboBox AppNotificationComboBox::SetTitle(winrt::hstring const& value)
{
m_title = value;
m_title = EncodeXml(value).c_str();

return *this;
}
Expand All @@ -34,7 +35,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
THROW_HR_IF_MSG(E_INVALIDARG, id.empty(), "You must provide an id for the selected item");

m_selectedItem = id;
m_selectedItem = EncodeXml(id).c_str();

return *this;
}
Expand Down
7 changes: 4 additions & 3 deletions dev/AppNotifications/AppNotificationBuilder/AppNotificationProgressBar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "pch.h"
#include "AppNotificationProgressBar.h"
#include "Microsoft.Windows.AppNotifications.Builder.AppNotificationProgressBar.g.cpp"
#include "AppNotificationBuilderUtility.h"

namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
{
Expand All @@ -16,13 +17,13 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

void AppNotificationProgressBar::Title(winrt::hstring const& value)
{
m_title = value;
m_title = EncodeXml(value).c_str();
m_titleBindMode = BindMode::Value;
}

void AppNotificationProgressBar::Status(winrt::hstring const& value)
{
m_status = value;
m_status = EncodeXml(value).c_str();
m_statusBindMode = BindMode::Value;
}

Expand All @@ -36,7 +37,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

void AppNotificationProgressBar::ValueStringOverride(winrt::hstring const& value)
{
m_valueStringOverride = value;
m_valueStringOverride = EncodeXml(value).c_str();
m_valueStringOverrideBindMode = BindMode::Value;
}

Expand Down
Loading