Skip to content
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

fix(std/testing): sanitize escape sequences from assertion errors #7516

Conversation

caspervonb
Copy link
Contributor

No description provided.

Copy link
Collaborator

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

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

I've been meaning to add Deno.InspectOptions::color = true, which can be set to false here, stopping Deno.inspect() from adding stylistic colour while preserving colour in the inspected value (so only they can be escaped like in this PR). We need that before we can apply this fix.

? stripColor(Deno.inspect(v, {
? Deno.inspect(v, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The intention of stripColor() here is to strip the colour that's added stylistically by Deno.inspect() based on types. This change will make regular objects look like this:

> deno eval 'console.log(Deno.inspect([123456789, 123456789, 123456789, 123456789, 123456789, 123456789, 123456789]).replaceAll("\x1b", "<esc>"))'
[
  <esc>[33m123456789<esc>[39m,
  <esc>[33m123456789<esc>[39m,
  <esc>[33m123456789<esc>[39m,
  <esc>[33m123456789<esc>[39m,
  <esc>[33m123456789<esc>[39m,
  <esc>[33m123456789<esc>[39m,
  <esc>[33m123456789<esc>[39m
]

Copy link
Contributor Author

@caspervonb caspervonb Sep 16, 2020

Choose a reason for hiding this comment

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

his change will make regular objects look

Currently only sanitizing string inputs so that's not the case as-is; but suppose there's an edge case there with objects that have string properties with escape sequences in them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But actually; this should be handled at a lower level so I'll end up reverting that.

@caspervonb
Copy link
Contributor Author

This should be fixed in inspect instead.

@caspervonb caspervonb closed this Sep 17, 2020
@caspervonb caspervonb deleted the fix-std-testing-sanitize-assertion-errors branch May 24, 2021 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants