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

Logger disable HTML escaping #4828

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Conversation

andrewwdye
Copy link
Contributor

Why are the changes needed?

When logging, html characters are escaped (JSONFormatter default). This prevents copying logged urls into the browser without modifying first.

What changes were proposed in this pull request?

This change sets logger's JSONFormatter to disable HTML escaping. While this may cause issues with HTML, this is a sane config for a logger where we don't intend to log HTML generally.

When logging with characters &, <, >

logger.Info(ctx, "foo&bar")

Before

{"json":{"src":"logger_test.go:619"},"level":"info","msg":"foo\u0026bar","ts":"2024-02-04T20:27:18-08:00"}

After

{"json":{"src":"logger_test.go:619"},"level":"info","msg":"foo&bar","ts":"2024-02-04T20:27:18-08:00"}

How was this patch tested?

Added a unit test

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Feb 5, 2024
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (aedde59) 58.51% compared to head (6aac4d7) 58.96%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4828      +/-   ##
==========================================
+ Coverage   58.51%   58.96%   +0.45%     
==========================================
  Files         493      644     +151     
  Lines       42771    55146   +12375     
==========================================
+ Hits        25026    32517    +7491     
- Misses      15749    20055    +4306     
- Partials     1996     2574     +578     
Flag Coverage Δ
unittests 58.96% <100.00%> (+0.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 5, 2024
@andrewwdye andrewwdye merged commit 05233f9 into master Feb 5, 2024
49 checks passed
@andrewwdye andrewwdye deleted the logger-disable-html-escaping branch February 5, 2024 18:54
pmahindrakar-oss pushed a commit that referenced this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants