Skip to content

LinkTagHelper should not use inline scripts #14736

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.

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

Open
mkArtakMSFT opened this issue Oct 4, 2019 · 3 comments
Open

LinkTagHelper should not use inline scripts #14736

mkArtakMSFT opened this issue Oct 4, 2019 · 3 comments
Labels
affected-few This issue impacts only small number of customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates breaking-change This issue / pr will introduce a breaking change, when resolved / merged. enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views Priority:1 Work that is critical for the release, but we could probably ship without severity-minor This label is used by an internal tool
Milestone

Comments

@mkArtakMSFT
Copy link
Member

As per security guidance we should make sure LinkTagHelper doesn't use inline-script.

Related issues: #11111 , #14104

@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one breaking-change This issue / pr will introduce a breaking change, when resolved / merged. area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Oct 4, 2019
@mkArtakMSFT mkArtakMSFT added this to the 5.0.0-preview1 milestone Oct 4, 2019
@DamianEdwards
Copy link
Member

We looked at this a long time back (I believe @SteveSandersonMS did some research) and concluded then that removing the current inline-script approach would likely break the feature, due to issues with loading order of the related resources (CSS and JavaScript files). It's worth another look to see if the current behavior can be maintained via another approach, but if it can't, we should consider different approaches to removing this, as it's potentially a very impactful breaking change, e.g. we could make the use of the related attributes on the effected Tag Helpers throw in 5.0 by default, and add an option to enable the behavior again. We could then remove them completely in 6.0.

@pranavkm pranavkm added affected-few This issue impacts only small number of customers severity-minor This label is used by an internal tool and removed cost: M labels Nov 6, 2020
@javiercn javiercn added the feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views label Apr 18, 2021
@pranavkm pranavkm modified the milestones: Backlog, .NET 7 Planning Oct 19, 2021
@pranavkm pranavkm added the Priority:1 Work that is critical for the release, but we could probably ship without label Oct 28, 2021
@pranavkm
Copy link
Contributor

pranavkm commented Nov 5, 2021

Triage notes: We should figure out if this is possible at all. Also the current best practices is to avoid CDN for serving static content, so perhaps the work here is to obsolete the fallback feature.

@Atulin
Copy link

Atulin commented May 30, 2022

Maybe a bit of a tangential proposition, but perhaps instead of completely obsoleting the feature, have the CDN files be downloaded on build time and replace the fallback URLs?

Just a wild thought

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates breaking-change This issue / pr will introduce a breaking change, when resolved / merged. enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views Priority:1 Work that is critical for the release, but we could probably ship without severity-minor This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

10 participants