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

Plaintext fields with control chars are not marshalled correctly in sparse structs #2

Closed
DavesPlanet opened this issue Oct 14, 2024 · 5 comments · Fixed by #5
Closed

Comments

@DavesPlanet
Copy link

DavesPlanet commented Oct 14, 2024

Chestnut cannot save a sparse struct with a plaintext field which contains the beginning and ending markers of a PGP public key block such as

"-----BEGIN PGP PUBLIC KEY BLOCK----- -----END PGP PUBLIC KEY BLOCK-----"

The field will be saved as blank.

I had wished for my public keys to be available to the .Sparse() function but the private key to require the full decryption of .Load()

The following test will reproduce the bug and will fail with "thingToSave did not match".

package repo

import (
	"testing"

	"github.com/jrapoport/chestnut/storage/nuts"

	"github.com/jrapoport/chestnut"
	"github.com/jrapoport/chestnut/encryptor/aes"
	"github.com/jrapoport/chestnut/encryptor/crypto"
)

func Test_Bug(t *testing.T) {

	store := nuts.NewStore("my.db")

	opt := chestnut.WithAES(crypto.Key256, aes.CFB, crypto.TextSecret("my secret"))

	cn := chestnut.NewChestnut(store, opt)
	if err := cn.Open(); err != nil {
		t.Error(err)
	}

	defer cn.Close()

	
	type myStruct struct {
		ThingToSave string
		OtherThing  string `json:"1,secure"`
	}

	s1 := "-----BEGIN PGP PUBLIC KEY BLOCK-----	-----END PGP PUBLIC KEY BLOCK-----"
	s2 := "some other thing"
	id := []byte{1}
	bucket := "MyBucket"

	s := myStruct{ThingToSave: s1, OtherThing: s2}

	err := cn.Save(bucket, id, s)
	if err != nil {
		t.Error(err)
	}

	fromDb := myStruct{}

	err = cn.Load(bucket, id, &fromDb)
	if err != nil {
		t.Error(err)
	}

	if fromDb.OtherThing != s.OtherThing {
		t.Error("otherThing did not match")
	}
	
	if fromDb.ThingToSave != s.ThingToSave {
		t.Error("thingToSave did not match")
	}
}
@jrapoport
Copy link
Owner

err = cn.Load(bucket, id, &fromDb)

should be

err = cn.Load(bucket, id, fromDb)

@DavesPlanet
Copy link
Author

I believe you are mistaken on two counts. Firstly
err = cn.Load(bucket, id, &fromDb)
is the correct syntax and does load data, setting it to what you suggest causes error. Second, if you run my original code and set s1 = "some thing to save" instead of PGP key headers then my original code passes. I am 100% certain this is a valid bug.

@jrapoport jrapoport reopened this Oct 21, 2024
@jrapoport jrapoport changed the title cannot save armored public key as plaintext Plaintext fields with control chars are not marshalled correctly in sparse structs Oct 21, 2024
@jrapoport
Copy link
Owner

I took another look and realized your issue was causes by the presence of a tab character \t in the plaintext string. Secure fields and non-sparse structs handle this correctly.

Looking into what is going on.

@jrapoport
Copy link
Owner

jrapoport commented Oct 21, 2024

looks like this is a known issue with json-iterator: #json-iterator/go#698

Thinking that a brute force workaround might be the best idea. The other option would be to fork json-iterator and fix whatever the issue is but that's not something I currently have time for.

@jrapoport
Copy link
Owner

not going to investigate past this to fully confirm but suspect the bug in json-iterator is related to the behavior of WriteString in json-iterator/stream_str.go:328

the standard string encoder starts off here:

// WriteString write string to stream without html escape
func (stream *Stream) WriteString(s string) {

but then the function ends with:

writeStringSlowPath(stream, i, s, valLen)

which (unfortunately) is an escaping function.

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.

2 participants