diff --git a/CHANGELOG.md b/CHANGELOG.md index 237bfac..a1adda7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - New configuration params: `MOJANG_API_BASE_URL` and `MOJANG_SESSION_SERVER_BASE_URL`, that allow you to spoof Mojang API base addresses. +### Fixed +- Handle the case when there is no textures property in Mojang's response. + ### Changed - `ely.skinsystem.{hostname}.app.mojang_textures.usernames.round_time` timer will not be recorded if the iteration was empty. diff --git a/api/mojang/mojang.go b/api/mojang/mojang.go index 6f74b72..c64179f 100644 --- a/api/mojang/mojang.go +++ b/api/mojang/mojang.go @@ -3,11 +3,11 @@ package mojang import ( "bytes" "encoding/json" - "errors" "fmt" "io/ioutil" "net/http" "strings" + "sync" "time" ) @@ -19,14 +19,17 @@ var HttpClient = &http.Client{ } type SignedTexturesResponse struct { - Id string `json:"id"` - Name string `json:"name"` - Props []*Property `json:"properties"` + Id string `json:"id"` + Name string `json:"name"` + Props []*Property `json:"properties"` + + once sync.Once decodedTextures *TexturesProp + decodedErr error } func (t *SignedTexturesResponse) DecodeTextures() (*TexturesProp, error) { - if t.decodedTextures == nil { + t.once.Do(func() { var texturesProp string for _, prop := range t.Props { if prop.Name == "textures" { @@ -36,18 +39,18 @@ func (t *SignedTexturesResponse) DecodeTextures() (*TexturesProp, error) { } if texturesProp == "" { - return nil, errors.New("unable to find the textures property") + return } decodedTextures, err := DecodeTextures(texturesProp) if err != nil { - return nil, err + t.decodedErr = err + } else { + t.decodedTextures = decodedTextures } + }) - t.decodedTextures = decodedTextures - } - - return t.decodedTextures, nil + return t.decodedTextures, t.decodedErr } type Property struct { diff --git a/api/mojang/mojang_test.go b/api/mojang/mojang_test.go index b5f2001..b9c1124 100644 --- a/api/mojang/mojang_test.go +++ b/api/mojang/mojang_test.go @@ -32,7 +32,7 @@ func TestSignedTexturesResponse(t *testing.T) { Props: []*Property{}, } textures, err := obj.DecodeTextures() - testify.Errorf(t, err, "unable to find the textures property") + testify.Nil(t, err) testify.Nil(t, textures) }) } diff --git a/http/skinsystem.go b/http/skinsystem.go index ed03b80..ab77070 100644 --- a/http/skinsystem.go +++ b/http/skinsystem.go @@ -2,7 +2,6 @@ package http import ( "encoding/json" - "errors" "io" "net/http" "strings" @@ -67,6 +66,11 @@ func (ctx *Skinsystem) skinHandler(response http.ResponseWriter, request *http.R } texturesProp, _ := mojangTextures.DecodeTextures() + if texturesProp == nil { + response.WriteHeader(http.StatusNotFound) + return + } + skin := texturesProp.Textures.Skin if skin == nil { response.WriteHeader(http.StatusNotFound) @@ -105,6 +109,11 @@ func (ctx *Skinsystem) capeHandler(response http.ResponseWriter, request *http.R } texturesProp, _ := mojangTextures.DecodeTextures() + if texturesProp == nil { + response.WriteHeader(http.StatusNotFound) + return + } + cape := texturesProp.Textures.Cape if cape == nil { response.WriteHeader(http.StatusNotFound) @@ -164,8 +173,7 @@ func (ctx *Skinsystem) texturesHandler(response http.ResponseWriter, request *ht texturesProp, _ := mojangTextures.DecodeTextures() if texturesProp == nil { - ctx.Emit("skinsystem:error", errors.New("unable to find textures property")) - apiServerError(response) + response.WriteHeader(http.StatusNoContent) return } diff --git a/http/skinsystem_test.go b/http/skinsystem_test.go index f2cb229..cd04de4 100644 --- a/http/skinsystem_test.go +++ b/http/skinsystem_test.go @@ -166,7 +166,7 @@ var skinsTestsCases = []*skinsystemTestCase{ Name: "Username doesn't exists on the local storage, but exists on Mojang and has textures", BeforeTest: func(suite *skinsystemTestSuite) { suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponse(true, false), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponseWithTextures(true, false), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(301, response.StatusCode) @@ -174,10 +174,20 @@ var skinsTestsCases = []*skinsystemTestCase{ }, }, { - Name: "Username doesn't exists on the local storage, but exists on Mojang and has no textures", + Name: "Username doesn't exists on the local storage, but exists on Mojang and has no skin texture", BeforeTest: func(suite *skinsystemTestSuite) { suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponse(false, false), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponseWithTextures(false, false), nil) + }, + AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { + suite.Equal(404, response.StatusCode) + }, + }, + { + Name: "Username doesn't exists on the local storage, but exists on Mojang and has an empty properties", + BeforeTest: func(suite *skinsystemTestSuite) { + suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createEmptyMojangResponse(), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(404, response.StatusCode) @@ -270,7 +280,7 @@ var capesTestsCases = []*skinsystemTestCase{ Name: "Username doesn't exists on the local storage, but exists on Mojang and has textures", BeforeTest: func(suite *skinsystemTestSuite) { suite.CapesRepository.On("FindCapeByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponse(true, true), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponseWithTextures(true, true), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(301, response.StatusCode) @@ -278,10 +288,20 @@ var capesTestsCases = []*skinsystemTestCase{ }, }, { - Name: "Username doesn't exists on the local storage, but exists on Mojang and has no textures", + Name: "Username doesn't exists on the local storage, but exists on Mojang and has no cape texture", BeforeTest: func(suite *skinsystemTestSuite) { suite.CapesRepository.On("FindCapeByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponse(false, false), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponseWithTextures(false, false), nil) + }, + AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { + suite.Equal(404, response.StatusCode) + }, + }, + { + Name: "Username doesn't exists on the local storage, but exists on Mojang and has an empty properties", + BeforeTest: func(suite *skinsystemTestSuite) { + suite.CapesRepository.On("FindCapeByUsername", "mock_username").Return(nil, nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createEmptyMojangResponse(), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(404, response.StatusCode) @@ -439,7 +459,7 @@ var texturesTestsCases = []*skinsystemTestCase{ BeforeTest: func(suite *skinsystemTestSuite) { suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) suite.CapesRepository.On("FindCapeByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Once().Return(createMojangResponse(true, true), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Once().Return(createMojangResponseWithTextures(true, true), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(200, response.StatusCode) @@ -456,11 +476,22 @@ var texturesTestsCases = []*skinsystemTestCase{ }, }, { - Name: "Username not exists, but Mojang profile available, but there is no textures", + Name: "Username not exists, but Mojang profile available, but there is an empty skin and cape textures", BeforeTest: func(suite *skinsystemTestSuite) { suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) suite.CapesRepository.On("FindCapeByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Once().Return(createMojangResponse(false, false), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Once().Return(createMojangResponseWithTextures(false, false), nil) + }, + AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { + suite.Equal(204, response.StatusCode) + }, + }, + { + Name: "Username not exists, but Mojang profile available, but there is an empty properties", + BeforeTest: func(suite *skinsystemTestSuite) { + suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) + suite.CapesRepository.On("FindCapeByUsername", "mock_username").Return(nil, nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Once().Return(createEmptyMojangResponse(), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(204, response.StatusCode) @@ -567,7 +598,7 @@ var signedTexturesTestsCases = []*signedTexturesTestCase{ AllowProxy: true, BeforeTest: func(suite *skinsystemTestSuite) { suite.SkinsRepository.On("FindSkinByUsername", "mock_username").Return(nil, nil) - suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponse(true, false), nil) + suite.MojangTexturesProvider.On("GetForUsername", "mock_username").Return(createMojangResponseWithTextures(true, false), nil) }, AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { suite.Equal(200, response.StatusCode) @@ -666,7 +697,15 @@ func createCapeModel() *model.Cape { return &model.Cape{File: bytes.NewReader(createCape())} } -func createMojangResponse(includeSkin bool, includeCape bool) *mojang.SignedTexturesResponse { +func createEmptyMojangResponse() *mojang.SignedTexturesResponse { + return &mojang.SignedTexturesResponse{ + Id: "00000000000000000000000000000000", + Name: "mock_username", + Props: []*mojang.Property{}, + } +} + +func createMojangResponseWithTextures(includeSkin bool, includeCape bool) *mojang.SignedTexturesResponse { timeZone, _ := time.LoadLocation("Europe/Minsk") textures := &mojang.TexturesProp{ Timestamp: time.Date(2019, 4, 27, 23, 56, 12, 0, timeZone).Unix(), @@ -687,16 +726,11 @@ func createMojangResponse(includeSkin bool, includeCape bool) *mojang.SignedText } } - response := &mojang.SignedTexturesResponse{ - Id: "00000000000000000000000000000000", - Name: "mock_username", - Props: []*mojang.Property{ - { - Name: "textures", - Value: mojang.EncodeTextures(textures), - }, - }, - } + response := createEmptyMojangResponse() + response.Props = append(response.Props, &mojang.Property{ + Name: "textures", + Value: mojang.EncodeTextures(textures), + }) return response } diff --git a/mojangtextures/in_memory_textures_storage.go b/mojangtextures/in_memory_textures_storage.go index d5b7957..7a95a4d 100644 --- a/mojangtextures/in_memory_textures_storage.go +++ b/mojangtextures/in_memory_textures_storage.go @@ -1,12 +1,9 @@ package mojangtextures import ( - "fmt" "sync" "time" - "github.com/getsentry/raven-go" - "github.com/elyby/chrly/api/mojang" "github.com/tevino/abool" @@ -80,19 +77,6 @@ func (s *InMemoryTexturesStorage) StoreTextures(uuid string, textures *mojang.Si if textures != nil { decoded, err := textures.DecodeTextures() if err != nil { - tags := map[string]string{ - "textures.id": textures.Id, - "textures.name": textures.Name, - } - - for i, prop := range textures.Props { - tags[fmt.Sprintf("textures.props[%d].name", i)] = prop.Name - tags[fmt.Sprintf("textures.props[%d].value", i)] = prop.Value - tags[fmt.Sprintf("textures.props[%d].signature", i)] = prop.Signature - } - - raven.CaptureErrorAndWait(err, tags) - panic(err) } diff --git a/mojangtextures/in_memory_textures_storage_test.go b/mojangtextures/in_memory_textures_storage_test.go index ef0f831..ac244ec 100644 --- a/mojangtextures/in_memory_textures_storage_test.go +++ b/mojangtextures/in_memory_textures_storage_test.go @@ -111,12 +111,18 @@ func TestInMemoryTexturesStorage_StoreTextures(t *testing.T) { t.Run("should panic if textures prop is not decoded", func(t *testing.T) { toStore := &mojang.SignedTexturesResponse{ - Id: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", - Name: "mock", - Props: []*mojang.Property{}, + Id: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + Name: "mock", + Props: []*mojang.Property{ + { + Name: "textures", + Value: "totally not base64 encoded json", + Signature: "totally not base64 encoded signature", + }, + }, } - assert.PanicsWithError(t, "unable to find the textures property", func() { + assert.Panics(t, func() { storage := NewInMemoryTexturesStorage() storage.StoreTextures("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", toStore) })