From 2e0496c1f9faccb47df0914fde868cbf0a3ff285 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 3 Apr 2021 14:02:37 -0700 Subject: [PATCH] Add note about handling of UTF-8 sequences in properties files. --- parser/parser.go | 49 +++++++++++++++++++++++++++++++++++++---- router/router_server.go | 5 ----- server/server.go | 10 ++++----- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/parser/parser.go b/parser/parser.go index db0f30f..678d988 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -35,16 +35,37 @@ type ReplaceValue struct { valueType jsonparser.ValueType } +// Value returns the underlying value of the replacement. Be aware that this +// can include escaped UTF-8 sequences that will need to be handled by the caller +// in order to avoid accidentally injecting invalid sequences into the running +// process. +// +// For example the expected value may be "§Foo" but you'll be working directly +// with "\u00a7FOo" for this value. This will cause user pain if not solved since +// that is clearly not the value they were expecting to be using. func (cv *ReplaceValue) Value() []byte { return cv.value } +// Type returns the underlying data type for the Value field. func (cv *ReplaceValue) Type() jsonparser.ValueType { return cv.valueType } +// String returns the value as a string representation. This will automatically +// handle casting the UTF-8 sequence into the expected value, switching something +// like "\u00a7Foo" into "§Foo". func (cv *ReplaceValue) String() string { - str, _ := jsonparser.ParseString(cv.value) + if cv.Type() != jsonparser.String { + if cv.Type() == jsonparser.Null { + return "" + } + return "" + } + str, err := jsonparser.ParseString(cv.value) + if err != nil { + panic(errors.Wrap(err, "parser: could not parse value")) + } return str } @@ -95,15 +116,16 @@ func (f *ConfigurationFile) UnmarshalJSON(data []byte) error { return nil } -// Defines a single find/replace instance for a given server configuration file. +// ConfigurationFileReplacement defines a single find/replace instance for a +// given server configuration file. type ConfigurationFileReplacement struct { Match string `json:"match"` IfValue string `json:"if_value"` ReplaceWith ReplaceValue `json:"replace_with"` } -// Handles unmarshaling the JSON representation into a struct that provides more useful -// data to this functionality. +// UnmarshalJSON handles unmarshaling the JSON representation into a struct that +// provides more useful data to this functionality. func (cfr *ConfigurationFileReplacement) UnmarshalJSON(data []byte) error { m, err := jsonparser.GetString(data, "match") if err != nil { @@ -494,6 +516,25 @@ func (f *ConfigurationFile) parsePropertiesFile(path string) error { s.WriteString(key) s.WriteByte('=') + // There is no winning with this logic. This fixes a bug where users with hand + // rolled UTF-8 escape sequences would have all sorts of pain in their configurations + // because we were writing the UTF-8 literal characters which their games could + // not actually handle. + // + // However, by adding this fix to only store the escaped UTF-8 sequence we unwittingly + // introduced a "regression" that causes _other_ games to have issues because they + // can only handle the unescaped representations. I cannot think of a simple approach + // to this problem that doesn't just lead to more complicated cases and problems. + // + // So, if your game cannot handle parsing UTF-8 sequences that are escaped into + // the string, well, sucks. There are fewer of those games than there are games + // that have issues parsing the raw UTF-8 sequence into a string? Also how does one + // really know what the user intended at this point? We'd need to know if the value + // was escaped or not to begin with before setting it, which I suppose can work but + // jesus that is going to be some annoyingly complicated logic? + // + // @see https://github.com/pterodactyl/panel/issues/2308 (original) + // @see https://github.com/pterodactyl/panel/issues/3009 ("bug" introduced as result) s.WriteString(strings.Trim(strconv.QuoteToASCII(value), `"`)) s.WriteString("\n") } diff --git a/router/router_server.go b/router/router_server.go index 17d14b3..ec01483 100644 --- a/router/router_server.go +++ b/router/router_server.go @@ -16,11 +16,6 @@ import ( "github.com/pterodactyl/wings/server" ) -type serverProcData struct { - server.ResourceUsage - Suspended bool `json:"suspended"` -} - // Returns a single server from the collection of servers. func getServer(c *gin.Context) { s := ExtractServer(c) diff --git a/server/server.go b/server/server.go index 0bbb336..e485ed9 100644 --- a/server/server.go +++ b/server/server.go @@ -143,12 +143,12 @@ func (s *Server) Log() *log.Entry { return log.WithField("server", s.Id()) } -// Syncs the state of the server on the Panel with Wings. This ensures that we're always -// using the state of the server from the Panel and allows us to not require successful -// API calls to Wings to do things. +// Sync syncs the state of the server on the Panel with Wings. This ensures that +// we're always using the state of the server from the Panel and allows us to +// not require successful API calls to Wings to do things. // -// This also means mass actions can be performed against servers on the Panel and they -// will automatically sync with Wings when the server is started. +// This also means mass actions can be performed against servers on the Panel +// and they will automatically sync with Wings when the server is started. func (s *Server) Sync() error { cfg, err := s.client.GetServerConfiguration(s.Context(), s.Id()) if err != nil {