Encoding of JSON file contains unicode representation instead of symbol

Moskovych commented 3 years ago

Relations

Possible related to issues: #464 & #740

Issue

Inputs

Currently, having simple JSON file, like (test.json):

{
    "TestValue": "test <test@test.test>",
    "TestValue2": "&",
    "TestValue3": "@"
}

Using the .sops.yaml file with KMS defined and command:

sops -e -i test.json

And decrypt:

sops -d --input-type=json -i test.json

Results

After decrypting the input file doesn't decode all symbols, like (this is founds only, might be more):

and result is the following:

{
    "TestValue": "test \u003ctest@test.test\u003e",
    "TestValue2": "\u0026",
    "TestValue3": "@"
}

It might be a huge issue, while the example in the first value should represent the proper email info, like: User Name <user@example.org> instead of User Name \u003cuser@example.org\u003e.

Environment

SOPS version: sops 3.7.1 (latest)
Cloud: AWS / KMS
OS: MacOS Big Sur (11.3.1)

Expected results

The JSON values should be decoded in the same manner for all symbols to avoid data discrepancy.

felixfontein commented 3 years ago

Looks like a duplicate of #464. The result is semantically equivalent (as a JSON file) to the original data.

Moskovych commented 3 years ago

@felixfontein , yep, I've mentioned that issue too, but don't see any progress on it. Would it will be possible to control such behavior? Kind a flag? Or better parameter for .sops.yaml configuration file?

felixfontein commented 3 years ago

@Moskovych I would assume it would be a CLI parameter. I guess the discussion should rather continue in #464 though, and this issue should probably be closed.

Moskovych commented 3 years ago

I would like to keep it and probably will try to implement that feature.

Looking to the code, I've found the place where in goes (line 159):

https://github.com/mozilla/sops/blob/66043e71a81787d6513bc2e5505a29aac67dc6f1/stores/json/store.go#L152-L161

So, by default it can't be changed due to (line 161):

https://github.com/golang/go/blob/master/src/encoding/json/encode.go#L158-L170

And it can be replaced by something like:

func marshal(t interface{}) ([]byte, error) {
    buffer := &bytes.Buffer{}
    encoder := json.NewEncoder(buffer)
    encoder.SetEscapeHTML(false)
    err := encoder.Encode(t)
    return buffer.Bytes(), err
}

I've tested it locally on MacOS, it works.

But I would like to add it as flag to sops and possibly in the config file. I'm not Golang expert. So, who can point me to the right direction how to do it in better way to avoid violation of universality of the code?

@felixfontein , what do you think?

autrilla commented 3 years ago

For something like this, I’d say not even a flag is needed. My guess is most people would like unescaped JSON, and we make no guarantees about the output, other than keeping it semantically the same. So it’s fine to change this without a flag behind it.

On Mon, 7 Jun 2021 at 14:33, Oleh Moskovych @.***> wrote:

I would like to keep it and probably will try to implement that feature.

Looking to the code, I've found the place where in goes (line 159):

https://github.com/mozilla/sops/blob/66043e71a81787d6513bc2e5505a29aac67dc6f1/stores/json/store.go#L152-L161

So, by default it can be changed due to (line 161):

https://github.com/golang/go/blob/master/src/encoding/json/encode.go#L158-L170

And it can be replaced by something like:

func marshal(t interface{}) ([]byte, error) { buffer := &bytes.Buffer{} encoder := json.NewEncoder(buffer) encoder.SetEscapeHTML(false) err := encoder.Encode(t) return buffer.Bytes(), err }

I've tested it locally on MacOS, it works.

But I would like to add it as flag to sops and possibly in the config file. I'm not Golang expert. So, who can point me to the right direction how to do it in better way to avoid violation of universality of the code?

@felixfontein https://github.com/felixfontein , what do you think?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mozilla/sops/issues/881#issuecomment-855886131, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARH4VYHMPDHAJXA3QHEO3DTRS4C3ANCNFSM4565MHYA .

Moskovych commented 3 years ago

@autrilla , wouldn't it be a breaking change switching it without flag? We have no guarantee that people not rely on it, or have?

autrilla commented 3 years ago

We don’t consider changes in the style of the file to be breaking.

On Mon, 7 Jun 2021 at 17:18, Oleh Moskovych @.***> wrote:

@autrilla https://github.com/autrilla , wouldn't it be a breaking change switching it without flag? We have no guarantee that people not rely on it, or have?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/mozilla/sops/issues/881#issuecomment-856025027, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARH4VZXKOS2MZ27MNUQS6LTRTPMTANCNFSM4565MHYA .

Moskovych commented 3 years ago

@autrilla , ok, then I'll prepare the PR with change. Would you be able to approve builds workflow?

sirantd commented 1 year ago

Any updates on that? Would be nice to have it solved.

Moskovych commented 1 year ago

PR #887 is open and blocked by #977. Can't proceed with unit tests.

getsops / sops