Closed Bug 21568 Opened 25 years ago Closed 25 years ago

xpconnect and returning wrong size strings from nsIFileSpec

Categories

(Core :: XPCOM, defect, P3)

x86
Windows NT
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: 3jrgm, Assigned: dougt)

References

()

Details

Attachments

(1 file)

The attachment (to follow) seems to a be a small bug with either XPConnect
(in populating [out] values), or perhaps nsIFileSpec. That is, it appears to
me to be wrong, but if it's just something I misunderstand about XPConnect or
JS, then I won't be real surprised.

 The test simply gets a filespec (for a small file), and then calls read to
slurp the whole file, passing a JS object in as a read buffer.

 For a read on the filespec: "n = fileSpec.read(buf, len)", in about 50% of my
tests, ( buf.length > n ) by anyhere from 1 to 13 bytes, and is also greater
than the actual filesize (garbage is appended to the buffer).

 Here's the test (and I'll attach it also). [I was using WinNT 1999121008].

  -----
<html><head><script>
function debug(msg) {
    // Uncomment to print out debug info.
    dump(msg);
}

function readFileSpec() {
  var fs;
  fs = Components.classes["component://netscape/filespec"].createInstance();
  fs = fs.QueryInterface(Components.interfaces.nsIFileSpec);
  fs.unixStyleFilePath = "test.txt";
  fs.openStreamForReading();

  var tmp   = { value:null };  // out buf for fs.read
  var sz    = 0;               // length read by fs.read
  var ex;

  debug('\n\n--------OUTPUT-------------------------------------------\n');
  debug('fs.URLString : ' + fs.URLString +'\n');
  debug('fs.fileSize  : ' + fs.fileSize  +'\n');
  try {
      sz = fs.read(tmp, fs.fileSize);
      debug("fs -- tmp.value.length = " + tmp.value.length + '\n');
      debug("fs -- fs.read returned = " + sz + '\n');
      if ( tmp.value.length != sz )
	  debug("fs -- *** DIFFERENT SIZES *** " +
		tmp.value.length + " != " + sz + '\n');
      debug("fs -- tmp.value follows below.\n---------------------------\n" +
	    tmp.value + "\n-----------------------------\n");
  } catch(ex) {
      debug("fs -- Failed fs.read : " + ex + "\n");
      return;
  }
}

readFileSpec();

</script></head>
<body>
<p>hello, have a look at the console output</p>
</body></html>
  -----


Here's a sample of the output when things are wrong (note: the
string '/mozbin/moz/±' is not in the original file):

--------OUTPUT-------------------------------------------
fs.URLString : file:///C|/JohnStuff/mozbin/moz/bin/test.txt
fs.fileSize  : 220
fs -- tmp.value.length = 233
fs -- fs.read returned = 220
fs -- *** DIFFERENT SIZES *** 233 != 220
fs -- tmp.value follows below.
---------------------------
abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz
abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz
abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz
abcdefghijklmnopqrstuvwxyz abcdefghijklmnopqrstuvwxyz
/mozbin/moz/±
-----------------------------
Document file:///c|/temp/xpc-bug.html loaded successfully
Document: Done (0.881 secs)
Assignee: jband → dougt
Component: XPConnect → XPCOM
The problem is that the nsIFileSpec.idl doesn't describe the interace well
enough. This is true for both read and readLine...
 long read(inout string buffer, in long requestedCount);
 void readLine(inout string line, in long bufferSize, out boolean wasTruncated);

The signatures need to be changed to use [size_is()] so that xpconnect can
gather the information it needs to know how large the strings are when doing
conversions. As it is now since the buffers are described as 'string's xpconnect
must use strlen. However, 'read' could be binary data and is not zero
terminated. This could do bad things on some platforms as strlen scans out of
range.

The signatures of these methods have a few problems.
- the size params needs to be PRUint32 (i.e. unsigned) for size_is
- since the buffers are 'inout' to allow for buffer reuse the size_is param is
used in both directions. So having 'read' return the count of bytes read is
somewhat broken. It might be better to reuse the same param for requestedCount
and readCount...

 void read([size_is(count)] inout string buffer,
           inout PRUint32 count);

This will require JS callers to do something like:

 var count = {value:0};
 var buffer = {};
 foo.read(buffer, count);
 var byteCount = count.value;

readLine (and read too) could be converted to use size_is and count_is to allow
for better buffer reuse.

FWIW, the test function ought to end with "fs.closeStream();"
Target Milestone: M14
nsIFileSpec.idl is going to be replaced with nsIFile.idl.   Setting milestone
Depends on: 22047
will be fixed by 22047
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → WONTFIX
nsIFileSpec should not be used any longer.  nsIFile and nsILocalFile should be
used instead.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: