ClosestStorm / v8cgi

Automatically exported from code.google.com/p/v8cgi
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Binary module toString method reads beyond buffer end on Windows #100

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The following program...

var Buffer = require('binary').Buffer;
var buf = new Buffer('fred', 'UTF-8');
var str = buf.toString('UTF-8');
system.stdout("buf.length = " + buf.length + '\n');
system.stdout("str.length = " + str.length + '\n');
system.stdout("str = " + str + '\n');

...produces the following output on Windows...

buf.length = 4
str.length = 12
str = fred´+¢´+¢´+¢´+¢!´+¢2Y

Also, looking at binary.cc it seems that the charset argument is always ignored 
and utf-8 is assumed in every case.

Original issue reported on code.google.com by andy.bis...@gmail.com on 14 Oct 2011 at 10:06

GoogleCodeExporter commented 9 years ago
This only seems to happen when built with Visual Studio.  The distributed 
version works ok and I assume this was built with MinGW. (I cannot get it to 
build with MinGW so I cannot confirm this.)

Original comment by andy.bis...@gmail.com on 14 Oct 2011 at 10:58

GoogleCodeExporter commented 9 years ago
I will look into this.

The charset argument is used: it is passed to the view.transcode() call on line 
binary.cc:151.

Original comment by ondrej.zara on 14 Oct 2011 at 12:19

GoogleCodeExporter commented 9 years ago
I would say that the MSVC build has some issues with iconv. The whole 
toString() operation is basically an invocation of iconv; for some strange 
reasons, iconv decided that the output is a string longer than 4 characters.

What is the result of running all tests, e.g. "v8cgi unit/runner unit/tests" ?

And what issues do you encounter when building with MinGW?

Original comment by ondrej.zara on 14 Oct 2011 at 12:30

GoogleCodeExporter commented 9 years ago
I had overlooked the use of the charset argument and it is indeed used as you 
state.  I also completely agree with your diagnosis of the issue, it does seem 
that the iconv library is failing.

With regard to building under MinGW, it seems that scons defaults to using cl 
(the visual studio compiler) even when building from the MinGW shell.  If I add 
env.Tool('mingw') to explicitly use mingw then scons fails with an error that I 
thought had been fixed (see http://scons.tigris.org/issues/show_bug.cgi?id=2101)
(note that I am only using env.Tool('mingw') once in the sconstruct file.

$ /c/Python27/python /c/Python27/Scripts/scons.py
scons: Reading SConscript files ...
Checking for C header file sys/mman.h... (cached) no
Checking for C function sleep()... (cached) no
AttributeError: 'str' object has no attribute 'attributes':
  File "C:\v8cgi-0.9.1-src\v8cgi\SConstruct", line 447:
    build_binary(env)
etc..........

Original comment by andy.bis...@gmail.com on 14 Oct 2011 at 1:36

GoogleCodeExporter commented 9 years ago
Please stay tuned until I get to my MinGW box (which will be on Tuesday); I 
will then let you know about the configuration, versions etc. 

I have also seen your bugreport with missing symbols during linkage (at 
"Compiling" comments); I believe that should not be hard to fix.

Original comment by ondrej.zara on 14 Oct 2011 at 9:19

GoogleCodeExporter commented 9 years ago
So, I use 32-bit MinGW with GCC 4.4.0.

I use shared build of V8 (scons library=shared) as static does not work with 
v8cgi.

Please let me know how your compilation adventure continues :) If in doubt, 
join #v8cgi at freenode...

Original comment by ondrej.zara on 18 Oct 2011 at 12:06

GoogleCodeExporter commented 9 years ago
I now have v8 and v8cgi building under mingw.  I still cannot prevent
scons using visual studio if present but compiling on a VM with visual
studio not installed works OK.
I am only building the binary, socket, process and fs libraries as
these are all I need.
I am still not sure why it was not building. In the end I scrapped
everything and got it all from distributions again.

Andy

Original comment by andy.bis...@gmail.com on 19 Oct 2011 at 2:35

GoogleCodeExporter commented 9 years ago

Original comment by ondrej.zara on 14 Dec 2011 at 12:39