Fixes CHRLY-B. Handle the case when the textures property is not presented in Mojang's response

This commit is contained in:
ErickSkrauch 2020-04-29 21:54:40 +03:00
parent 8001eab9db
commit 05c68c6ba6
7 changed files with 94 additions and 56 deletions

View File

@ -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.

View File

@ -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 {

View File

@ -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)
})
}

View File

@ -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
}

View File

@ -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
}

View File

@ -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)
}

View File

@ -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)
})