Fix #881 (#464): Added JSON encoder to disable escaping html symbols #887
Conversation
|
@felixfontein , @autrilla , could you please review PR and approve builds? |
autrilla
left a comment
autrilla
left a comment
There was a problem hiding this comment.
LGTM. Because this is technically a breaking change, I'd like @ajvb's LGTM too. Like I've mentioned before, I think this kind of breaking change is not really breaking, since we make no guarantees about the syntax of the output, and only about the semantics.
|
A test for this would be nice to have |
Ok, will work on it. |
|
Hmmm, looks like the tests are broken |
|
@autrilla , and looks like it's pgp one, which I didn't changed. |
|
I can't proceed with adding new tests as existing are not working properly (see #882) |
|
hi guys we have the same issue, any updates here ? |
|
@tanandy , sorry for delaying, but this PR still require unit test to be covered. I'll try to add them asap. |
|
Still blocked by #977. |
|
Have you tried one of the keys from this history tree? https://github.com/getsops/sops/commits/main/pgp/sops_functional_tests_key.asc |
|
@hiddeco , thanks for pointing me out, but correct me if I'm wrong: From what I see, there are no such key (the last one, gpg), even in history of repository. Did I missed something? |
|
stores/json/test_resources/example.json isn't used in the tests. |
Signed-off-by: Oleh Moskovych <[email protected]>
Moskovych
force-pushed
the
ISSUE-881
branch
from
cfe0db4 to
e8d1289
Compare
|
@felixfontein , github workflows are not running without any approvals. Can you help with it? |
| buffer := &bytes.Buffer{} | ||
| encoder := json.NewEncoder(buffer) | ||
| encoder.SetEscapeHTML(false) | ||
| err := encoder.Encode(v) |
There was a problem hiding this comment.
Encode appends a newline (https://pkg.go.dev/encoding/json#Encoder.Encode), which json.Marshal apparently does not do. (This makes the unit tests fail.) This is apparently because Encoder is for JSON streams, while Marshal produces a single JSON file.
There was a problem hiding this comment.
Simply adding code which removes a trailing newline (if exists) probably suffices to fix this. I don't see another way to do this with Go's API.
There was a problem hiding this comment.
Shouldn't we stick with newline in new files instead?
There was a problem hiding this comment.
This function is called from encodeValue and used during serialization of any JSON value, so if you use this to create JSON of a map, you get someting like
{
"foo"
: "bar"
,
"baz"
: 42
,
"bam"
: null
}
|
Any updates here? |
8 participants
Fixes #881
Fixes #464