Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Correct HTML and JavaScript encoding of <link> and <script> attribute values #4272

Closed
wants to merge 1 commit into from

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Mar 11, 2016

  • <link> and <script> tag helpers do not encode correctly #4083
  • <link> tag helper did not HTML encode href values in fallback elements
  • <script> tag helper did not correctly encode any attribute value in fallback elements
    • e.g. double quotes in literal strings would slip through
  • only needed to change one existing unit test (!!); so added a bunch

nit: use Process(), not ProcessAsync() in <script> tag helper tests

…bute values

- #4083
- `<link>` tag helper did not HTML encode `href` values in fallback elements
- `<script>` tag helper did not correctly encode any attribute value in fallback elements
 - e.g. double quotes in literal strings would slip through
- only needed to change one existing unit test (!!); so added a bunch

nit: use `Process()`, not `ProcessAsync()` in `<script>` tag helper tests
else
{
var writer = StringWriter;
RazorPage.WriteTo(writer, HtmlEncoder, value);
Copy link
Member Author

Choose a reason for hiding this comment

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

This static method isn't fitting so well in RazorPage but I haven't got a better place to throw it. Will leave as-is unless someone has a good suggestion.

@ryanbrandenburg
Copy link
Contributor

:shipit:, seems straighforward.

// Value likely came from an HTML context in the .cshtml file but may still contain double quotes
// since attribute could have been enclosed in single quotes.
stringValue = htmlEncodedString.Value;
if (stringValue.Contains("\""))
Copy link
Contributor

Choose a reason for hiding this comment

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

This check shouldn't be required. Calling stringValue.Replace would no-op if it doesn't have a quote.

Copy link
Member Author

Choose a reason for hiding this comment

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

👌 double-checked the native source and confirmed that.

@pranavkm
Copy link
Contributor

:shipit:

@dougbu
Copy link
Member Author

dougbu commented Mar 12, 2016

ffe2d26

@dougbu dougbu closed this Mar 12, 2016
@dougbu dougbu deleted the dougbu/more.encode.4083 branch March 12, 2016 04:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants