Skip to content
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.

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

what is the proper encoding of multi-line strings? The newlines currently seem to get gobbled up #64

Closed
jstrachan opened this issue Feb 12, 2013 · 13 comments
Milestone

Comments

@jstrachan
Copy link
Contributor

@jstrachan jstrachan commented Feb 12, 2013

I've tried replacing "\n" with "\n" or replacing a string with an array of strings for each line; but can't figure out how from jolokia's javascript client to invoke a JMX method passing in a single String with the newline characters preserved

Any ideas?

@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 12, 2013

I tried this - but then the call to jolokia.execute() decided that the single String argument with carriage returns was now really a sequence of arguments - which then broke the JMX call

        contents = contents.replace(/\n/g, '\\n');
@rhuss
Copy link
Owner

@rhuss rhuss commented Feb 12, 2013

For GET requests, there is indeed an issue (mostly because '' cannot be used in URL's pathinfo, neither URL encoded nor otherwise, since it gets translated by all JEE container I know to a '/' before its given over to any servlet or so. Security related, I bet. One has to use the version of GET request, which uses a query parameter, but the JS client lib should take transparently care of it)

For POST request the standard semantics apply (no special escaping necessary for newlines).

This is true for the jolokia-simple API as well: POST works, GET not. I.e. these calls to execute works:

value = j4p.execute("jolokia.it:type=operation", "arrayArguments", 
                                "Max\nMorlock,blub", "x", { method: "POST" });
equals(value, "Max\nMorlock");
value = j4p.execute("jolokia.it:type=operation", "arrayArguments", 
                                 [ "Max\nMorlock", "blub"], "x", { method: "POST"});
equals(value, "Max\nMorlock");

for an MBean operation

public String arrayArguments(String args[], String extra) {
   return args[0];
}

BTW, you can set the default HTTP method in the constructor of the Jolokia client. POST is typically more robust then GET anyways (and is required e.g. bulk requests).

This will be fixed for the 1.1.0 which is scheduled for calendar week 9.

rhuss added a commit that referenced this issue Feb 12, 2013
@rhuss
Copy link
Owner

@rhuss rhuss commented Feb 12, 2013

Fix was easy (added encodeURIComponent for the URl parts) ;-)

1.1.0-SNAPSHOT is on the way to our repo.

@rhuss rhuss closed this Feb 12, 2013
@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 12, 2013

Awesome - many thanks! :)

@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 13, 2013

I just tried 1.1.0-SNAPSHOT and still have the same issues (I should have read your response a bit closer :).

I'm trying to pass in a string with newlines to an MBean which currently just takes a String result...
https://github.com/hawtio/hawtio/blob/master/hawtio-git/src/main/java/io/hawt/git/GitFacadeMXBean.java#L32

I'm trying to use jolokia.execute(mbean, operation, argument, onSuccess) as follows:
https://github.com/hawtio/hawtio/blob/master/hawtio-web/src/main/webapp/app/git/js/git.ts#L51

Would it be more 'jolokia' to use a String[] in the mbean and pass in an array of strings? Sounds like a potentially easier solution?

@rhuss rhuss reopened this Feb 13, 2013
@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 13, 2013

I just experimented btw using String[] as the parameter on the mbean then tried passing in either a String with newlines and a JS array and got various JSON parsing errors from joloka. e.g.
Jolokia request failed: java.lang.IllegalArgumentException : Cannot parse JSON abc: Unexpected character (a) at position 0.

Experimenting some more...

@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 13, 2013

Aha - so ignoring the String[] stuff and just using String, and using 1.1.0-SNAPSHOT and making sure I used method: "POST" (and properly rebuilding & reloading everything :) - things seem to work great for sending the newline strings!

I just need to confirm I can get them back now. Sorry for the noise!

@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 13, 2013

Yay! I can confirm that both passing a string with newlines via method: "POST" and consuming them is working fine for me now on 1.1.0-SNAPSHOT! Many thanks!

@jstrachan jstrachan closed this Feb 13, 2013
@rhuss
Copy link
Owner

@rhuss rhuss commented Feb 13, 2013

Sorry, my snapshot upload yesterday wasn't succesful (as it seems) ;-(. In your joloki.js, can you see this line --> https://github.com/rhuss/jolokia/blob/integration/client/javascript/src/main/javascript/jolokia.js#L623

I'll publish a snapshot right now, hopefully it will finish before I leave today (pushing to our repo takes ridicously long, don't exactly know why ....)

What you can always do (already with 1.0.6): In

this.jolokia.execute(this.mbean, "write", this.branch, path, commitMessage, 
            authorName, authorEmail, contents, onSuccess(fn));

could you add in the onSuccess() return value a method: "POST" so that it results in something like

this.jolokia.execute(this.mbean, "write", this.branch, path, commitMessage,
            authorName, authorEmail, contents, { method : "POST", success: fn});

Jolokia itself doesn't have any particular preference for how the signature should like, so your MBean is perfectly fine.

@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 13, 2013

Many thanks - btw hope I'm not disturbing your skiing :)

@jstrachan
Copy link
Contributor Author

@jstrachan jstrachan commented Feb 13, 2013

Yeah - I hacked in the method: "POST" thing and its all working great now!

@rhuss
Copy link
Owner

@rhuss rhuss commented Feb 13, 2013

No problem ;-). Being away for four days from the keyboard is long enough anyways, so some last minute fixes are fine ;-)

@rhuss
Copy link
Owner

@rhuss rhuss commented Feb 13, 2013

Good to know that it works with POST, will work with GET, too as soon as I get my SNAPSHOT up to the repo ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.