Uploaded image for project: 'ShrinkWrap'
  1. ShrinkWrap
  2. SHRINKWRAP-453

Paths in webarchives are not calculated correctly

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: 1.1.2
    • Fix Version/s: 1.2.3
    • Component/s: api
    • Labels:
      None
    • Environment:

      Win7x64, JDK1.7.0_17x32, JBOSS 7.1.3.Final, Arquillian 1.0.3Final, JUnit 4.11

      Description

      When creating a WebArchive via ShrinkWrap from a directory that is an exploded war file, paths inside this archive (generated with "arquillian.xml/arquillian/engine/property[deploymentExportPath]->target/deployments") generated invalid in some cases.
      When investigating the sources I found "org.jboss.shrinkwrap.impl.base.importer.ExplodedImporterImpl.calculatePath(File root, File child)" uses "String.replaceFirst" to replace the occurance of rootPath in childPath to create a local war file path. My path of the directory that needs compression contains brackets, which (as well as other path elements, e.g. ".") are interpreted as regular expression tokens and therefore don't match.
      The resulting archive can not be delpoyed in AS and tests can not be performed automatically.

      The code to create the WebArchive looks like this:
      ShrinkWrap.create(WebArchive.class, resultingWarFileNameAsString)
      .as(ExplodedImporter.class).importDirectory(absolutePathToExplodedWarDirectoryAsFile)
      .as(WebArchive.class);

        Gliffy Diagrams

          Activity

          Hide
          shoesel Stefan Hösel added a comment -

          The directory can be copied to another "safe" location before creating the WebArchive but that's not really a good solution since it needs to be removed afterwards, uses additional space etc.

          Show
          shoesel Stefan Hösel added a comment - The directory can be copied to another "safe" location before creating the WebArchive but that's not really a good solution since it needs to be removed afterwards, uses additional space etc.
          Hide
          alrubinger Andrew Rubinger added a comment -

          I'm not quite groking from the description what inputs you're providing. Would you mind sending along a PR showing the failing test case? Doesn't have to be perfect, just any failing test in the ShrinkWrap test suite will do and we'll fix it up to project spec. Thanks for the report!

          Show
          alrubinger Andrew Rubinger added a comment - I'm not quite groking from the description what inputs you're providing. Would you mind sending along a PR showing the failing test case? Doesn't have to be perfect, just any failing test in the ShrinkWrap test suite will do and we'll fix it up to project spec. Thanks for the report!
          Hide
          tommysdk Tommy Tynjä added a comment -

          First of all, please notice that the greater than character is not supported in paths on all platforms. The problem in ExplodedImporterImpl in the calculatePath(File, File) method is that the replaceFirst method takes a regular expression as its first parameter. If the root path contains regular expression characters, the replaceFirst method call will most likely not behave as intended, in this case when brackets are present in the path. This could even cause a java.util.regex.PatternSyntaxException if e.g. only an opening bracket is present in the path, with no matching closing bracket for the regular expression evaluation.

          I've created a reproducing failing test case as well as a test case causing a java.util.regex.PatternSyntaxException. The failing tests are accompanied by a test case which shows the intended behaviour. Commit: https://github.com/tommysdk/shrinkwrap/commit/e09da7845156d93ded6fab14091938a10d1f509a

          One solution would be to escape all regular expression characters so that the path never would be evaluated as an regular expression. Another solution is to not support such paths, clearly document this behaviour and throw an appropriate exception for such paths, such as an IllegalArgumentException.

          Show
          tommysdk Tommy Tynjä added a comment - First of all, please notice that the greater than character is not supported in paths on all platforms. The problem in ExplodedImporterImpl in the calculatePath(File, File) method is that the replaceFirst method takes a regular expression as its first parameter. If the root path contains regular expression characters, the replaceFirst method call will most likely not behave as intended, in this case when brackets are present in the path. This could even cause a java.util.regex.PatternSyntaxException if e.g. only an opening bracket is present in the path, with no matching closing bracket for the regular expression evaluation. I've created a reproducing failing test case as well as a test case causing a java.util.regex.PatternSyntaxException. The failing tests are accompanied by a test case which shows the intended behaviour. Commit: https://github.com/tommysdk/shrinkwrap/commit/e09da7845156d93ded6fab14091938a10d1f509a One solution would be to escape all regular expression characters so that the path never would be evaluated as an regular expression. Another solution is to not support such paths, clearly document this behaviour and throw an appropriate exception for such paths, such as an IllegalArgumentException.
          Hide
          shoesel Stefan Hösel added a comment -

          Sorry, it took a while, but here it is, Commit:
          https://github.com/shoesel/shrinkwrap/commit/368f80f4f4da5209f4c34c16250e596e709c43bd

          The second added test case works fine (just to verify and maybe clarify the problem).

          Maybe your could just truncate the childpath by the length of the rootpath..
          Hope that helps.

          Show
          shoesel Stefan Hösel added a comment - Sorry, it took a while, but here it is, Commit: https://github.com/shoesel/shrinkwrap/commit/368f80f4f4da5209f4c34c16250e596e709c43bd The second added test case works fine (just to verify and maybe clarify the problem). Maybe your could just truncate the childpath by the length of the rootpath.. Hope that helps.
          Hide
          pawandubey Pawan Dubey added a comment -

          Karel Piwko Can we just not use Pattern.quote() here inside String.replaceFirst() to send the literal interpretation of the string instead of translating it as regex?

          Show
          pawandubey Pawan Dubey added a comment - Karel Piwko Can we just not use Pattern.quote() here inside String.replaceFirst() to send the literal interpretation of the string instead of translating it as regex?
          Show
          michael.vorburger Michael Vorburger added a comment - https://github.com/shrinkwrap/shrinkwrap/pull/86 ?
          Hide
          pawandubey Pawan Dubey added a comment -

          Michael Vorburger Thanks for posting it here. Andrew Rubinger knows about this though. I have to submit a patch for this.

          Show
          pawandubey Pawan Dubey added a comment - Michael Vorburger Thanks for posting it here. Andrew Rubinger knows about this though. I have to submit a patch for this.
          Hide
          alrubinger Andrew Rubinger added a comment -

          Upstream: b5253fd1242e908de922a02f3ff0ecf4676c7ce6

          Thanks, Pawan Dubey!

          Show
          alrubinger Andrew Rubinger added a comment - Upstream: b5253fd1242e908de922a02f3ff0ecf4676c7ce6 Thanks, Pawan Dubey !

            People

            • Assignee:
              Unassigned
              Reporter:
              shoesel Stefan Hösel
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development