axmolengine / axmol

Axmol Engine – A Multi-platform Engine for Desktop, XBOX (UWP) and Mobile games. (A fork of Cocos2d-x-4.0)
https://axmol.dev
MIT License
868 stars 195 forks source link

Base 64 decode is broken #1032

Closed qoozta closed 1 year ago

qoozta commented 1 year ago

Steps to Reproduce:

  1. Run cpp-tests
  2. Go to any test using base64decode, e.g. 53:Renderer > 8:Bug-AutoCulling
rh101 commented 1 year ago

Please post as detailed information as you can when posting issues, such as what you expected and actual output etc. It's just in case we cannot reproduce the exact issue, but at least we'll know what to look for.

halx99 commented 1 year ago

https://github.com/cocos2d/cocos2d-x/blob/v4/cocos/base/base64.cpp#L55 caused by different behavior when got invalid character, maybe not base64 bug(cames from beast), maybe upper layer bug, e.g. CCSAXParser should always trim whitespace for axmol engine use case.

halx99 commented 1 year ago

Two solution(which one do you like or more suitable):

Solution A: Modify base64 decode

AX_DLL std::size_t
decode(void* dest, char const* src, std::size_t len)
{
    char* out = static_cast<char*>(dest);
    auto in = reinterpret_cast<unsigned char const*>(src);
    unsigned char c3[3], c4[4] = {0,0,0,0};
    int i = 0;
    int j = 0;

    auto const inverse = base64::get_inverse();

    while(len-- && *in != '=')
    {
        auto const v = inverse[*in];
        if (v == -1) 
        { /* Note: skip invalid chars without break, not standard behavior, but for compatible with old cocos2d-x resource file, e.g. TMX parser */
            ++in;
            continue;  // break;
        }
        ++in;
        c4[i] = v;
        if(++i == 4)
        {
            c3[0] =  (c4[0]        << 2) + ((c4[1] & 0x30) >> 4);
            c3[1] = ((c4[1] & 0xf) << 4) + ((c4[2] & 0x3c) >> 2);
            c3[2] = ((c4[2] & 0x3) << 6) +   c4[3];

            for(i = 0; i < 3; i++)
                *out++ = c3[i];
            i = 0;
        }
    }

    if(i)
    {
        c3[0] = ( c4[0]        << 2) + ((c4[1] & 0x30) >> 4);
        c3[1] = ((c4[1] & 0xf) << 4) + ((c4[2] & 0x3c) >> 2);
        c3[2] = ((c4[2] & 0x3) << 6) +   c4[3];

        for(j = 0; j < i - 1; j++)
            *out++ = c3[j];
    }

    return out - static_cast<char*>(dest);
}

Solution B: Modify CCSAXParser flag:

bool SAXParser::parseIntrusive(char* xmlData, size_t dataLength)
{
    SAX2Hander handler;
    handler.setSAXParserImp(this);

    try
    {
        xsxml::xml_sax3_parser::parse<xsxml::parse_normal | xsxml::parse_trim_whitespace>(xmlData, static_cast<int>(dataLength), handler);
        return true;
    }
    catch (xsxml::parse_error& e)
    {
        AXLOG("axmol: SAXParser: Error parsing xml: %s at %s", e.what(), e.where<char>());
        return false;
    }

    return false;
}
halx99 commented 1 year ago

Explicit different parse flag in different use case maybe better:

rh101 commented 1 year ago

Explicit different parse flag in different use case maybe better:

This is the preferred solution (solution B, with explicit flags as in your examples).

The only additional suggestion would be to add an assert or AXLOG message at the location where the error occurs, so that it can be caught early in a debug build.

For instance:

        auto const v = inverse[*in];
        if (v == -1) 
        { 
            AXLOG("Invalid character....") etc etc
            break;
        }
halx99 commented 1 year ago

Done fix