superset icon indicating copy to clipboard operation
superset copied to clipboard

fix(frontend): allow "constructor" property in response data

Open SpencerTorres opened this issue 1 year ago • 7 comments

SUMMARY

Fixes #23953

Allows response JSON to be parsed with a property containing the name "constructor". The json-bigint module has some safety options in place to prevent prototype pollution, but it prevents the queries from loading data where the columns contain constructor in the name/value. For example, test_constructor and constructor would both throw an error.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

See #23953 for example query

ADDITIONAL INFORMATION

  • [x] Has associated issue: Fixes #23953
  • [ ] Required feature flags:
  • [ ] Changes UI
  • [ ] Includes DB Migration (follow approval process in SIP-59)
    • [ ] Migration is atomic, supports rollback & is backwards-compatible
    • [ ] Confirm DB migration upgrade and downgrade tested
    • [ ] Runtime estimates and downtime expectations provided
  • [ ] Introduces new feature or API
  • [ ] Removes existing feature or API

SpencerTorres avatar Sep 26 '23 02:09 SpencerTorres

Thanks for the rebase @SpencerTorres :) Running CI now and added a few reviewers. Hopefully we can get this across the finish line and close out the issue!

rusackas avatar Mar 12 '24 21:03 rusackas

looks like CI was broken on master when I pulled. Updated again

SpencerTorres avatar Mar 13 '24 17:03 SpencerTorres

Looks like master is consistently failing to run, however I did fix a new one-line lint issue related to test code added in this commit. Updated again

SpencerTorres avatar Mar 14 '24 16:03 SpencerTorres

CI running... fingers crossed!

rusackas avatar Mar 14 '24 16:03 rusackas

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.39%. Comparing base (2948abc) to head (cee73c4). Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #25407   +/-   ##
=======================================
  Coverage   67.39%   67.39%           
=======================================
  Files        1909     1909           
  Lines       74736    74738    +2     
  Branches     8326     8326           
=======================================
+ Hits        50367    50369    +2     
  Misses      22319    22319           
  Partials     2050     2050           
Flag Coverage Δ
javascript 57.22% <100.00%> (+<0.01%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 14 '24 16:03 codecov[bot]

Looks like CI is happy now! Re-pinging @betodealmeida @john-bodley @villebro to see if they have a moment to help with a review.

rusackas avatar Mar 14 '24 17:03 rusackas

@SpencerTorres would you be able to rebase the PR? The mandatory CI workflows have changed since CI last run, causing a few of them to now be missing.

villebro avatar Apr 05 '24 13:04 villebro

kicking off CI. Fingers crossed!

rusackas avatar Apr 06 '24 02:04 rusackas