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

Encoding of JSON file contains unicode representation instead of symbol #881

Open
Moskovych opened this issue Jun 2, 2021 · 10 comments · May be fixed by #887
Open

Encoding of JSON file contains unicode representation instead of symbol #881

Moskovych opened this issue Jun 2, 2021 · 10 comments · May be fixed by #887

Comments

@Moskovych
Copy link

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
Copy link
Contributor

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

@Moskovych
Copy link
Author

Moskovych commented Jun 2, 2021

@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
Copy link
Contributor

@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
Copy link
Author

Moskovych commented Jun 7, 2021

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
Copy link
Contributor

autrilla commented Jun 7, 2021 via email

@Moskovych
Copy link
Author

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

@Moskovych Moskovych changed the title Decoding of JSON file contains unicode representation instead of symbol Encoding of JSON file contains unicode representation instead of symbol Jun 7, 2021
@autrilla
Copy link
Contributor

autrilla commented Jun 7, 2021 via email

@Moskovych
Copy link
Author

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

@sirantd
Copy link

sirantd commented Apr 28, 2023

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

@Moskovych
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants