302430 - file_readln does not properly initialize realloc'd buffer

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

From [e-mail] on npm.jseng

<quote>
I was running into a problem using File.readln when the line in the
file contained more than 256 characters.  Some of the line was coming
out as junk, or as data from a previous line.

I tracked down the source of the problem to the function file_readln in
jsfile.c
(http://lxr.mozilla.org/seamonkey/source/js/src/jsfile.c#1649). When
the line exceeds the number of characters allocated in the buffer, we
re-allocate the buffer and copy the previous buffer's data
(http://lxr.mozilla.org/seamonkey/source/js/src/jsfile.c#1696).  But we
were not copying the proper number of bytes.  We were copying the
number of bytes equal to the returned value from JS_GetStringLength,
but that returns the number of double-byte Unicode characters, not the
number of bytes. Thus half of the original buffer was lost and the new
buffer was half full of junk.

Instead, since we already know the number of characters to copy
(offset), we should simply multiply that by the size of each character,
like we do in the call to allocate the buffer in the first place, and
send that to memcpy.

So, I changed the call to memcpy at lines 1696 and 1697 from this:

  memcpy(buf, JS_GetStringChars(file->linebuffer),
      JS_GetStringLength(file->linebuffer));

to this:

  memcpy(buf, file->linebuffer->chars, offset*sizeof data);

This copies the proper number of bytes, and the function works properly
for lines longer than 256 characters.
</quote>

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

While Dan's patch is fine, I don't really like file->linebuf at all, and since
it's only used by file_readln, it can go. I've done away with it with my patch
in bug 132949.

Comment 2

[Boris Zbarsky [:bzbarsky]]

So is this still an issue?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Yes, file->linebuf no longer exists.