Validate accessor type

[zvdm]

Hi, We found some interesting behavior of the glTF document unmarshaling. According to the official documentation, the accessor type can be only one of the seven values. But if we try to unmarshal and then marshal a document with an incorrect accessor type, we get unexpected results.

In the code, we can see the following

const (
    // AccessorScalar corresponds to a single dimension value.
    AccessorScalar AccessorType = iota
    // AccessorVec2 corresponds to a two dimensions array.
    AccessorVec2
    // AccessorVec3 corresponds to a three dimensions array.
    AccessorVec3
    // AccessorVec4 corresponds to a four dimensions array.
    AccessorVec4
    // AccessorMat2 corresponds to a 2x2 matrix.
    AccessorMat2
    // AccessorMat3 corresponds to a 3x3 matrix.
    AccessorMat3
    // AccessorMat4 corresponds to a 4x4 matrix.
    AccessorMat4
)

that means AccessorScalar is 0.

According to this block of code

// UnmarshalJSON unmarshal the accessor type with the correct default values.
func (a *AccessorType) UnmarshalJSON(data []byte) error {
    var tmp string
    err := json.Unmarshal(data, &tmp)
    if err == nil {
        *a = map[string]AccessorType{
            "SCALAR": AccessorScalar,
            "VEC2":   AccessorVec2,
            "VEC3":   AccessorVec3,
            "VEC4":   AccessorVec4,
            "MAT2":   AccessorMat2,
            "MAT3":   AccessorMat3,
            "MAT4":   AccessorMat4,
        }[tmp]
    }
    return err
}

the accessor type SCALAR will return us the value 0 and true as the second parameter (which is not handled here). But, if we pass to the function UnmarshalJSON an incorrect accessor type (like CUSTOM_TYPE) the map[string]AccessorType will return us also the value 0 (and false for the second parameter ok). The function UnmarshalJSON doesn't validate the second parameter of the map and that's why all incorrect accessor types will return the result like for SCALAR.

But, unfortunately, while marshaling the incorrect accessor type will be always parsed in SCALAR.

Could we fix it? Maybe we can validate the second parameter of the map[string]AccessorType and return an error if get false.

Also, I wrote some test cases to show the current described behavior.

func TestAccessorType_UnmarshalJSON(t *testing.T) {
    { // correct unmarshalling to 2 (for VEC3)
        typeStr := []byte(`"VEC3"`)
        var accType gltf.AccessorType = 0
        err := json.Unmarshal(typeStr, &accType)
        if err != nil {
            t.Errorf("Error while unmarshaling accessor type: %s, %s", string(typeStr), err)
        }
        if accType != gltf.AccessorVec3 {
            t.Errorf("Expected: %d, got: %d", gltf.AccessorVec3, accType)
        }
    }

    { // when the string of accessor type is incorrect, the value will be always 0 -> as a result, will be marshaled to SCALAR
        typeStr := []byte(`"CUSTOM_TYPE"`)
        var accType gltf.AccessorType = 100
        err := json.Unmarshal(typeStr, &accType)
        if err != nil {
            t.Errorf("Expected an error while unmarshaling accessor type: %s", string(typeStr))
        }
        if accType != gltf.AccessorScalar {
            t.Errorf("Expected: %d, got: %d", gltf.AccessorScalar, accType)
        }
    }
}

I've prepared the appropriate PR.

Thank you, in advance!

[qmuntal]

Thanks for spotting the issue and sending a fix PR. I'll review it and create a new version once it is merged.

[qmuntal]

Fixed in #47