LilyPad / GoLilyPad

GNU General Public License v3.0
98 stars 58 forks source link

Make sessionserver configurable. #73

Closed CoreyShupe closed 3 years ago

CoreyShupe commented 3 years ago

https://github.com/LilyPad/GoLilyPad/blob/224e730cbaeac3191c36be7f6309330d1897e198/server/proxy/auth/constants.go#L4

It would be nice to have this URL format come from a sys environment variable defaulting to the constant rather than having it static.

This could probably be done by querying the environment here: response, err := http.Get(fmt.Sprintf(URL, name, MojangSha1Hex([]byte(serverId), sharedSecret, publicKey)))

coelho commented 3 years ago

Hi @CoreyShupe ! Thanks for the input.

Pseudo-code I imagine is:

var URL = "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s"

func init() {
  url := os.GetEnv(...)
  if len(url) > 0 {
     URL = url
  }
}

Improved string arguments (vs %s) may also be necessary? What do you think?

CoreyShupe commented 3 years ago

Hi @CoreyShupe ! Thanks for the input.

* What is your suggestion for the environment variable?

* Would you be able to do a PR for this?

Pseudo-code I imagine is:

var URL = "https://sessionserver.mojang.com/session/minecraft/hasJoined?username=%s&serverId=%s"

func init() {
  url := os.GetEnv(...)
  if len(url) > 0 {
     URL = url
  }
}

Improved string arguments (vs %s) may also be necessary? What do you think?

1) My suggestion would be something like MOJANG_SESSION_SERVER_URL 2) I could scratch up a PR (I'm a garbage go dev but I could make something work)

The psuedo code you provided could work well, that begs the question of if we want to keep it as a constant there or move it into location it's used and use something like this:

    if value, ok := os.LookupEnv(key); ok {
        return value
    }
    return fallback
}

from SO: https://stackoverflow.com/questions/40326540/how-to-assign-default-value-if-env-var-is-empty

That would allow us to define the static as a potential fallback instead of defining it statically. Of course environment variables aren't necessarily static but for the purpose of containerized environments I could see it working out as a constant.

I would have to touch up a go playground a bit to figure out how to format it, but it seems like a good idea.

coelho commented 3 years ago

@CoreyShupe

coelho commented 3 years ago

74