Correctly Encode Text Fields

[armaganpekatik]

Summary

Encoded content is not always correctly decoded, this addition resolves the issue.

[dnfadmin]

All CLA requirements met.

[valadas]

Great improvement, should we catch also decimals by using "&#" instead of "&#x" only (for hex) ?

[armaganpekatik]

Great improvement, should we catch also decimals by using "&#" instead of "&#x" only (for hex) ?

Why do we need to catch decimals as per text may create an issue? Decimals without texts should be out of scope.

[valadas]

If I understand correctly this is to also encode entities such as <

An entity can be named or defined as an hexadecimal or a decimal as such:

So &# instead of &#x would cover both the hex or decimal entities right?

[armaganpekatik]

If I understand correctly this is to also encode entities such as <

An entity can be named or defined as an hexadecimal or a decimal as such:

So &# instead of &#x would cover both the hex or decimal entities right?

Do you think if this is better

if (tempInput.Contains("&gt;") || tempInput.Contains("&lt;") || tempInput.Contains("&#60") || tempInput.Contains("&#x3C;") || tempInput.Contains("<") || tempInput.Contains("&#62;") || tempInput.Contains("&#x3E;") || tempInput.Contains(">"))
And surely we may put them to a dictionary and compare. May be checking all is not necessary without these symbols?

[valadas]

Works for me, the only public code path to this is deprecated anyway so this will eventually be removed and consumers are advised to use HtmlEncode instead. So, for now, that works for me.

[valadas]

Love it, thanks!

[valadas]

There is a totally unrelated test failing though, I'll re-run the build to see if it's a fluke...

[valadas]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[armaganpekatik]

There is a totally unrelated test failing though, I'll re-run the build to see if it's a fluke...

I did a clean push
May be it is because of first line change was there starting with License text

EDIT: Failing test : https://github.com/armaganpekatik/Dnn.Platform/blob/bugfix/Issue-5105/DNN%20Platform/Tests/DotNetNuke.Tests.Core/Providers/Folder/FileManagerTests.cs#L257
It has PORTAL_ID : 20 but fileContent.Length returns 305

EDIT 2: The AddFile_No_Error_When_File_Content_Is_Valid test is failing when we replace the "<", ">" strings. I may break any functionality with doing this. So reverting

[armaganpekatik]

I had to revert it to beginning and applying the advise that you gave @valadas

[donker]

LGTM