ARTEMIS-4160 Handle hostnames with invalid char sequences. #4362
Conversation
|
Hi all. This is my first PR for this project, and I didn't find anything that talked about needing a Bugzilla bug before submitting. We ran into this issue. We have hosts with "--" in the name, and that ended up in the comment in the jolokia-access.xml file as a comment. That's an invalid char sequence for an xml comment. I've tried to handle other illegal char sequences too. I hope this PR is useful. |
davidlanouette
force-pushed
the
fix-xml-comment
branch
from
30be8ba
to
24e2629
Compare
|
@davidlanouette, thanks for the contribution! Most changes will, in fact, need an associated Jira (we don't use Bugzilla). See more details in our Hacking Guide. I went ahead and created one for you. |
davidlanouette
changed the title
|
@jbertram Do I need to get a jira login and update the bug before it gets reviewed? |
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/jolokia-access.xml
Outdated
Show resolved
Hide resolved
|
@gtully this is the same issue we have seen ArtemisCloud.io test suite |
That is where we found this originally. The pod name had "--" in the middle of the name. We ended up needing to add an init container with a simple script that modified the |
davidlanouette
force-pushed
the
fix-xml-comment
branch
from
3cd141f
to
da17acf
Compare
|
@davidlanouette your previous test would be good to avoid future regressions, could you add it back? |
Yes, I'll add it back, but with some tweaks to test the whole file. |
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
|
There are |
|
@brusdev Thanks for the feedback. I'll address the xml DocumentBuilder and the checkstyle violations. |
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
davidlanouette
force-pushed
the
fix-xml-comment
branch
from
164843b
to
bf29d4d
Compare
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Show resolved
Hide resolved
|
@brusdev thanks for the feedback. I've incorporated it with the latest commit. |
artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/CreateTest.java
Outdated
Show resolved
Hide resolved
|
@davidlanouette this PR LGTM, could you rebease your branch and squash your commits? |
Previously, the code added a comment with the host name in it.
Sometimes hostnames don't follow the xml standards for comments.
Specifically, having a double dash ("--") in the host name would cause
an error when jolokia tried to load the config file - because it was
invalid xml.
The simple solution was to not put the host name into the comment.
A unit test has been included to ensure the same thing doesn't happen in the
future.
Signed-off-by: David Lanouette <David.Lanouette@RedHat.com>
davidlanouette
force-pushed
the
fix-xml-comment
branch
from
4d3c3e0
to
82479d9
Compare
|
@davidlanouette thanks for your contribution. |
The hostname gets put into an xml comment in the jolokia-access.xml file. If the hostname has invalid char sequences for an xml comment, jolokia will fail at load time because the xml file is invalid.
Signed-off-by: David Lanouette David.Lanouette@RedHat.com