Skip to content

Queue browse improvements in webconsole #1937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cshannon wants to merge 1 commit into from
Closed

Queue browse improvements in webconsole #1937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions activemq-karaf/src/main/resources/features-core.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
<bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-annotation_1.3_spec/1.0</bundle>
<bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-jta_1.1_spec/1.1.1</bundle>
<bundle>mvn:org.apache.commons/commons-pool2/${commons-pool2-version}</bundle>
<bundle>mvn:org.apache.commons/commons-lang3/${commons-lang3-version}</bundle>
<bundle>mvn:org.apache.commons/commons-text/${commons-text-version}</bundle>
Copy link
Copy Markdown
Contributor

@mattrpav mattrpav Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add these dependencies back in, they should be tied to the web console feature, and not the activemq-client

Copy link
Copy Markdown
Member

@jbonofre jbonofre Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because it goes in the activemq-osgi artifact, so it has to be in this feature.
This feature is core/common to all other features, including broker and webconsole.

So, adding these bundles in the webconsole feature won't work.

Copy link
Copy Markdown
Contributor

@mattrpav mattrpav Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The web console is deployed as an independent feature. Requiring these dependencies for activemq-client only users in Karaf (Camel, etc) is incorrect. Same for users using activemq-broker-noweb feature.

These dependencies should be added to the activemq-web-console feature here:

activemq/activemq-karaf/src/main/resources/features.xml

Line 49 in ca34bcd

<feature name="activemq-web-console" version="${project.version}">

Copy link
Copy Markdown
Member

@jbonofre jbonofre Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't get my point: today the activemq-osgi requires commons-text because the web-console import leak in activemq-osgi.
That's not in the webconsole war, it's in the activemq-osgi bundle.

Copy link
Copy Markdown
Contributor

@mattrpav mattrpav Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the web leak in activemq-osgi?

Copy link
Copy Markdown
Member

@jbonofre jbonofre Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/apache/activemq/blob/main/activemq-osgi/pom.xml#L150

Copy link
Copy Markdown
Member

@jbonofre jbonofre Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a different issue (unrelated to this PR).

<!-- uber osgi bundle means client is not that lean, todo: introduce client osgi bundle -->
<bundle>mvn:org.apache.activemq/activemq-osgi/${project.version}</bundle>
</feature>
Expand Down
5 changes: 5 additions & 0 deletions activemq-web/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@
<artifactId>websocket-jetty-server</artifactId>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
</dependency>

<!-- Rome RSS Reader -->
<dependency>
<groupId>com.rometools</groupId>
Expand Down
10 changes: 3 additions & 7 deletions activemq-web/src/main/java/org/apache/activemq/web/view/RssMessageRenderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class RssMessageRenderer extends SimpleMessageRenderer {
private String feedType = "rss_2.0";
private SyndFeed feed;
private String description = "This feed is auto-generated by Apache ActiveMQ";
private String entryContentType = "text/plain";
private static final String ENTRY_CONTENT_TYPE = "text/plain";

public void renderMessage(PrintWriter writer, HttpServletRequest request, HttpServletResponse response, QueueBrowser browser, Message message) throws JMSException {
SyndFeed feed = getFeed(browser, request);
Expand Down Expand Up @@ -79,11 +79,7 @@ public void setFeedType(String feedType) {
}

public String getEntryContentType() {
return entryContentType;
}

public void setEntryContentType(String entryContentType) {
this.entryContentType = entryContentType;
return ENTRY_CONTENT_TYPE;
}

// Implementation methods
Expand Down Expand Up @@ -122,7 +118,7 @@ protected SyndEntry createEntry(QueueBrowser browser, Message message, HttpServl

protected SyndContent createEntryContent(QueueBrowser browser, Message message, HttpServletRequest request) throws JMSException {
SyndContent description = new SyndContentImpl();
description.setType(entryContentType);
description.setType(getEntryContentType());

if (message instanceof TextMessage) {
String text = ((TextMessage)message).getText();
Expand Down
22 changes: 10 additions & 12 deletions activemq-web/src/main/java/org/apache/activemq/web/view/SimpleMessageRenderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.commons.text.StringEscapeUtils;

/**
* A simple rendering of the contents of a queue appear as a list of message
Expand All @@ -35,11 +36,12 @@
*/
public class SimpleMessageRenderer implements MessageRenderer {

private String contentType = "text/xml";
protected static final String DEFAULT_CONTENT_TYPE = "text/xml";

private int maxMessages;

public void renderMessages(HttpServletRequest request, HttpServletResponse response, QueueBrowser browser) throws IOException, JMSException, ServletException {
// lets use XML by default
// XML is used by default unless a child class overrides this method
response.setContentType(getContentType());
PrintWriter writer = response.getWriter();
printHeader(writer, browser, request);
Expand All @@ -53,10 +55,10 @@ public void renderMessages(HttpServletRequest request, HttpServletResponse respo
printFooter(writer, browser, request);
}

public void renderMessage(PrintWriter writer, HttpServletRequest request, HttpServletResponse response, QueueBrowser browser, Message message) throws JMSException, ServletException {
public void renderMessage(PrintWriter writer, HttpServletRequest request, HttpServletResponse response, QueueBrowser browser, Message message) throws JMSException {
// lets just write the message IDs for now
writer.print("<message id='");
writer.print(message.getJMSMessageID());
writer.print(StringEscapeUtils.escapeXml11(message.getJMSMessageID()));
writer.println("'/>");
}

Expand All @@ -71,25 +73,21 @@ public void setMaxMessages(int maxMessages) {
}

public String getContentType() {
return contentType;
}

public void setContentType(String contentType) {
this.contentType = contentType;
return DEFAULT_CONTENT_TYPE;
}

// Implementation methods
// -------------------------------------------------------------------------

protected void printHeader(PrintWriter writer, QueueBrowser browser, HttpServletRequest request) throws IOException, JMSException, ServletException {
protected void printHeader(PrintWriter writer, QueueBrowser browser, HttpServletRequest request) throws IOException, JMSException {
writer.println("");
writer.print("<messages queue='");
writer.print(browser.getQueue());
writer.print(StringEscapeUtils.escapeXml11(String.valueOf(browser.getQueue())));
writer.print("'");
String selector = browser.getMessageSelector();
if (selector != null) {
writer.print(" selector='");
writer.print(selector);
writer.print(StringEscapeUtils.escapeXml11(selector));
writer.print("'");
}
writer.println(">");
Expand Down
4 changes: 4 additions & 0 deletions assembly/src/main/descriptors/common-bin.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,10 @@
<include>com.rometools:rome-utils</include>
<include>org.jdom:jdom2</include>

<!-- String text utils -->
<include>org.apache.commons:commons-text</include>
<include>org.apache.commons:commons-lang3</include>

<!-- REST API -->
<include>org.jolokia:jolokia-core</include>
<include>org.jolokia:jolokia-server-core</include>
Expand Down
13 changes: 13 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
<commons-collections-version>3.2.2</commons-collections-version>
<commons-dbcp2-version>2.14.0</commons-dbcp2-version>
<commons-io-version>2.21.0</commons-io-version>
<commons-lang3-version>3.20.0</commons-lang3-version>
Copy link
Copy Markdown
Contributor

@mattrpav mattrpav Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really struggle with adding these dependencies back in for essentially one method. I think we'd end up being tied to their release cycle for security patches and ActiveMQ get flagged for CVE vulnerability, even if we don't use vulnerable methods.

Copy link
Copy Markdown
Member

@jbonofre jbonofre Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are back to what we had before 😄 Going in circle 😄

Copy link
Copy Markdown
Contributor Author

@cshannon cshannon Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any issue with adding dependencies on things like commons libraries and they are quite useful so I disagree with that. Limiting dependencies is good but I'm not going to worry about useful ones like commons-lang3 which I found hard to believe it didn't exist.

I mostly just want to get rid of the Spring dependencies but anything Apache Commons I am fine with.

Copy link
Copy Markdown
Contributor Author

@cshannon cshannon Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I will point out if we want to get rid of Spring we actually will want more of these types of libraries. We are currently using Spring utils in a lot of cases for things like HTML validation etc. We can switch to using Apache commons instead

Copy link
Copy Markdown
Contributor

@mattrpav mattrpav Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we'll always have custom buffer, io, and text handling classes due to all the protocol and admin functionality due to the nature of operating a performant broker. Part of the reason to re-home activemq-protobuf is to be able to consolidate those classes UTF8Buffer, ASCIIBuffer, etc.. into a single activemq-io module. We have a couple sets of these classes and custom stream handlers across the project, and the hawt* dependencies. I think once those classes are consolidated to a single module, it'll be more apparent.

I'm not opposed to adding this to the web console, b/c it is expedient and the desire to overhaul/replace this broker.

However, I think adoption of wider use of third-party I/O dependencies warrants a bigger discussion when it comes to client/broker usage.

<commons-text-version>1.15.0</commons-text-version>
<commons-logging-version>1.3.6</commons-logging-version>
<commons-pool2-version>2.13.1</commons-pool2-version>
<directory-version>2.0.0.AM25</directory-version>
Expand Down Expand Up @@ -901,6 +903,17 @@
<version>${commons-io-version}</version>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>${commons-lang3-version}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
<version>${commons-text-version}</version>
</dependency>

<!-- ACTIVEMQ-WEB Specific Dependencies -->
<dependency>
<groupId>com.rometools</groupId>
Expand Down