Forum OpenACS Development: Re: File Upload Fails on Windows OACS Installaion

Collapse
Posted by Gustaf Neumann on

What i can see from the output is that it looks like the Tcl file tempfile subcommand looks broken. The command below

::file tempfile tmpFileName $template

seems to be ignoring the privided template mostly. From the commands below,

set template "c:/naviserver/servers/openacs/tmp/nsd-XXXXXX"
set file [::file tempfile tmpFileName $template]

the expected tmpFileName is wrong

expected: c:/naviserver/servers/openacs/tmp/nsd-29771
you got:  c:/WINDOWS/TEMP/nsd-XXXXXX29771.TMP

Please double-check this. Of course, the numeric part will differ. If my hypothesis is correct, then please replace the function ns_opentmpfile in ns/tcl/form.tcl with the following

proc ns_opentmpfile {varFilename {template ""}} {
  upvar $varFilename tmpFileName
  if {$template eq ""} {
     set template [ns_config ns/parameters tmpdir]/nsd-XXXXXX
  }
  if {$::tcl_platform(platform) eq "windows"} {
      set tmpFileName [ns_mktemp -nocomplain $template]
      return [open $tmpFileName {RDWR CREAT EXCL}]
  } else {
     return [::file tempfile tmpFileName {*}$template]
  }
}

This is not the perfect solution and violates security considerations (creating the time file should be an atomic operation, we do it here like in old Tcl implementations), but it is as good as it gets with reasonable effort.

If my suspicion is true, I would regard this as a bug in Tcl, although the documentation in Tcl is very vague on this point. It says:

though some platforms may ignore some or all of these parts and use a built-in default instead.”

What version of Tcl are you using?

All the best
-g

Collapse
Posted by Raul Rodriguez on
Hi,
We applied the change to
C:\naviserver\tcl\form.tcl

and got this:

wrong # args: should be "ns_mktemp ?template?"
    while executing
"ns_mktemp -nocomplain $template"
    (procedure "ns_opentmpfile" line 7)
    invoked from within
"ns_opentmpfile tmpfile"
    (procedure "ns_getform" line 53)
    invoked from within
"ns_getform"
    (procedure "::nsf::procs::ad_page_contract" line 315)
    invoked from within
"ad_page_contract {
    page to add a new file to the system

    @author Kevin Scaldeferri (mailto:kevin@arsdigita.com)
    @creation-date 6 Nov 2000
    @cv..."
    ("uplevel" body line 2)
    invoked from within
"uplevel {
    ad_page_contract {
    page to add a new file to the system

We removed:

-nocomplain

The updated procedure is:

proc ns_opentmpfile {varFilename {template ""}} {
  upvar $varFilename tmpFileName
  if {$template eq ""} {
     set template [ns_config ns/parameters tmpdir]/nsd-XXXXXX
  }
  if {$::tcl_platform(platform) eq "windows"} {
      set tmpFileName [ns_mktemp $template]
      return [open $tmpFileName {RDWR CREAT EXCL}]
  } else {
     return [::file tempfile tmpFileName {*}$template]
  }
}

We uploaded a file and got this:

[04/Nov/2025:09:15:25][3300.4b04][-conn:openacs:default:0:16-] Notice: >>> STARTING TMPFILE tmpfile: c:/naviserver/servers/openacs/tmp/nsd-a19204
[04/Nov/2025:09:15:25][3300.4b04][-conn:openacs:default:0:16-] Notice: >>> UPLOADCHECK tmpfile = c:/naviserver/servers/openacs/tmp/nsd-a19204
[04/Nov/2025:09:15:25][3300.4b04][-conn:openacs:default:0:16-] Notice: >>> UPLOADCHECK tmpdir = c:/naviserver/servers/openacs/tmp
[04/Nov/2025:09:15:25][3300.4b04][-conn:openacs:default:0:16-] Notice: >>> UPLOADCHECK file owned False
[04/Nov/2025:09:15:25][3300.4b04][-conn:openacs:default:0:16-] Warning: They tried to sneak in invalid tmpfile 'c:/naviserver/servers/openacs/tmp/nsd-a19204'
:        called from ad_page_contract {} 1 {} 0 __unknown__ {\n    page to add a new file to the syste...} {\n    file_id:naturalnum,optional,notnull...} -properties {\n    folder_id:onevalue\n    context:one...} -validate \\n\ \ \ \ file_id_or_folder_id\ \{\\n\ \ \ \ \ \ \ \ if\ ...
:        called from template::adp_parse c:/naviserver/servers/openacs//packages/file-storage/www/file-add {}
:        called from adp_parse_ad_conn_file
:        called from rp_serve_concrete_file c:/naviserver/servers/openacs/packages/file-storage/www/file-add.adp
:        called from rp_serve_abstract_file 0 0 .* c:/naviserver/servers/openacs/packages/file-storage/www/file-add
:        called from rp_handle_request
:        called from rp_handler
:            POST http://localhost:8000/file-storage/file-add? referred by 'http://localhost:8000/file-storage/file-add?folder_id=979'; peer ::1 user_id 729
Collapse
Posted by Raul Rodriguez on
We reverted this logging logic after our test:
  ns_log Notice ">>> STARTING TMPFILE tmpfile: $tmpfile"
    set tmpfile [ns_normalizepath $tmpfile]
    set tmpdir [string trimright [ns_config ns/parameters tmpdir] /]
  ns_log Notice ">>> UPLOADCHECK tmpfile = $tmpfile"
  ns_log Notice ">>> UPLOADCHECK tmpdir = $tmpdir"

    if {[ad_file dirname $tmpfile] ne $tmpdir} {
        #
        # File is not a direct child of the tmpfolder: not safe
        #
    ns_log Notice ">>> UPLOADCHECK direct child False"
        #return false
    }

    if {![ad_file exists $tmpfile]} {
        #
        # File does not exist yet: safe, unless we demand for the file
        # to exist.
        #
    ns_log Notice ">>> UPLOADCHECK file not exists"
        #return [expr {!$must_exist_p}]
    }

    if {![ad_file owned $tmpfile]} {
        #
        # File does not belong to us: not safe
        #
    ns_log Notice ">>> UPLOADCHECK file owned False"
        #return false
    }

    if {![ad_file readable $tmpfile]} {
        #
        # We cannot read the file: not safe
        #
    ns_log Notice ">>> UPLOADCHECK file can't read"
        #return false
    }

    if {![ad_file writable $tmpfile]} {
        #
        # We cannot write the file: not safe
        #
    ns_log Notice ">>> UPLOADCHECK file can't write"
        #return false
    }

    #
    # The file is safe
    #
    return true