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

        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
        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.
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development