Setting JVM memory options

1,259 views
Skip to first unread message

psiinon

unread,
Aug 13, 2015, 8:41:57 AM8/13/15
to OWASP ZAP Developer Group
Just had a great suggestion for setting JVM memory options on Linux/Macs.
You cant set the memory options from a Java program, but we could save the memory options to a file and then read those from zap.sh
So the changes would be:
  1. Add a new options page for JVM parameters, at least -Xmx, maybe others? We will need to warn that a restart will be required
  2. Save those options to a file, something like ~/.ZAP_JVM.properties
  3. Change zap.sh to check if such a file exists and then to use it when starting ZAP

Does this sound like a workable plan?

Is the suggested file ok? It will mean that we'll only support one set of options per user.

Should we parse and validate the params either in ZAP and/or in the script?

Alternatively we could just have a single text field for the options, and if present write the contents to the properties file, and always add these to the java call no matter what. That would be much more flexible, make the implementation easier but could introduce the possibility of someone using this as a way to attack ZAP, eg by calling a different class.


Note that the Windows exe is created using Launch4j. It looks like this already supports overriding the JVM options via a config file: http://launch4j.sourceforge.net/docs.html#Additional_jvm_options


Feedback appreciated...


Simon

April

unread,
Aug 13, 2015, 2:04:44 PM8/13/15
to OWASP ZAP Developer Group
I think having memory usage as an option inside ZAP is a fine idea.

As essentially the core maintainer of zap.sh, I'd much rather that we did *not* store the setting in a dotfile in somebody's home directory.  I'd way rather we just store the setting the same way that ZAP stores all of its other preferences, and then just add a function that I can call via the command line that will tell me what the current setting is.  Alternatively, I can just try to grep that setting right out of config.xml, which might just be the least amount of work for everyone.

Anyways, I don't know if that will work for Windows, but I don't like the idea of having our preferences stored in multiple places.

psiinon

unread,
Aug 14, 2015, 3:11:06 AM8/14/15
to OWASP ZAP Developer Group
The reasons I didnt suggest using the config.xml file were:
  1. We have to find it, which means duplicating a load of ZAP code, with the potential for this to get out of step. Eg we need to know if its Mac or Linux (they use different default dirs) and if a -dir option is supplied
  2. We then have to parse an XML file to find the value, which could introduce dependencies on software that might not be available on all systems
A command line function _might_ be a safe option, but I'm worried about how long that could take to run. But I'll have a play :)
Any other thoughts or suggestions?


Cheers,


Simon

psiinon

unread,
Aug 14, 2015, 9:28:56 AM8/14/15
to OWASP ZAP Developer Group
I've knocked up a quick function that initialises the config file using the existing ZAP classes.
Unfortunately this takes over 2 seconds to run, which might not sound much, but I'm not keen on anything that impacts the startup time.
We could obviously use different code which doesnt use the existing methods, but then we have duplicate code which could get out of step :(
The code that I thinks doing too much is the call to Model.getSingleton().init(getOverrides()) - without that it takes just under 1/2 a second, so we could drop that and use the XML parsing classes directly.

So our options are:
  1. Use a new config file - v fast, but another config file
  2. Use existing config.xml with existing code, reuses existing code, but 2 sec startup penalty
  3. Use existing config.xml with simpler xml parsing, some duplicated code, but less that 1/2 sec startup penalty
  4. Insert your suggestion here ;)

What are your (addressed to everyone;) preferences?


Cheers,


Simon

April King

unread,
Aug 14, 2015, 11:11:31 AM8/14/15
to zaproxy...@googlegroups.com
I don’t see what #1 gains us over simply using grep on the existing config.xml files.

It would be /relatively/ simple in zap.sh to:

1) search for -dir or -a or whatever it is that specifies the path to the config.xml file
1a) if present, grep in that config.xml file for JavaMaxMemory (or whatever), passing it to cut
1b) if not present, and release, grep in ~/Library/Application Support/ZAP/config.xml and ~/.zap/config.xml for that setting
1c) if not present, and dev, grep in ~/Library/Application Support/ZAP_D/config.xml and ~/.zap_d/config.xml (is that right?) for that setting
2) Use that setting

Barring doing it that way, I think #3 is the best option.  I definitely think #1 is no good simply because it has the same problem as grepping for it in zap.sh.  And 2 seconds extra is already too long for what is already the slowest loading program on my computer.

If you can get #3 down to a reasonably speedy startup time, then I think that would be ideal since it would make my life a lot easier.

~ April


--
You received this message because you are subscribed to a topic in the Google Groups "OWASP ZAP Developer Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/zaproxy-develop/9iMqEgzG1nE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to zaproxy-devel...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

psiinon

unread,
Aug 17, 2015, 3:33:40 AM8/17/15
to OWASP ZAP Developer Group
I think #1 can be implemented _very_ simply via something like:

exec java `cat ~/.ZAP_JVM.properties` -jar "${BASEDIR}/zap-dev.jar" "$@"

(assuming ~/.ZAP_JVM.properties exists;)
Thats simple and really fast.
We might even be able to support that in the zap.bat file, which would be a bonus.

If we go down the config.xml file route then I think there could be a load of 'gotchas'.
Some that come to mind:
  • We'd need to do something like checking the ZAP jar name to see if we're using a full or dev release - you can just check one then the other, as a user can use either
  • I've seen config.xml files with duplicate entries - we'd need to make sure the script uses the same value as java
I'll have to admit that I'm not a big fan of supporting shell scripts - I've done that in previous jobs and they were always a complete pain.
And I dont have enough experience to know all of the issues we'll get over the wide variety of *nix systems people use.

I'm going to start looking at the Windows exe version, which will _have_ to use the properties file that Launch4j supports.
Most of our downloads are for the Windows version, so I think this is important.
Then I'll have a look at option 3 and see if I can make that suitably quick.

Cheers,

Simon
To unsubscribe from this group and all its topics, send an email to zaproxy-develop+unsubscribe@googlegroups.com.

psiinon

unread,
Aug 17, 2015, 11:20:18 AM8/17/15
to OWASP ZAP Developer Group
I've implemented this using the ~/.ZAP_JVM.properties file.
This works when either the zap.sh or zap.bat file is used - it wont work for the Windows exe.
I like the fact that this is simple and keeps the JVM options used by the scripts away from the ZAP options.
But I'm still very open to objections, especially ones that come with full PRs ;)

Cheers,

Simon

April King [marumari]

unread,
Aug 17, 2015, 3:15:38 PM8/17/15
to OWASP ZAP Developer Group
If we do it this way, can we do it like this:

~/.zap/jvm.properties

This way, if we ever need another file, we can avoid polluting the people's home directories with a bunch of configuration files.  This is pretty standard, and I think all we should need to do is add code to create ~/.zap if it isn't there.
To unsubscribe from this group and all its topics, send an email to zaproxy-devel...@googlegroups.com.

psiinon

unread,
Aug 18, 2015, 7:24:09 AM8/18/15
to OWASP ZAP Developer Group
OK, ok, we really should use the default dirs, as per https://github.com/zaproxy/zaproxy/wiki/FAQconfig
I think its ok to use the default 'release' ones for both release and dev/weekly releases, rather than using the'_D' versions.
Reply all
Reply to author
Forward
0 new messages