Issue1549

Java.io.InputStreams are binary streams, containing bytes. They are used, among other things, for delivering POST data to java servlets, through a ServletInputStream. As such, there should be no line-ending translation on such InputStreams, since they are byte oriented.

But when wrapping an InputStream with a PyFile, through the use of org.python.core.util.FileUtil.wrap(inputstream), line-ending translation takes place. See attached java code which illustrates the issue.

This may only be happening on Windows, where "\r\n" is the standard line ending.

This issue appears to affecting processing of servlet POST data, which by web standards is separated with "\r\n". Again, this may only be happening on Windows.

Perhaps the correct solution to this problem is to add a mode argument to the FileUtil.wrap() method, so that text or binary mode can be explicitly specified.

I agree with that. I'm cc-ing Phillipe since (AFAIR) he knows more than anyone else about file mode handling on Jython.

I'd just like to point out that it's linked to these tickets:
http://bugs.jython.org/issue1171#
http://bugs.jython.org/issue1204

They never resolved Alan's question: Why add the FileUtil class?
It just wraps a call to PyFile anyways. 
And why do I get the "TypeError: coercing to Unicode: need string" error from issue 1204 if I call the PyFile constructor directly?

The next three posts will be patches I created to fix the issue.

It says "You do not have permission to add a file" when I try to add a file, so I'm pasting all my svn diffs here:

Index: FileUtil.java
===================================================================
--- FileUtil.java	(revision 6976)
+++ FileUtil.java	(working copy)
@@ -18,6 +18,13 @@
 public class FileUtil {
 
     /**
+     * Creates a PyFile that reads from the given <code>InputStream</code> with mode.
+     */
+    public static PyFile wrap(InputStream is, String mode) {
+        return new PyFile(is, "<Java InputStream '" + is + "' as file>", mode, -1, true);    
+    }
+
+    /**
      * Creates a PyFile that reads from the given <code>InputStream</code> with bufsize.
      */
     public static PyFile wrap(InputStream is, int bufsize) {



Index: PyFile.java
===================================================================
--- PyFile.java	(revision 6976)
+++ PyFile.java	(working copy)
@@ -96,7 +96,7 @@
         file___init__(raw, name, mode, bufsize);
     }
 
-    PyFile(InputStream istream, String name, String mode, int bufsize, boolean closefd) {
+    public PyFile(InputStream istream, String name, String mode, int bufsize, boolean closefd) {
         parseMode(mode);
         file___init__(new StreamIO(istream, closefd), name, mode, bufsize);
     }



Index: modjy_wsgi.py
===================================================================
--- modjy_wsgi.py	(revision 6976)
+++ modjy_wsgi.py	(working copy)
@@ -125,7 +125,7 @@
 
     def set_wsgi_streams(self, req, resp, dict):
         try:
-            dict["wsgi.input"]  = create_py_file(req.getInputStream())
+            dict["wsgi.input"]  = create_py_file(req.getInputStream(),"rb")
             dict["wsgi.errors"] =  create_py_file(System.err)
         except IOException, iox:
             raise ModjyIOException(iox)

Alan: are you going to further review the patch or is it ready to commit? 

I think it is fine, but since you reported the bug (and well, you are the modjy guru) I'm deferring to you.

I think that Philip Jenvey is the right person to review the patch.

Although the bug showed up for someone using modjy, this is really an I/O subsystem issue, and Philip is the authority for that.

The intention of the patch is sound. My only correction is to add additional constructor methods to PyFile to accomodate this argspec along with the FileUtil wrappers. Even though PyFile has enough constructors, this is mostly to match how the other wrapper methods do it and then we don't have to duplicate the "<Java InputStream"> filename business