Archiva
  1. Archiva
  2. MRM-789

Archiva may delete you app server installation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.0.2
    • Fix Version/s: 1.1
    • Component/s: repository interface
    • Labels:
      None
    • Environment:
      linux, jdk 1.6, tomcat 6

      Description

      I installed the WAR version of Archiva into my tomcat instance... no problem so far.
      I then attempted to delete the default "internal" repository. I hit the delete config and contents button.

      At that moment I noticed that the repository directory was the tomcat home directory.

      Archiva managed to completely delete my Tomcat installation.

      To reproduce this, install it as a war, point a repo dir at your app server home, and hit the delete button (make sure you have a backup).

        Issue Links

          Activity

          Hide
          Brett Porter added a comment -

          is the problem here that it deletes it, or that it used the tomcat home as the location for the internal repository?

          Show
          Brett Porter added a comment - is the problem here that it deletes it, or that it used the tomcat home as the location for the internal repository?
          Hide
          Brill Pappin added a comment -

          The delete worked flawlessly... and that was and was not the problem because it was pointed at tomcat home by default (or at least it was without me setting it).

          A quick fix would be to make sure you append you own dir name on the end of whatever path chosen, that way a delete won't wipe out a complex app server installation.

          I was very luck in that this particular host was entirely in SVN, so I was able to get the whole thing going again in a minute or so... but anyone else that tries this is not going to be so lucky.

          Show
          Brill Pappin added a comment - The delete worked flawlessly... and that was and was not the problem because it was pointed at tomcat home by default (or at least it was without me setting it). A quick fix would be to make sure you append you own dir name on the end of whatever path chosen, that way a delete won't wipe out a complex app server installation. I was very luck in that this particular host was entirely in SVN, so I was able to get the whole thing going again in a minute or so... but anyone else that tries this is not going to be so lucky.
          Hide
          Maria Odea Ching added a comment - - edited

          I'm not sure if this would work, but maybe we could put a check in the webapp before saving a repo config--if the location specified is equal to the value of 'appserver.base' or 'appserver.home', then it shouldn't be allowed? Would this address the problem?

          Show
          Maria Odea Ching added a comment - - edited I'm not sure if this would work, but maybe we could put a check in the webapp before saving a repo config--if the location specified is equal to the value of 'appserver.base' or 'appserver.home', then it shouldn't be allowed? Would this address the problem?
          Hide
          Brill Pappin added a comment -

          that might work, but I think that app server specific isn't it?

          Show
          Brill Pappin added a comment - that might work, but I think that app server specific isn't it?
          Hide
          Maria Odea Ching added a comment -

          yeah.. and this can only be checked if the appserver.base or appserver.home variable is set.

          Show
          Maria Odea Ching added a comment - yeah.. and this can only be checked if the appserver.base or appserver.home variable is set.
          Hide
          Maria Odea Ching added a comment -

          Would checking if the repo location is equivalent to any value in the system properties would do here?

          For example, Tomcat sets the property catalina.home for it's base directory. We could get the value of it via the system properties. So if the repo location is equal to a value of any of the system properties, then the delete won't be allowed. Wdyt?

          Show
          Maria Odea Ching added a comment - Would checking if the repo location is equivalent to any value in the system properties would do here? For example, Tomcat sets the property catalina.home for it's base directory. We could get the value of it via the system properties. So if the repo location is equal to a value of any of the system properties, then the delete won't be allowed. Wdyt?
          Hide
          Maria Odea Ching added a comment -

          I committed the above fix in trunk -r662662:

          • check if the repo location is referenced in the system properties before deleting the repo in the file system.

          Please verify if the fix is sufficient enough to address your problem.. Thanks!

          Show
          Maria Odea Ching added a comment - I committed the above fix in trunk -r662662: check if the repo location is referenced in the system properties before deleting the repo in the file system. Please verify if the fix is sufficient enough to address your problem.. Thanks!
          Hide
          Brill Pappin added a comment -

          It looks like a good solution to this specific issue, but still wouldn't stop it from happening.
          Maybe the check should be that when setting a repository path, it can't mat h any of the system properties.

          the only way can think of making sure, is to create some sort of "work" directory under whatever directory the repo is set to.

          Show
          Brill Pappin added a comment - It looks like a good solution to this specific issue, but still wouldn't stop it from happening. Maybe the check should be that when setting a repository path, it can't mat h any of the system properties. the only way can think of making sure, is to create some sort of "work" directory under whatever directory the repo is set to.
          Hide
          Maria Odea Ching added a comment -

          Yeah, creating a "work" directory would ensure this but I'm hesitant in having a "work" directory. Doing this would require importing the contents of the repository into the "work" directory. This would mean that there'll be duplicate copies of the repo in the filesystem.. do we really want that? I'm also taking into account how long it would take to add a new managed repo if the contents would have to be imported.. it might take a while if it's a large repo. Maybe we should bring this up to the dev list..

          Anyway, I'll move the checking of the system properties to when a managed repo is created instead of during delete.
          Thanks for your suggestions Brill!

          Show
          Maria Odea Ching added a comment - Yeah, creating a "work" directory would ensure this but I'm hesitant in having a "work" directory. Doing this would require importing the contents of the repository into the "work" directory. This would mean that there'll be duplicate copies of the repo in the filesystem.. do we really want that? I'm also taking into account how long it would take to add a new managed repo if the contents would have to be imported.. it might take a while if it's a large repo. Maybe we should bring this up to the dev list.. Anyway, I'll move the checking of the system properties to when a managed repo is created instead of during delete. Thanks for your suggestions Brill!
          Hide
          Brill Pappin added a comment -

          I agree duplication would be bad, but what I mean was that if the repo definition root is mydir/ then put the actually repo under /mydir/repository
          you could use the same root for things like configuration as well, which would make the repository very portable.

          Anyway, as long as you folks are aware of the issue, I'm sure something workable will come out of it

          Show
          Brill Pappin added a comment - I agree duplication would be bad, but what I mean was that if the repo definition root is mydir/ then put the actually repo under /mydir/repository you could use the same root for things like configuration as well, which would make the repository very portable. Anyway, as long as you folks are aware of the issue, I'm sure something workable will come out of it
          Hide
          Maria Odea Ching added a comment -

          Ok, point taken I'll open a separate issue for this and move it to 1.2 for review. I don't think we'll have enough time to make this change for 1.1 given the issues list that we still have to work on for 1.1.

          Thanks again Brill!

          Show
          Maria Odea Ching added a comment - Ok, point taken I'll open a separate issue for this and move it to 1.2 for review. I don't think we'll have enough time to make this change for 1.1 given the issues list that we still have to work on for 1.1. Thanks again Brill!
          Hide
          Brett Porter added a comment -

          checking against the system properties is dangerous - for example, the recommended way to run is with -Dappserver.base, and typically the data is a sudirectory of that value.

          I would be content if the default was ensured of not pointing at an important place. For values the user enters on their own for new/editing repositories, I would ask for confirmation before accepting one that already exists.

          Show
          Brett Porter added a comment - checking against the system properties is dangerous - for example, the recommended way to run is with -Dappserver.base, and typically the data is a sudirectory of that value. I would be content if the default was ensured of not pointing at an important place. For values the user enters on their own for new/editing repositories, I would ask for confirmation before accepting one that already exists.
          Hide
          Maria Odea Ching added a comment -

          Ok, I'll remove the checks against the system properties. I guess this (the default was ensured of not pointing at an important place) is already covered because the default config has the 'data' subdirectory. It's this (for values the user enters on their own for new/editing repositories, I would ask for confirmation before accepting one that already exists) that still needs to be addressed.

          Thanks Brett!

          Show
          Maria Odea Ching added a comment - Ok, I'll remove the checks against the system properties. I guess this (the default was ensured of not pointing at an important place) is already covered because the default config has the 'data' subdirectory. It's this (for values the user enters on their own for new/editing repositories, I would ask for confirmation before accepting one that already exists) that still needs to be addressed. Thanks Brett!
          Hide
          Maria Odea Ching added a comment -

          Added confirmation page before saving a repo config (whether new or update) if the repo location already exists in trunk -r663788.

          This is the new flow:
          1. Add

          • Add a managed repo.
          • If the repo location set already exists, then user will be directed to the confirmation page.
          • If user clicks Save, then repo will be added. Otherwise, user will be redirected back to the repositories page.
            2. Edit
          • Edit a managed repo config.
          • If the repo location has been changed, then the new location would be checked if it already exists. If it does, then user will be directed to the confirmation page.
          • If user clicks Save, then the repo config will be updated. Otherwise, user will be redirected back to the repositories page.

          Please verify if the fix is sufficient for this issue. Thanks..

          Show
          Maria Odea Ching added a comment - Added confirmation page before saving a repo config (whether new or update) if the repo location already exists in trunk -r663788. This is the new flow: 1. Add Add a managed repo. If the repo location set already exists, then user will be directed to the confirmation page. If user clicks Save, then repo will be added. Otherwise, user will be redirected back to the repositories page. 2. Edit Edit a managed repo config. If the repo location has been changed, then the new location would be checked if it already exists. If it does, then user will be directed to the confirmation page. If user clicks Save, then the repo config will be updated. Otherwise, user will be redirected back to the repositories page. Please verify if the fix is sufficient for this issue. Thanks..
          Hide
          Brill Pappin added a comment -

          Thanks for the work Maria... but the issue was actually that the repository was somehow set to my tomcat root and when i deleted it, it deleted the entire tomcat deployment

          The set to root thing was a default install of the war version (so it would not have made any difference adding a new repo).

          So, your change will help, but I think more importantly make sure on install that the directory doesn't exist.

          Show
          Brill Pappin added a comment - Thanks for the work Maria... but the issue was actually that the repository was somehow set to my tomcat root and when i deleted it, it deleted the entire tomcat deployment The set to root thing was a default install of the war version (so it would not have made any difference adding a new repo). So, your change will help, but I think more importantly make sure on install that the directory doesn't exist.
          Hide
          Maria Odea Ching added a comment -

          I tried replicating the problem you encountered, but the location of the internal & snapshots repositories did not default to the Tomcat installation for me.. it was data/repository/internal & data/repository/snapshots. Anyway, I'll see if I can put some checks during startup regarding this. Thanks

          Show
          Maria Odea Ching added a comment - I tried replicating the problem you encountered, but the location of the internal & snapshots repositories did not default to the Tomcat installation for me.. it was data/repository/internal & data/repository/snapshots. Anyway, I'll see if I can put some checks during startup regarding this. Thanks
          Hide
          Maria Odea Ching added a comment -

          Fixed in trunk -r670114:

          • added an additional check in ArchivaConfiguration to append 'data/repositories/[repo_id]' in the repo location if the location already exists & does not end with 'data/repositories/[repo_id]' when loaded from the default config (from default-archiva.xml)
          • added test
          Show
          Maria Odea Ching added a comment - Fixed in trunk -r670114: added an additional check in ArchivaConfiguration to append 'data/repositories/ [repo_id] ' in the repo location if the location already exists & does not end with 'data/repositories/ [repo_id] ' when loaded from the default config (from default-archiva.xml) added test

            People

            • Assignee:
              Maria Odea Ching
              Reporter:
              Brill Pappin
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development