Velocity
  1. Velocity
  2. VELOCITY-144

Allow absolute filename with FileResourceLoader

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.3-rc1
    • Fix Version/s: 1.5
    • Component/s: Engine
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: PC

      Description

      There is no way of providing an absolute path (e.g. c:\temp\wibble.tmp) to the
      FileResourceLoader because it always attempts to use the 2-argument File
      constructor (even if the path component is empty).

      The following fix resolves this problem:
      In FileResourceLoader.findTemplate replace:
      File file = new File( path, template );
      with
      File file = null;

      if("".equals(path))
      file = new File( template );
      else
      file = new File ( path, template );

      Note this does not introduce any security risks as the FileResourceLoader must
      be configured to search the empty ("") path.

        Activity

        Dale Peakall created issue -
        Hide
        Will Glass-Husain added a comment -

        This seems a useful feature enhancement. Especially for those building stand-
        alone applications it would be nice to allow absolute paths when desired
        (indicated by including an empty string for the template path). As the
        original poster notes, this remains secure for normal use where the template
        path is specified.

        I've included a patch and a test case. (ant test works fine, of course). I'd
        be happy to submit a patch for the docs when this is committed.

        Show
        Will Glass-Husain added a comment - This seems a useful feature enhancement. Especially for those building stand- alone applications it would be nice to allow absolute paths when desired (indicated by including an empty string for the template path). As the original poster notes, this remains secure for normal use where the template path is specified. I've included a patch and a test case. (ant test works fine, of course). I'd be happy to submit a patch for the docs when this is committed.
        Hide
        Will Glass-Husain added a comment -

        Created an attachment (id=9006)
        FileResourceLoader.java patch

        Show
        Will Glass-Husain added a comment - Created an attachment (id=9006) FileResourceLoader.java patch
        Hide
        Will Glass-Husain added a comment -

        Created an attachment (id=9007)
        AbsoluteFileResourceLoaderTest.java

        Show
        Will Glass-Husain added a comment - Created an attachment (id=9007) AbsoluteFileResourceLoaderTest.java
        Hide
        Will Glass-Husain added a comment -

        Created an attachment (id=9008)
        absolute.zip (test cases)

        Show
        Will Glass-Husain added a comment - Created an attachment (id=9008) absolute.zip (test cases)
        Hide
        Will Glass-Husain added a comment -

        Patched. Revision # 124382. Thanks for the suggestion.

        Show
        Will Glass-Husain added a comment - Patched. Revision # 124382. Thanks for the suggestion.
        Hide
        Will Glass-Husain added a comment -

        security concerns need to be discussed on dev list

        Show
        Will Glass-Husain added a comment - security concerns need to be discussed on dev list
        Jeff Turner made changes -
        Field Original Value New Value
        issue.field.bugzillaimportkey 17379 12315014
        Hide
        Will Glass-Husain added a comment -

        Geir expressed concern over security issues with this patch. I posted this on the dev list... no push back from other developers. So I think we should re-apply this.

        <quote>
        Here's why I'm not worried:

        (1) Outside users do not directly provide a template name. In a typical back-end use this is programmed by the developer. In a web use this comes from the URL (which can be filtered before sending to Velocity).

        (2) If a developer does not want to allow absolute file names, he/she just needs to configure a template path. (Note that this patch only applies for cases where the template path is not set).

        (3) This doesn't affect any existing code, because all existing uses of FileResourceLoader set a template path.
        </quote>

        Show
        Will Glass-Husain added a comment - Geir expressed concern over security issues with this patch. I posted this on the dev list... no push back from other developers. So I think we should re-apply this. <quote> Here's why I'm not worried: (1) Outside users do not directly provide a template name. In a typical back-end use this is programmed by the developer. In a web use this comes from the URL (which can be filtered before sending to Velocity). (2) If a developer does not want to allow absolute file names, he/she just needs to configure a template path. (Note that this patch only applies for cases where the template path is not set). (3) This doesn't affect any existing code, because all existing uses of FileResourceLoader set a template path. </quote>
        Hide
        Will Glass-Husain added a comment -

        no one argued against this. patch applied.

        Show
        Will Glass-Husain added a comment - no one argued against this. patch applied.
        Will Glass-Husain made changes -
        Assignee Velocity-Dev List [ velocity-dev@jakarta.apache.org ]
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Henning Schmiedehausen added a comment -

        Close all resolved issues for Engine 1.5 release.

        Show
        Henning Schmiedehausen added a comment - Close all resolved issues for Engine 1.5 release.
        Henning Schmiedehausen made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Mark Thomas made changes -
        Workflow jira [ 12325019 ] Default workflow, editable Closed status [ 12551906 ]
        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12551906 ] jira [ 12552687 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Reopened Reopened Resolved Resolved
        953d 13h 40m 1 Will Glass-Husain 06/Oct/05 15:16
        Resolved Resolved Closed Closed
        517d 9h 48m 1 Henning Schmiedehausen 08/Mar/07 00:04

          People

          • Assignee:
            Unassigned
            Reporter:
            Dale Peakall
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development