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

converting between struct types with different struct tags breaks #801

Closed
Wezery opened this issue Oct 10, 2023 · 2 comments · Fixed by #805
Closed

converting between struct types with different struct tags breaks #801

Wezery opened this issue Oct 10, 2023 · 2 comments · Fixed by #805
Assignees
Labels
bug Something isn't working

Comments

@Wezery
Copy link

Wezery commented Oct 10, 2023

What version of Garble and Go are you using?

$ garble version
v0.10.2-0.20231009211313-45b591d8eb92
$ go version
go1.21.1 linux/amd64
go1.21.2 darwin/arm64

Faced struct cast issue with golang.org/x/crypto v0.14.0 ssh module

It uses two structs:

type pingMsg struct {
	Data string `sshtype:"192"`
}

type pongMsg struct {
	Data string `sshtype:"193"`
}

expected that they are "garbled" identically, but Data turns to garbled1 and garbled2.
Since in new ssh code used cast pongMsg(pingMsg) build fails with obvious error
:1: cannot convert kmohymSBLtRj (variable of type pingMsg) to type wWObn1U

During test founded this.
Example1. Builds and works fine

package main

import (
	"fmt"
)

type Test1 struct {
	Data string
}

type Test2 struct {
	Data string
}

func main() {
	t2 := Test2{Data:"2"}
	test := Test1(t2)
	fmt.Println(test.Data)
}

turns to

//line :1
package main

import (
	fmt "fw0ZR0h"
)

type UNSbt0XRHe struct {
	BPkrPRX string
}

type Ia2EJJoSk struct {
	BPkrPRX string
}

func main() {
	kE6sk8 := Ia2EJJoSk{BPkrPRX: "2"}
	j1aqHbggMN :=  /*line :1*/ UNSbt0XRHe(kE6sk8)
	 /*line :1*/ fmt.Ork_AO3wQag(j1aqHbggMN.BPkrPRX)
}

var _ = fmt.WspUe2S

Example2. Errors similar to ssh

package main

import (
	"fmt"
)

type Test1 struct {
	Data string `sshtype:"192"`
}

type Test2 struct {
	Data string `sshtype:"193"`
}

func main() {
	t2 := Test2{Data:"2"}
	test := Test1(t2)
	fmt.Println(test.Data)
}

turns to

//line :1
package main

import (
	fmt "fw0ZR0h"
)

type JeXhXvN struct {
	SN39xkW0LMFx string `sshtype:"192"`
}

type P4tGkaKgZ struct {
	WMEAOdA4bm string `sshtype:"193"`
}

func main() {
	neRq3vd0MLA := P4tGkaKgZ{WMEAOdA4bm: "2"}
	rGSFu5 :=  /*line :1*/ JeXhXvN(neRq3vd0MLA)
	 /*line :1*/ fmt.Ork_AO3wQag(rGSFu5.SN39xkW0LMFx)
}

var _ = fmt.WspUe2S

Text in back quotes mumbles logic of garble?

@mvdan mvdan changed the title Struct cast issue. Text in back quotes mumbles logic of garble? converting between struct types with different struct tags breaks Nov 12, 2023
@mvdan
Copy link
Member

mvdan commented Nov 12, 2023

Thanks for filing this bug - we've had a TODO about this edge case for some time, but you're the first one to run into the issue in practice :)

garble/hash.go

Lines 237 to 239 in 5e80f12

// TODO: We should probably strip field tags here.
// Do we need to do anything else to make a
// struct type "canonical"?

@mvdan mvdan self-assigned this Nov 12, 2023
@mvdan mvdan added the bug Something isn't working label Nov 12, 2023
mvdan added a commit to mvdan/garble-fork that referenced this issue Nov 12, 2023
This was a long standing TODO, and a user finally ran into it.
The fix isn't terribly straightforward, but I'm pretty happy with it.

Add a test case where the same struct field is identical
with no tag, with the "tagged1" json tag, and again with "tagged2".
While here, we add a test case for a regular named field too.

Fixes burrowers#801.
mvdan added a commit to mvdan/garble-fork that referenced this issue Nov 12, 2023
This was a long standing TODO, and a user finally ran into it.
The fix isn't terribly straightforward, but I'm pretty happy with it.

Add a test case where the same struct field is identical
with no tag, with the "tagged1" json tag, and again with "tagged2".
While here, we add a test case for a regular named field too.

Fixes burrowers#801.
pagran pushed a commit that referenced this issue Nov 13, 2023
This was a long standing TODO, and a user finally ran into it.
The fix isn't terribly straightforward, but I'm pretty happy with it.

Add a test case where the same struct field is identical
with no tag, with the "tagged1" json tag, and again with "tagged2".
While here, we add a test case for a regular named field too.

Fixes #801.
@pielgrzym
Copy link

pielgrzym commented Nov 14, 2023

fix works :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants