Bug 48674 - Tomcat Virtual Host Manager application doesn't persist newly created virtual hosts
Summary: Tomcat Virtual Host Manager application doesn't persist newly created virtual...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Manager application (show other bugs)
Version: 6.0.29
Hardware: All Linux
: P2 enhancement with 1 vote (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 48673 59027 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-02-03 10:48 UTC by Peter Skopek
Modified: 2016-02-22 20:23 UTC (History)
2 users (show)



Attachments
Rough patch to provide persistance to the host manager. (24.80 KB, patch)
2010-09-12 14:40 UTC, Wesley
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Skopek 2010-02-03 10:48:37 UTC
If one create virtual host using /host-manager application all work as
expected. Start/Stop/Remove actions.
But newly created host is not persistently added to the Tomcat configuration.
After restart of Tomcat there is only localhost host listed.
Comment 1 Konstantin Kolinko 2010-02-03 10:51:37 UTC
*** Bug 48673 has been marked as a duplicate of this bug. ***
Comment 2 Mark Thomas 2010-02-06 13:44:21 UTC
Patches to enhance the host-manager are always welcome.
Comment 3 Wesley 2010-09-12 14:40:28 UTC
Created attachment 26019 [details]
Rough patch to provide persistance to the host manager.

This needs some cleanup but it is functional.

At tbe meoment errors are being swallowed. I'm not sure what to do with the though.

Should the manager show success if the runtime addition was successful but the persistent addition wasn't?
Comment 4 Wesley 2010-09-24 16:03:43 UTC
Is this worth continuing with?  I know its rough but I'd like to know if its a start again job, or continue from here.

Wes
Comment 5 Chuck Caldarale 2010-09-24 16:16:06 UTC
(In reply to comment #4)
> Is this worth continuing with?  I know its rough but I'd like to know if its a
> start again job, or continue from here.

I definitely think it's worthwhile.

As far as the error handling goes, I'm in agreement with another e-mail that as much as possible, it should appear to be an atomic operation.  It might not always be possible to undo an inserted <Host> from the live Tomcat, but you can at least tell the user what the situation is.

 - Chuck
Comment 6 Mark Thomas 2016-02-18 15:12:19 UTC
*** Bug 59027 has been marked as a duplicate of this bug. ***
Comment 7 Mark Thomas 2016-02-18 15:15:17 UTC
Patch based on StoreConfig attached to Bug 59027.
Comment 8 Remy Maucherat 2016-02-18 15:35:46 UTC
I had a request for this item, and since we already have a component responsible for persisting the configuration, I figured it should be used rather than add a lot of specific code which would have similar issues.

The location of the store config button is open for discussion though, but normally the regular manager doesn't need it.
Comment 9 Christopher Schultz 2016-02-18 18:30:57 UTC
This can be a dangerous feature, for a couple of reasons.

1. A bad configuration or vulnerability in the host-manager allows a remote party to write to the filesystem, rather than just trash an in-memory configuration
2. Important information in the file may be overwritten inadvertently
3. NOP configuration information in the file (e.g. comments) will likely be lost when the file is saved

I had a look at the StoreConfig-based patch, and I must admit that I got lost in the whole architecture at the point that I started reading code in the o.a.c.storeconfig package. There is very little javadoc explaining what the heck is going on. It looks quite over-engineered and has a lot of code that looks very similar across classes.
Comment 10 Mark Thomas 2016-02-18 19:24:15 UTC
(In reply to Christopher Schultz from comment #9)
> This can be a dangerous feature, for a couple of reasons.
> 
> 1. A bad configuration or vulnerability in the host-manager allows a remote
> party to write to the filesystem, rather than just trash an in-memory
> configuration

The remote user can almost certainly deploy applications so it is pretty much game over anyway.

> 2. Important information in the file may be overwritten inadvertently

I think Store config saves the old version with a timestamp.

> 3. NOP configuration information in the file (e.g. comments) will likely be
> lost when the file is saved

Price you pay...

> I had a look at the StoreConfig-based patch, and I must admit that I got
> lost in the whole architecture at the point that I started reading code in
> the o.a.c.storeconfig package. There is very little javadoc explaining what
> the heck is going on. It looks quite over-engineered and has a lot of code
> that looks very similar across classes.

Saving configuration is extremely tricky. There might be some clean-up possible but my recollection from the last time if looked at the code was that it was fundamentally sound.

Overall, I think this is the way to go.
Comment 11 Remy Maucherat 2016-02-18 22:02:19 UTC
(In reply to Christopher Schultz from comment #9)
> This can be a dangerous feature, for a couple of reasons.

The user has to add the storeconfig listener in his configuration to use this patch. Then all this does is a JMX method call, which is most likely already accessible elsewhere.

> I had a look at the StoreConfig-based patch, and I must admit that I got
> lost in the whole architecture at the point that I started reading code in
> the o.a.c.storeconfig package. There is very little javadoc explaining what
> the heck is going on. It looks quite over-engineered and has a lot of code
> that looks very similar across classes.

Maybe, but the Catalina structure is built using reflection with a lot of special cases. It's a separate optional module, so feel free to rewrite it :)
Comment 12 Mark Thomas 2016-02-22 20:23:23 UTC
I've applied a variation of Coty Sutherland's patch to trunk for 9.0.0.M4 onwards. We can tweak that as feedback is received.