Bug 60883

Summary: Add ${__escapeXml()} function
Product: JMeter - Now in Github Reporter: Michael Osipov <michaelo>
Component: MainAssignee: JMeter issues mailing list <issues>
Status: RESOLVED FIXED    
Severity: enhancement CC: p.mouawad
Priority: P2    
Version: 3.1   
Target Milestone: ---   
Hardware: All   
OS: All   
Bug Depends on: 60886    
Bug Blocks:    

Description Michael Osipov 2017-03-17 15:39:30 UTC
Add a counterpart for __escapeHtml for XML too: __escapeXml

Simple impl based off __escapeHtml:

> import java.util.Collection;
> import java.util.LinkedList;
> import java.util.List;
> 
> import org.apache.commons.lang3.StringEscapeUtils;
> import org.apache.jmeter.engine.util.CompoundVariable;
> import org.apache.jmeter.functions.AbstractFunction;
> import org.apache.jmeter.functions.InvalidVariableException;
> import org.apache.jmeter.samplers.SampleResult;
> import org.apache.jmeter.samplers.Sampler;
> 
> public class EscapeXml extends AbstractFunction {
> 
> 	private static final List<String> desc = new LinkedList<String>();
> 
> 	private static final String KEY = "__escapeXml"; //$NON-NLS-1$
> 
> 	static {
> 		desc.add("Escape XML string"); //$NON-NLS-1$
> 	}
> 
> 	private Object[] values;
> 
> 	public EscapeXml() {
> 	}
> 
> 	/** {@inheritDoc} */
> 	@Override
> 	public String execute(SampleResult previousResult, Sampler currentSampler)
> 			throws InvalidVariableException {
> 
> 		String rawString = ((CompoundVariable) values[0]).execute();
> 		return StringEscapeUtils.escapeXml10(rawString);
> 
> 	}
> 
> 	/** {@inheritDoc} */
> 	@Override
> 	public void setParameters(Collection<CompoundVariable> parameters)
> 			throws InvalidVariableException {
> 		checkParameterCount(parameters, 1);
> 		values = parameters.toArray();
> 	}
> 
> 	/** {@inheritDoc} */
> 	@Override
> 	public String getReferenceKey() {
> 		return KEY;
> 	}
> 
> 	/** {@inheritDoc} */
> 	public List<String> getArgumentDesc() {
> 		return desc;
> 	}
> }
Comment 1 Philippe Mouawad 2017-03-17 15:42:19 UTC
Hello Michael,
Thanks for your proposal.
Would it be possible to provide a patch or a PR on our github :
- https://github.com/apache/jmeter

See:
- http://jmeter.apache.org/building.html

Thanks
Comment 2 Michael Osipov 2017-03-17 16:52:44 UTC
(In reply to Philippe Mouawad from comment #1)
> Hello Michael,
> Thanks for your proposal.
> Would it be possible to provide a patch or a PR on our github :
> - https://github.com/apache/jmeter
> 
> See:
> - http://jmeter.apache.org/building.html

The patch is the least problem. The problem is this:

https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065

Non-Maven build, it requires direct Internet connection. I have to see how I can change this to use either our HTTP proxy or Nexus instance otherwise I won't be able to build anything at work. "Setproxy Task" should do it. You should consider taking this from tomcat https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713

...repo forked...PR pending...
Comment 3 Philippe Mouawad 2017-03-17 17:35:23 UTC
(In reply to Michael Osipov from comment #2)
> (In reply to Philippe Mouawad from comment #1)
> > Hello Michael,
> > Thanks for your proposal.
> > Would it be possible to provide a patch or a PR on our github :
> > - https://github.com/apache/jmeter
> > 
> > See:
> > - http://jmeter.apache.org/building.html
> 
> The patch is the least problem. The problem is this:
> 
> https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065
> 
> Non-Maven build, it requires direct Internet connection. I have to see how I
> can change this to use either our HTTP proxy or Nexus instance otherwise I
> won't be able to build anything at work. "Setproxy Task" should do it. You
> should consider taking this from tomcat
> https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713

Good idea
Will you also contribute a patch for this :-) ?
Or shall I do it ?

> 
> ...repo forked...PR pending...
Comment 4 Michael Osipov 2017-03-17 18:34:41 UTC
(In reply to Philippe Mouawad from comment #3)
> (In reply to Michael Osipov from comment #2)
> > (In reply to Philippe Mouawad from comment #1)
> > > Hello Michael,
> > > Thanks for your proposal.
> > > Would it be possible to provide a patch or a PR on our github :
> > > - https://github.com/apache/jmeter
> > > 
> > > See:
> > > - http://jmeter.apache.org/building.html
> > 
> > The patch is the least problem. The problem is this:
> > 
> > https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065
> > 
> > Non-Maven build, it requires direct Internet connection. I have to see how I
> > can change this to use either our HTTP proxy or Nexus instance otherwise I
> > won't be able to build anything at work. "Setproxy Task" should do it. You
> > should consider taking this from tomcat
> > https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713
> 
> Good idea
> Will you also contribute a patch for this :-) ?
> Or shall I do it ?

Please go ahead and do it. My Ant foo has faded since Maven.
I will focus on the PR.
Comment 5 Felix Schumacher 2017-03-17 18:43:27 UTC
(In reply to Michael Osipov from comment #2)
> (In reply to Philippe Mouawad from comment #1)
> > Hello Michael,
> > Thanks for your proposal.
> > Would it be possible to provide a patch or a PR on our github :
> > - https://github.com/apache/jmeter
> > 
> > See:
> > - http://jmeter.apache.org/building.html
> 
> The patch is the least problem. The problem is this:
> 
> https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065
> 
> Non-Maven build, it requires direct Internet connection. I have to see how I
> can change this to use either our HTTP proxy or Nexus instance otherwise I
> won't be able to build anything at work. "Setproxy Task" should do it. You
> should consider taking this from tomcat
> https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713
> 
> ...repo forked...PR pending...

Just add your nexus repo url as maven2.repo to build-local.properties.

That should be enough to use nexus as the source for the downloads.
Comment 6 Michael Osipov 2017-03-17 19:25:11 UTC
(In reply to Felix Schumacher from comment #5)
> (In reply to Michael Osipov from comment #2)
> > (In reply to Philippe Mouawad from comment #1)
> > > Hello Michael,
> > > Thanks for your proposal.
> > > Would it be possible to provide a patch or a PR on our github :
> > > - https://github.com/apache/jmeter
> > > 
> > > See:
> > > - http://jmeter.apache.org/building.html
> > 
> > The patch is the least problem. The problem is this:
> > 
> > https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065
> > 
> > Non-Maven build, it requires direct Internet connection. I have to see how I
> > can change this to use either our HTTP proxy or Nexus instance otherwise I
> > won't be able to build anything at work. "Setproxy Task" should do it. You
> > should consider taking this from tomcat
> > https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713
> > 
> > ...repo forked...PR pending...
> 
> Just add your nexus repo url as maven2.repo to build-local.properties.
> 
> That should be enough to use nexus as the source for the downloads.

Yes, this makes sense, but you still need to pass credentials for the <get> task. Can you add this to the build.xml?
Comment 7 Felix Schumacher 2017-03-17 19:40:55 UTC
(In reply to Michael Osipov from comment #6)
> (In reply to Felix Schumacher from comment #5)
> > (In reply to Michael Osipov from comment #2)
> > > (In reply to Philippe Mouawad from comment #1)
> > > > Hello Michael,
> > > > Thanks for your proposal.
> > > > Would it be possible to provide a patch or a PR on our github :
> > > > - https://github.com/apache/jmeter
> > > > 
> > > > See:
> > > > - http://jmeter.apache.org/building.html
> > > 
> > > The patch is the least problem. The problem is this:
> > > 
> > > https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065
> > > 
> > > Non-Maven build, it requires direct Internet connection. I have to see how I
> > > can change this to use either our HTTP proxy or Nexus instance otherwise I
> > > won't be able to build anything at work. "Setproxy Task" should do it. You
> > > should consider taking this from tomcat
> > > https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713
> > > 
> > > ...repo forked...PR pending...
> > 
> > Just add your nexus repo url as maven2.repo to build-local.properties.
> > 
> > That should be enough to use nexus as the source for the downloads.
> 
> Yes, this makes sense, but you still need to pass credentials for the <get>
> task. Can you add this to the build.xml?

If your nexus is secured by password, than this will probably not work, but you could have a look at:

https://bz.apache.org/bugzilla/show_bug.cgi?id=53709

Hope that helps.
Comment 8 Michael Osipov 2017-03-17 19:59:12 UTC
(In reply to Felix Schumacher from comment #7)
> (In reply to Michael Osipov from comment #6)
> > (In reply to Felix Schumacher from comment #5)
> > > (In reply to Michael Osipov from comment #2)
> > > > (In reply to Philippe Mouawad from comment #1)
> > > > > Hello Michael,
> > > > > Thanks for your proposal.
> > > > > Would it be possible to provide a patch or a PR on our github :
> > > > > - https://github.com/apache/jmeter
> > > > > 
> > > > > See:
> > > > > - http://jmeter.apache.org/building.html
> > > > 
> > > > The patch is the least problem. The problem is this:
> > > > 
> > > > https://github.com/apache/jmeter/blob/trunk/build.xml#L3052-L3065
> > > > 
> > > > Non-Maven build, it requires direct Internet connection. I have to see how I
> > > > can change this to use either our HTTP proxy or Nexus instance otherwise I
> > > > won't be able to build anything at work. "Setproxy Task" should do it. You
> > > > should consider taking this from tomcat
> > > > https://github.com/apache/tomcat85/blob/trunk/build.xml#L2709-L2713
> > > > 
> > > > ...repo forked...PR pending...
> > > 
> > > Just add your nexus repo url as maven2.repo to build-local.properties.
> > > 
> > > That should be enough to use nexus as the source for the downloads.
> > 
> > Yes, this makes sense, but you still need to pass credentials for the <get>
> > task. Can you add this to the build.xml?
> 
> If your nexus is secured by password, than this will probably not work, but
> you could have a look at:
> 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=53709

Given that Tomcat does the right thing in its build.xml and it works in our corporate environment, I really do not understand why 53709 was closed as invalid. This is truly an important thing for everything trying to contribute.

BTW, this does its job:

> diff --git a/build.xml b/build.xml
> index 4ddd4513f..d68adb9e6 100644
> --- a/build.xml
> +++ b/build.xml
> @@ -3054,7 +3054,8 @@ run JMeter unless all the JMeter jars are added.
>      <echo message="Fetching: ${path}/${jar}" level="info"/>
>      <get src="${loc}/${jar}"
>           dest="${build.dir}/${jar}"
> -         usetimestamp="false" ignoreerrors="false"/>
> +         usetimestamp="false" ignoreerrors="false"
> +         username="${nexus.username}" password="${nexus.password}"/>
>      <antcall target="_checkMD5">
>        <param name="file" value="${jar}"/>
>        <param name="path" value="${build.dir}"/>
>
Comment 9 Michael Osipov 2017-03-17 20:10:33 UTC
I must correct myself, it doesn't. Checkstyle is downloaded from SF.net as an Über JAR.
Comment 10 Michael Osipov 2017-03-17 20:50:18 UTC
OK, I have to take care of the patch at home next week. The tests don't pass at work because some of the require outbound, direct connection to the Internet like to 8.8.8.8 or HTTP Proxy Sampler to jmeter.apache.org.
Comment 11 Felix Schumacher 2017-03-17 20:51:16 UTC
(In reply to Michael Osipov from comment #9)
> I must correct myself, it doesn't. Checkstyle is downloaded from SF.net as
> an Über JAR.

I think, that the discussion about ant ant proxy gets a bit out of control on this bug entry. So let us take it either to the dev list, or to 53709, where it is probably better suited.
Comment 12 Felix Schumacher 2017-03-17 20:53:33 UTC
(In reply to Michael Osipov from comment #10)
> OK, I have to take care of the patch at home next week. The tests don't pass
> at work because some of the require outbound, direct connection to the
> Internet like to 8.8.8.8 or HTTP Proxy Sampler to jmeter.apache.org.

I think, those bugs could be reported on 
https://bz.apache.org/bugzilla/show_bug.cgi?id=60435

(or discussed on dev mailing list)
Comment 13 Michael Osipov 2017-03-17 21:27:00 UTC
(In reply to Felix Schumacher from comment #12)
> (In reply to Michael Osipov from comment #10)
> > OK, I have to take care of the patch at home next week. The tests don't pass
> > at work because some of the require outbound, direct connection to the
> > Internet like to 8.8.8.8 or HTTP Proxy Sampler to jmeter.apache.org.
> 
> I think, those bugs could be reported on 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=60435
> 
> (or discussed on dev mailing list)

Already did.
Comment 14 Michael Osipov 2017-04-05 13:50:41 UTC
Created PR: https://github.com/apache/jmeter/pull/288
Comment 15 Felix Schumacher 2017-04-06 17:12:52 UTC
Thanks for your contribution.

Date: Thu Apr  6 17:04:19 2017
New Revision: 1790429

URL: http://svn.apache.org/viewvc?rev=1790429&view=rev
Log:
Add ${__escapeXml()} function. Contributed by Michael Osipov (michaelo at apache.org)

This closes #288 on github.

Bugzilla Id: 60883

Modified:
    jmeter/trunk/docs/changes.html
    jmeter/trunk/docs/usermanual/functions.html
    jmeter/trunk/src/core/org/apache/jmeter/resources/messages.properties
    jmeter/trunk/src/core/org/apache/jmeter/resources/messages_fr.properties
    jmeter/trunk/test/src/org/apache/jmeter/functions/TestSimpleFunctions.java
    jmeter/trunk/xdocs/changes.xml
    jmeter/trunk/xdocs/usermanual/functions.xml

Date: Thu Apr  6 17:10:23 2017
New Revision: 1790432

URL: http://svn.apache.org/viewvc?rev=1790432&view=rev
Log:
Add the missing part of pr #288, which is the new function itself.

Contributed by Michael Osipov.

Bugzill Id: 60883

Added:
    jmeter/trunk/src/functions/org/apache/jmeter/functions/EscapeXml.java
Comment 16 The ASF infrastructure team 2022-09-24 20:38:08 UTC
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/4336