Validation message enhancements #6608
Conversation
…essage. PacakgesController converts those to JsonValidationMessage for Json responses and passes through IValidationMessage to Views for rendering. All PackageController Json responses use JsonValidationMessage in case of failure for consistency. ApiController always uses plain text version of the IValidationMessage. Updated tests to accomodate for the changes.
agr
requested review from
joelverhagen,
shishirx34,
skofman1,
cristinamanum,
loic-sharma,
ryuyu,
scottbommarito,
zhhyu,
chenriksson and
xavierdecoster
…es not emit raw HTML.
| @ViewHelpers.AlertDanger(@<text><span data-bind="text:$data"></span></text>) | ||
| @ViewHelpers.AlertDanger( | ||
| @<text> | ||
| <!-- ko if: typeof($data) === "string" --> |
There was a problem hiding this comment.
I am a bit conflicted on this check. It would allows us to provide good user experience if I messed up somewhere (i.e. forgot to update returned array of strings to array of JsonValidationMessage objects). On the other hand if I actually did mess up, it will hide it.
I thought about making the string - object to be the distinguishing factor for determining whether we produce text or raw HTML, but that would complicate things on C# side when we return an array of mixed responses.
| @ViewHelpers.AlertDanger( | ||
| @<text> | ||
| <!-- ko if: typeof($data) === "string" --> | ||
| <span data-bind="text:$data"></span> |
| We found the following issue(s): | ||
| <ul data-bind="foreach: $data.Warnings"> | ||
| <!-- ko if: typeof($data) === "string" --> | ||
| <li><span data-bind="text: $data"></span></li> |
| public bool HasRawHtmlRepresentation => true; | ||
|
|
||
| public string RawHtmlMessage | ||
| => HttpUtility.HtmlEncode( |
There was a problem hiding this comment.
HtmlEncode [](start = 27, length = 10)
Neither of these include HTML today.
You can linkify Account Settings if you want this, otherwise just make it plain text.
There was a problem hiding this comment.
Previous implementation contained a hack in PackagesController that made sure different message is displayed on Web UI vs the CLI. I removed that hack and created this class instead. It exists not to provide raw HTML of that message, but to contain that hack and the code here actively resists raw HTML in the strings. I wanted to emphasize that the strings are not expected to contain HTML code.
src/NuGetGallery/Infrastructure/HttpStatusCodeWithServerWarningResult.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| if (existingPackageRegistration.IsLocked) | ||
| { | ||
| return Json(HttpStatusCode.Forbidden, new[] { string.Format(CultureInfo.CurrentCulture, Strings.PackageIsLocked, existingPackageRegistration.Id) }); | ||
| return Json(HttpStatusCode.Forbidden, new[] { |
There was a problem hiding this comment.
Json [](start = 23, length = 4)
Thoughts on an overload here that accepts a string array still and maps to JsonValidationMessage?
There was a problem hiding this comment.
I thought about that, but that would be confusing. It would only exist in PackagesController and then similar code in other places would behave differently.
Progress on #6545.
I needed a way to enhance the errors and warning produced on a package upload and verify pages to allow passing raw HTML as an error, so I could include links in an error or warning message.
The gist of the change is that validation code now instead of returning errors and warnings as strings returns objects (implementing the
IValidationMessageinterface). Those object provide a way to render the message in either plain text or optional HTML. If object chooses not to provide HTML, its plain text representation is used instead. For API purposes plain text representation is always used. Errors passed throughTempData["Message"]are also always plain text (that's a bit more global change that I don't want to touch right now).Errors and warnings were passed as an array of strings in a JSON object to be rendered on client side (either through API, or baked into the page (search for
InProgressPackage)). This is replaced with passing an object instead that should either havePlainTextMessageorRawHtmlMessageproperties set and will render one of those preferring plain text if both provided. On C# side it is covered by theJsonValidationMessageobject.Given the inherent insecurity of pasting raw HTML into the output, some precautions were taken to not allow mindless HTML emitting: the default generic implementation of
IValidationMessage(PlainTextOnlyValidationMessagea simple wrapper to allow fast transition to passing objects instead of strings) only accepts plain text as an input and comments in theIValidationMessagestrongly discourage the implementation that would allow any HTML in. TheJsonValidationMessageconstructors either accept a string, treating it as a plain text (and JS code will encode it) or accept aIValidationMessagefrom which raw HTML will be taken if provided.The only non-generic implementation of
IValidationMessagethat is currently provided isPackageShouldNotBeSignedUserFixableValidationMessagewhich does not actually produce any raw HTML, but is used to provide different message when error is shown on a web page than if the error is passed in API response. previously that difference was hardcoded inPackagesControllerand required its own value inPackageValidationResultTypewhich was eliminated.