jwt icon indicating copy to clipboard operation
jwt copied to clipboard

MarshalSingleStringAsArray global package variable

Open charlesdaniels opened this issue 1 year ago • 0 comments

I have noticed that the package github.com/golang-jwt/jwt/v5 has a global config flag MarshalSingleStringAsArray. This was previously discussed in #277, which was closed as fixed by #278. However, #278 did not actually remove the MarshalSingleStringAsArray as far as I can tell. It seems like folks were in agreement at the time that having global mutable state at the package level was not preferred.

Imagine a program that needs to use this library with different settings in different areas of the program

In fact, I am writing such a program.

I did spend a little time thinking on how this could be accomplished. This is in fact a somewhat tricky problem, because the custom MarshalJSON function has no way to directly receive extra parameters. I think however that the type system could be used to encode this information. I have created a simple proof of concept for how this could work which you can see on the Go Playground here, and which I have reproduced below for posterity:

// You can edit this code!
// Click here and start typing.
package main

import (
	"encoding/json"
	"fmt"
)

type ClaimStrings interface {
	GetClaims() []string
	SetClaims([]string)
}

var _ ClaimStrings = (*ClaimStringsMarshalSingletonAsString)(nil)
var _ ClaimStrings = (*ClaimStringsMarshalSingletonAsArray)(nil)

type ClaimStringsMarshalSingletonAsString struct {
	claims []string
}

func (c *ClaimStringsMarshalSingletonAsString) GetClaims() []string {
	return c.claims
}

func (c *ClaimStringsMarshalSingletonAsString) SetClaims(newClaims []string) {
	c.claims = newClaims
}

func (c *ClaimStringsMarshalSingletonAsString) MarshalJSON() ([]byte, error) {
	if len(c.claims) == 1 {
		return json.Marshal(c.claims[0])
	}
	return json.Marshal(c.claims)
}

type ClaimStringsMarshalSingletonAsArray struct {
	claims []string
}

func (c *ClaimStringsMarshalSingletonAsArray) GetClaims() []string {
	return c.claims
}

func (c *ClaimStringsMarshalSingletonAsArray) SetClaims(newClaims []string) {
	c.claims = newClaims
}

func (c *ClaimStringsMarshalSingletonAsArray) MarshalJSON() ([]byte, error) {
	return json.Marshal(c.claims)
}

func main() {
	fmt.Println("Hello, 世界")

	cases := []ClaimStrings{
		&ClaimStringsMarshalSingletonAsArray{claims: []string{"a", "b", "c"}},
		&ClaimStringsMarshalSingletonAsString{claims: []string{"a", "b", "c"}},
		&ClaimStringsMarshalSingletonAsArray{claims: []string{"x"}},
		&ClaimStringsMarshalSingletonAsString{claims: []string{"x"}},
	}

	for i, c := range cases {
		j, err := json.Marshal(c)
		if err != nil {
			panic(err)
		}
		fmt.Printf("---- case %d\ngo: %+v\njson: %s\n", i, c, j)
	}
}

I think the biggest problem I see is that this will break compatibility for existing API users. Perhaps there might be a way to make the existing ClaimStrings type implement the interface (which would then need to be named something different -- also how would SetClaims() work? maybe it would need to be immutable).

charlesdaniels avatar Oct 21 '24 20:10 charlesdaniels