Skip to content

Use HTML meta charset attribute value, if present, when the Context-Type header does not specify it. #4338

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

Merged
TravisEz13 merged 6 commits into from
Aug 16, 2017
Merged

Use HTML meta charset attribute value, if present, when the Context-Type header does not specify it. #4338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2fa0f51
Use HTML meta charset attribute value, if present, when the Context-T…
dantraMSFT Jul 24, 2017
74d816c
Switch to regex for searching for meta/charset attribute
dantraMSFT Aug 7, 2017
e5cfbb1
Update tests to use encodings that are available on Linux
dantraMSFT Aug 8, 2017
d1a5b2b
[Feature]
dantraMSFT Aug 8, 2017
4f4066b
[Feature]
dantraMSFT Aug 9, 2017
09e7a46
[Heading]
dantraMSFT Aug 9, 2017
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
14 changes: 12 additions & 2 deletions ...l.Commands.Utility/commands/utility/WebCmdlet/Common/BasicHtmlWebResponseObject.Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ public partial class BasicHtmlWebResponseObject : WebResponseObject
/// </summary>
public new string Content { get; private set; }

/// <summary>
/// Gets the Encoding that was used to decode the Content
/// </summary>
/// <value>
/// The Encoding used to decode the Content; otherwise, a null reference if the content is not text.
/// </value>
public Encoding Encoding { get; private set; }

private WebCmdletElementCollection _inputFields;

/// <summary>
Expand Down Expand Up @@ -217,14 +225,16 @@ private void ParseAttributes(string outerHtml, PSObject elementObject)
/// <summary>
/// Reads the response content from the web response.
/// </summary>
private void InitializeContent()
protected void InitializeContent()
{
string contentType = ContentHelper.GetContentType(BaseResponse);
if (ContentHelper.IsText(contentType))
{
Encoding encoding = null;
// fill the Content buffer
string characterSet = WebResponseHelper.GetCharacterSet(BaseResponse);
this.Content = StreamHelper.DecodeStream(RawContentStream, characterSet);
this.Content = StreamHelper.DecodeStream(RawContentStream, characterSet, out encoding);
this.Encoding = encoding;
}
else
{
Expand Down
33 changes: 33 additions & 0 deletions ...hell.Commands.Utility/commands/utility/WebCmdlet/CoreCLR/HtmlWebResponseObject.CoreClr.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,41 @@ internal HtmlWebResponseObject(HttpResponseMessage response, Stream contentStrea

#endregion Constructors

#region Properties

/// <summary>
/// Gets the Encoding that was used to decode the Content
/// </summary>
/// <value>
/// The Encoding used to decode the Content; otherwise, a null reference if the content is not text.
/// </value>
public Encoding Encoding { get; private set; }
Copy link
Member

@SteveL-MSFT SteveL-MSFT Jul 25, 2017

Choose a reason for hiding this comment

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

Need doc change as you're adding to public api

Copy link
Contributor Author

@dantraMSFT dantraMSFT Aug 9, 2017

Choose a reason for hiding this comment

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

Done


#endregion Properties

#region Methods

// NOTE: Currently this code path is not enabled.
// See FillRequestStream in WebRequestPSCmdlet.CoreClr.cs and
// GetResponseObject in WebResponseObjectFactory.CoreClr.cs for details.
private void InitializeContent()
{
string contentType = ContentHelper.GetContentType(BaseResponse);
string content = null;
if (ContentHelper.IsText(contentType))
{
Encoding encoding = null;
// fill the Content buffer
string characterSet = WebResponseHelper.GetCharacterSet(BaseResponse);
this.Content = StreamHelper.DecodeStream(RawContentStream, characterSet, out encoding);
this.Encoding = encoding;
}
else
{
this.Content = string.Empty;
}
}

private void InitializeRawContent(HttpResponseMessage baseResponse)
{
StringBuilder raw = ContentHelper.GetRawContentHeader(baseResponse);
Expand Down
2 changes: 1 addition & 1 deletion ...ll.Commands.Utility/commands/utility/WebCmdlet/CoreCLR/InvokeRestMethodCommand.CoreClr.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal override void ProcessResponse(HttpResponseMessage response)
object obj = null;
Exception ex = null;

string str = StreamHelper.DecodeStream(responseStream, encoding);
string str = StreamHelper.DecodeStream(responseStream, ref encoding);
bool convertSuccess = false;

// On CoreCLR, we need to explicitly load Json.NET
Expand Down
84 changes: 67 additions & 17 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/StreamHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Text;
using System.Text.RegularExpressions;
using System.IO;
using System.IO.Compression;
using System.Management.Automation;
Expand Down Expand Up @@ -391,20 +392,8 @@ internal static void SaveStreamToFile(Stream stream, string filePath, PSCmdlet c
}
}

internal static string DecodeStream(Stream stream, string characterSet)
private static string StreamToString(Stream stream, Encoding encoding)
{
Encoding encoding = ContentHelper.GetEncodingOrDefault(characterSet);
return DecodeStream(stream, encoding);
}

internal static string DecodeStream(Stream stream, Encoding encoding)
{
if (null == encoding)
{
// just use the default encoding if one wasn't provided
encoding = ContentHelper.GetDefaultEncoding();
}

StringBuilder result = new StringBuilder(capacity: ChunkSize);
Decoder decoder = encoding.GetDecoder();

Expand All @@ -413,9 +402,8 @@ internal static string DecodeStream(Stream stream, Encoding encoding)
{
useBufferSize = encoding.GetMaxCharCount(10);
}
char[] chars = new char[useBufferSize];


char[] chars = new char[useBufferSize];
byte[] bytes = new byte[useBufferSize * 4];
int bytesRead = 0;
do
Expand Down Expand Up @@ -444,12 +432,74 @@ internal static string DecodeStream(Stream stream, Encoding encoding)
// Increment byteIndex to the next block of bytes in the input buffer, if any, to convert.
byteIndex += bytesUsed;
}
}
while (bytesRead != 0);
} while (bytesRead != 0);

return result.ToString();
}

internal static string DecodeStream(Stream stream, string characterSet, out Encoding encoding)
{
try
{
encoding = Encoding.GetEncoding(characterSet);
}
catch (ArgumentException)
{
encoding = null;
}
return DecodeStream(stream, ref encoding);
}

static bool TryGetEncoding(string characterSet, out Encoding encoding)
{
bool result = false;
try
{
encoding = Encoding.GetEncoding(characterSet);
result = true;
}
catch (ArgumentException)
{
encoding = null;
}
return result;
}

static readonly Regex s_metaexp = new Regex(@"<meta\s[.\n]*[^><]*charset\s*=\s*[""'\n]?(?<charset>[A-Za-z].[^\s""'\n<>]*)[\s""'\n>]");

internal static string DecodeStream(Stream stream, ref Encoding encoding)
{
bool isDefaultEncoding = false;
if (null == encoding)
{
// Use the default encoding if one wasn't provided
encoding = ContentHelper.GetDefaultEncoding();
isDefaultEncoding = true;
}

string content = StreamToString (stream, encoding);
if (isDefaultEncoding) do
{
// check for a charset attribute on the meta element to override the default.
Match match = s_metaexp.Match(content);
Copy link
Member

@TravisEz13 TravisEz13 Aug 10, 2017

Choose a reason for hiding this comment

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

In order to cache the regex, you need to use the static regex methods. See: https://msdn.microsoft.com/en-us/library/system.text.regularexpressions.regex(v=vs.110).aspx#static_vs_instance

Copy link
Contributor Author

@dantraMSFT dantraMSFT Aug 10, 2017

Choose a reason for hiding this comment

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

@TravisEz13 - According to my reading of the page, I'm doing exactly what it says...

"To prevent recompilation, you should instantiate a single Regex object that is accessible to all code that requires it, as shown in the following rewritten example."

Since s_metaexp is static/readonly, there is only one compilation. That appears to be the point of the of the caching; avoiding recompilation. Am I missing something?

Copy link
Member

@TravisEz13 TravisEz13 Aug 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Under the Important note:

In the .NET Framework versions 1.0 and 1.1, all compiled regular expressions, whether they were used in instance or static method calls, were cached. Starting with the .NET Framework 2.0, only regular expressions used in static method calls are cached.`

Basically, they changed the behavior and only added a not.

Copy link
Contributor Author

@dantraMSFT dantraMSFT Aug 11, 2017

Choose a reason for hiding this comment

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

My review of the corefx shows that constructing and reusing an instance (regex.Match) is equivalent to calling the static Match method. The constructor manages cache lookups/updates. The static Match method constructs a new instance and calls through the instance Match method.
The relevant static method is:
public static Match Match(string input, string pattern, RegexOptions options, TimeSpan matchTimeout)
{
return new Regex(pattern, options, matchTimeout, true).Match(input);
}

Copy link
Member

@TravisEz13 TravisEz13 Aug 11, 2017

Choose a reason for hiding this comment

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

Sound good, thanks for looking into it.

if (match.Success)
{
Encoding localEncoding = null;
string characterSet = match.Groups["charset"].Value;

if (TryGetEncoding(characterSet, out localEncoding))
{
stream.Seek(0, SeekOrigin.Begin);
content = StreamToString(stream, localEncoding);
// report the encoding used.
encoding = localEncoding;
}
}
} while (false);

return content;
}

internal static Byte[] EncodeToBytes(String str, Encoding encoding)
{
if (null == encoding)
Expand Down
Loading