Forum OpenACS Development: ns_cpfp broken in AOLserver 3.5.1

Collapse
Posted by Tom Jackson on

I know this is an AOLserver issue, but it may impact some OpenACS users, and it may be a good opportunity to clean up some code.

ns_cpfp appears to be broken, at least for copying JPEG image files. It seems to work on ACSII text. There are a few places in OpenACS that use ns_cpfp. The one I discovered was photo-album, but file-storage and the content-repository also use it.

The first question should be why is this being used at all. Its main purpose is to copy a partial file. An entire file copy is easily handled by ns_cp or now file copy without the need to open and then close the file and play with file descriptors.

To see the effect of the copy in all cases, check out my test case. You can upload your file, or see others that are already there.

My vote is to fix the three places that use ns_cpfp and associated open and close code with file copy.

Collapse
Posted by Bart Teeuwisse on
I came across the ns_cpfp issue in file-storage and it seems that ns_writefp appears to be broken too.

/Bart

Collapse
Posted by Tom Jackson on

Here is a sample of what can be done to replace total file copy using ns_cpfp with file copy.

Index: content-procs.tcl
===================================================================
RCS file: /cvsroot/openacs-4/packages/acs-content-repository/tcl/content-procs.tcl,v
retrieving revision 1.6
diff -r1.6 content-procs.tcl
76,83c76
<         # JCD: not sure why ns_cpfp is used.  tcl file copy is better 
<         # since it is smarter about buffer sizes.
<         set ifp [open $client_filename r]
<         set ofp [open [cr_fs_path]$content_file w]
<         
<         ns_cpfp $ifp $ofp
<         close $ifp
<         close $ofp
---

>         file copy $client_filename [cr_fs_path]$content_file

Bart, I'll add that one to my test case, thanks.

Collapse
Posted by Jamie Rasmussen on
I'd like to see this fixed as Tom suggests (It happens with AOLserver 4 too). Some of the complaints about broken file uploading on Win32 were actually a result of this error. I should have realized it was a cross-platform problem. I think you need a double-dash after file copy.

I've run into the ns_writefp problem before too, particularly in db_exec_lob. I'm not sure if there is a better solution than adding an fconfigure binary before the ns_writefp calls there.

Collapse
Posted by Tom Jackson on

Hmm, so the obvious answer ( I see the light ) is that if you use Tcl open, you need to use fconfigure to set the binary mode. The use of ns_cpfp in this instance would require a sequence of commands like:


set ifp [open infilename r]
set ofp [open outfilename w+]

fconfigure $ifp -translation binary
fconfigure $ofp -translation binary

ns_cpfp $ifp $ofp

close $ifp
close $ofp

Or just use either of the following:

# using OS open with O_BINARY flag set:
ns_cp infilename outfilename
# or Tcl: 
file copy infilename outfilename

I would still go with the Tcl only solution, but my question is to what branch should I create a patch? I don't know how to check out the head, any hints?

Also, I wonder why this works sometimes. The code for ns_cpfp hasn't changed in at least two years in this respect, it has probably always been this way.