Crashing the Engine with a NULL in a required file..

[NathanaelA]

Hey, I know it is totally a corner case; but I ran into it. 😀 and it took me a while to figure out what was going on.

If I do a

require('some-file.js');

and some-file.js has a null character anywhere in it (even in a string surrounded by quotes or a comment) ; the program will die a horrible death.

The issue appears to be https://github.com/NativeScript/android-runtime/blob/master/src/jni/File.cpp#L27 here. You are passing in the length of the file as loaded; however this is a string variable where a null is actually a terminating character.

I'm not sure if this is the proper solution; but if you eliminate the "len" on line 27 and you update the actual function ReadText that loads the data to be:

       char* File::ReadText(const string& filePath, int& charLength, bool& isNew)
        {
                FILE *file = fopen(filePath.c_str(), "rb");
                fseek(file, 0, SEEK_END);

                charLength = ftell(file);
                isNew = charLength >= BUFFER_SIZE;

                rewind(file);

                if(isNew)
                {
                        char* newBuffer = new char[charLength+1];
                        fread(newBuffer, 1, charLength, file);
                        fclose(file);
                        newBuffer[charLength] = 0;

                        return newBuffer;
                }

                fread(Buffer, 1, charLength, file);
                fclose(file);
                Buffer[charLength] = 0;

                return Buffer;
        }

Then everything works as expected as the null character actually terminates the file since IMHO it is actually supposed to be a text/string file...

To be honest I'm not sure if the better behavior is to truncate the file at a null; but it is better than having the engine crashing if a null is present. (And having the engine NOT crash, is a GOOD thing...)

Oh, and as a side effect, benchmarks appear to show that using the above modification to using String s(buffer) is FASTER than doing the String s(buffer, len); I'm not sure why; I was actually surprised; but on almost EVERY single core file (which of course a ton are loaded) there is a 10-30% speed increase for each file converted from a char array to the std:string... I have code in my version timing it..

I personally REALLY prefer truncating the file at a NULL; but I understand this is a issue that you guys will need to talk about; but if you agree that the file should be truncated at the null; I can give you a patch with the above code...

[slavchev]

Hi @NathanaelA

I managed to merge a last-minute PR with a fix for this issue. Right now we are preparing our release package and I didn't have much time to research the issue in depth but the simplest unit test is passing. If this change doesn't work for case then we will release version 1.5.1 in a week or two.

[jasssonpet]

Hello, @NathanaelA,

Have you tested this scenario on the iOS runtime? We were curious if there were any issues there.

Edit: It seems that it's not crashing, but also not working properly on iOS. Feel free to open a new issue on this.

[slavchev]

Thanks @NathanaelA. I forgot that one.