fix: SpecReader to escape embedded JSON files#733
fix: SpecReader to escape embedded JSON files#733disq wants to merge 6 commits intocloudquery:mainfrom disq:fix/specreader-escape-json-embeds
Conversation
Should fix #9094
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #733 +/- ##
==========================================
+ Coverage 48.82% 48.89% +0.07%
==========================================
Files 70 70
Lines 6923 6933 +10
==========================================
+ Hits 3380 3390 +10
Misses 3077 3077
Partials 466 466
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
⏱️ Benchmark resultsComparing with c9f95a5
|
| @@ -0,0 +1 @@ | |||
| {"key": "foo", "secret": "bar"} No newline at end of file | |||
There was a problem hiding this comment.
Let's test a JSON file with multiple lines as well (or only a file with multiple lines - if that works a file with a single line should also work)
| encoder.SetEscapeHTML(false) | ||
| expandErr = encoder.Encode(string(content)) | ||
| b := buffer.Bytes() | ||
| if l := len(b); l > 0 && b[l-1] == '\n' { |
There was a problem hiding this comment.
encoder.Encode always adds the trailing \n, there's no way to unset EscapeHTML otherwise.
| t.Fatal(err) | ||
| } | ||
| if runtime.GOOS == "windows" { | ||
| expectedCfg = bytes.ReplaceAll(expectedCfg, []byte(`\n`), []byte(`\r\n`)) |
There was a problem hiding this comment.
These are escaped sequences and not actual newline characters, so 2 and 4 bytes in length respectively.
There was a problem hiding this comment.
This is quite horrible 😅 , but also creative, and it solves a problem for our users. I still think we need to ban the use of file variables to expand yaml (see discussion in https://github.com/cloudquery/plugin-sdk/issues/480), but until we do that this should do 👍
Co-authored-by: candiduslynx <candiduslynx@users.noreply.github.com>
|
302 moved: cloudquery/plugin-pb-go#4 |
3 participants
Should fix #9094