Bug 615978
Summary: | Create bundle using 'Recipe' > 'Click to Upload A Recipe File' fails | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Other] RHQ Project | Reporter: | Rajan Timaniya <rtimaniy> | ||||||||||||||||
Component: | Documentation | Assignee: | John Mazzitelli <mazz> | ||||||||||||||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Mike Foley <mfoley> | ||||||||||||||||
Severity: | medium | Docs Contact: | |||||||||||||||||
Priority: | medium | ||||||||||||||||||
Version: | 3.0.0, 3.0.1 | CC: | cwelton, loleary, mazz, mhideo, skondkar | ||||||||||||||||
Target Milestone: | --- | Keywords: | Reopened | ||||||||||||||||
Target Release: | RHQ 4.4.0 | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | Linux | ||||||||||||||||||
Whiteboard: | |||||||||||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||||||||||
Doc Text: | Story Points: | --- | |||||||||||||||||
Clone Of: | Environment: | ||||||||||||||||||
Last Closed: | 2013-09-01 10:02:39 UTC | Type: | --- | ||||||||||||||||
Regression: | --- | Mount Type: | --- | ||||||||||||||||
Documentation: | --- | CRM: | |||||||||||||||||
Verified Versions: | Category: | --- | |||||||||||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||||||||||
Embargoed: | |||||||||||||||||||
Bug Depends On: | |||||||||||||||||||
Bug Blocks: | 577210, 735475, 782579, 806214 | ||||||||||||||||||
Attachments: |
|
Created attachment 432848 [details]
recipe xml
Created attachment 432849 [details]
server log
This is a known limitation of the direct "upload via recipe" feature. I documented this in our dev wiki here: http://rhq-project.org/display/RHQ/Design+-+Policy which yes is hidden away. Should probably put this in our user docs somewhere. Specifically, the yellow box that says: "For some reason, if you file upload a ANT script as the recipe, you can't use XML notation like <property name="a" /> - you need to explicitly provide the ending tag like this: <property name="a"></property>. If you don't want to be forced into using that notation, just copy-n-paste the ANT script content directly in the text field, as opposed to using the file upload mechanism." Fix the rhq:input-property tag in the attached deploy.xml so it doesn't use the <tag/> notation and try again. No tags should use the <tag/> notation when you directly upload the recipe xml. NOTE: this is ONLY a limitation when uploading the recipe directly - when uploading a bundle distribution file, your XML can have <tag/> notation. Moving back to ON_QA to test using a different XML file. *** Bug 615980 has been marked as a duplicate of this bug. *** Tested on jon-2.4.0.GA_QA build#70 Modified the recipe file which explicitly provides the ending tags. Uploaded via recipe upload control in firefox browser. The recipe is uploaded successfully. However, tried to upload the modified recipe file which explicitly provides the ending tags via recipe upload control in IE browser, and observed that only a portion of the file is uploaded and is shown in the recipe text area. (Please refer the screenshot and attached modified recipe file) Observed the issue in below IE versions: IE6: 6.0.2900.5512.xpsp_sp3_gdr.100216-1514 IE7: 7.0.5730.11 IE8: 8.0.6001.18702 Created attachment 433151 [details]
Screenshot on IE
Created attachment 433152 [details]
Modified recipe file
Mazz, I'm assuming the workaround "just copy-n-paste the ANT script content directly in the text field, as opposed to using the file upload mechanism." still applies here, if so we can drop the priority on this. you should always be able to just cut-n-paste directly into the text field and click next to submit the recipe that way. Perhaps we should just disable the whole "upload" part of that, I have no idea why the XML gets screwed up - I don't know if it happens on the client during the request, on the server's servlet that echoes it back, or on the client during the response processing. But somewhere in that chain, there is very bad xml processing going on and we need to stop it. org.rhq.enterprise.gui.coregui.client.components.upload.DynamicCallbackForm private void onFrameLoadImpl() { if (formHandlers != null) { // Fire onComplete events in a deferred command. This is necessary // because clients that detach the form panel when submission is // complete can cause some browsers (i.e. Mozilla) to go into an // 'infinite loading' state. See issue 916. DeferredCommand.addCommand(new Command() { public void execute() { formHandlers.fireOnComplete(DynamicCallbackForm.this, impl.getContents(synthesizedFrame)); } }); } } that "impl.getContents(synthesizedFrame) is coming back null and that is supposed to be the response from the server (i.e. the XML file content). Unless we return a content-type of text/html with <html> content, this doesn't want to work. I tweeked FileUploadServlet so it returns properly the file, verified with a TCP/IP listener that the server is indeed returning proper HTTP response (i.e. content type is text/xml and the response text is the full, valid XML file that I uploaded). But this client code doesn't like that. I reverted code back to the way it was (i did not check in any fix to FileUploadServlet; until we fix the client code, we can't fix FileUploadServlet). Here's the thread on the client - you can see the callback making its way into BundleUploadDistroFileStep into the complete-event callback method. Daemon Thread [Code server for org.rhq.enterprise.gui.coregui.CoreGUI from Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100330 Fedora/3.5.9-1.fc11 Firefox/3.5.9 on http://localhost:7080/coregui/CoreGUI.html?gwt.codesvr=127.0.0.1:9997#Bundles @ d""0pZe^|=W1z?qE] (Suspended (breakpoint at line 187 in BundleUploadDistroFileStep$4)) BundleUploadDistroFileStep$4.onSubmitComplete(DynamicFormSubmitCompleteEvent) line: 187 DynamicCallbackFormHandlerCollection.fireOnComplete(DynamicCallbackForm, String) line: 30 DynamicCallbackForm$1.execute() line: 108 CommandExecutor.doExecuteCommands(double) line: 310 CommandExecutor$2.run() line: 205 CommandExecutor$2(Timer).fire() line: 141 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25 Method.invoke(Object, Object...) line: 597 MethodAdaptor.invoke(Object, Object...) line: 103 MethodDispatch.invoke(JsValue, JsValue[], JsValue) line: 71 OophmSessionHandler.invoke(BrowserChannel, BrowserChannel$Value, int, BrowserChannel$Value[]) line: 157 BrowserChannelServer(BrowserChannel).reactToMessagesWhileWaitingForReturn(BrowserChannel$SessionHandler) line: 1713 BrowserChannelServer.invokeJavascript(CompilingClassLoader, JsValueOOPHM, String, JsValueOOPHM[], JsValueOOPHM) line: 165 ModuleSpaceOOPHM.doInvoke(String, Object, Class<?>[], Object[]) line: 120 ModuleSpaceOOPHM(ModuleSpace).invokeNative(String, Object, Class<?>[], Object[]) line: 507 ModuleSpaceOOPHM(ModuleSpace).invokeNativeObject(String, Object, Class<?>[], Object[]) line: 264 JavaScriptHost.invokeNativeObject(String, Object, Class<?>[], Object[]) line: 91 Impl.apply(Object, Object, Object) line: not available Impl.entry0(Object, Object, Object) line: 188 GeneratedMethodAccessor108.invoke(Object, Object[]) line: not available DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25 Method.invoke(Object, Object...) line: 597 MethodAdaptor.invoke(Object, Object...) line: 103 MethodDispatch.invoke(JsValue, JsValue[], JsValue) line: 71 OophmSessionHandler.invoke(BrowserChannel, BrowserChannel$Value, int, BrowserChannel$Value[]) line: 157 BrowserChannelServer(BrowserChannel).reactToMessages(BrowserChannel$SessionHandler) line: 1668 BrowserChannelServer.processConnection() line: 401 BrowserChannelServer.run() line: 222 Thread.run() line: 619 Here's the fix to FileUploadServlet that needs to get done: @@ -117,17 +117,19 @@ public class FileUploadServlet extends HttpServlet { if (retrieve && actualFiles.size() == 1) { // sending in "retrieve" form element with a single file means the client just wants the content echoed back FileItem fileItem = actualFiles.get(0); + resp.setContentType(fileItem.getContentType()); + ServletOutputStream outputStream = resp.getOutputStream(); - outputStream.print("<html>"); + //outputStream.print("<html>"); InputStream inputStream = fileItem.getInputStream(); try { StreamUtil.copy(inputStream, outputStream, false); } finally { inputStream.close(); } - outputStream.print("</html>"); + //outputStream.print("</html>"); outputStream.flush(); fileItem.delete(); } else { Dropping priority/severity on this since mazz indicated it occurs only IE can there is a work around (copy/paste) This issue needs to be doc'd This is a known issue in the release notes: http://www.redhat.com/docs/en-US/JBoss_ON/2.4/Release_Notes/html-single/#provision-issues I also have it as a note in the real provisioning docs (under step #3): http://www.redhat.com/docs/en-US/JBoss_ON/2.4/admin/html/uploading-bundles.html QA verified doc change Re-opened as this issue appears to happen on Firefox and other browsers and is not limited to Windows IE. Created attachment 573203 [details]
Propsed patch to resolve this issue
This patch will resolve this issue. Some important notes/consideration:
- it adds the dependency on the Apache commons-lang artifact to perform HTML encoding/escaping. If we do not want to bring this dependency in for that method, perhaps there is an alternative provided by one of the JBoss libraries or we could write our own.
- it reads the input stream provided to the servlet's doPost method and copies it to a String which it then performs HTML escaping. There is no character set handling which means that this may not always be safe when converted back to a byte array to be pushed into the output stream.
Basically, the reason this issue occurs in the first place is because we are pushing the contents of an XML file into an HTML element without any special handling. It is surprising that we are not seeing other issues with the recipe file being mangled during upload.
I am setting target to RHQ 4.4 in the hopes that I can get the proposed patch approved and in the 4.4 release. However, this is flexible so long as we get this issue fixed ASAP. A bit more explanation to the fix. The client must receive text/html or the result will be unpredictable. Additionally, FormPanel expects the server to return the content as type text/html - http://google-web-toolkit.googlecode.com/svn/javadoc/latest/com/google/gwt/user/client/ui/FormPanel.html#FormPanel() The encoding is to properly place the content of the XML into the <html></html> elements to preserve the content and then have the client un-escape it when copying it to the recipe text box. Have you actually tried this? I would be impressed that Apache commons-lang can be compilable and loadable in the GWT client. I'm reassigning to Mike T - he said he was going to look into this issue. (In reply to comment #17) > Have you actually tried this? I would be impressed that Apache commons-lang can > be compilable and loadable in the GWT client. Ah. I see this was just a change to StreamUtil which is in core/util - and that is not used by the coregui GWT client. So it isn't an issue. However, let's see what else we can do without adding yet another Apache commons dependency in the project. Because core/util is used everywhere, this means it adds another dependency to the agent and the CLI as well as the server and probably the installer war and the REST stuff. Agreed on the dependency. It is only being used here for the HTML encoding. A similar method may already be provided by one of the JBoss libs we pull in or we could implement our own. I am sure the list of characters that require escaping is short. (In reply to comment #19) > Agreed on the dependency. It is only being used here for the HTML encoding. A > similar method may already be provided by one of the JBoss libs we pull in or > we could implement our own. I am sure the list of characters that require > escaping is short. I'm thinking we have this somewhere in our code already. For example, org.rhq.enterprise.gui.legacy.action.resource.common.monitor.visibility.ViewChartAction.forHTMLTag(String) - which is in our portal-war but it simple enough that we can just copy and use a private method in the StreamUtil. private static String forHTMLTag(String aTagFragment) { final StringBuffer result = new StringBuffer(); final StringCharacterIterator iterator = new StringCharacterIterator(aTagFragment); for (char character = iterator.current(); character != CharacterIterator.DONE; character = iterator.next()) { switch (character) { case '<': result.append("<"); break; case '>': result.append(">"); break; case '\"': result.append("""); break; case '\'': result.append("'"); break; case '\\': result.append("\"); break; case '&': result.append("&"); break; case '|': result.append("|"); break; case ',': result.append(","); break; default: // the char is not a special one add it to the result as is result.append(character); break; } } return result.toString(); NOTE: we have one in coregui (which can't be used in the server jar) but it looks like it is not as complete: org.rhq.enterprise.gui.coregui.client.util.StringUtility.escapeHtml(String) Created attachment 573808 [details] Propsed patch without Commons Lang dependency New proposed patch based on response in comment 20. It removes the Commons Lang dependency and uses a private method to escape the HTML. Of course, I should note that the comment from our own code suggests that we should be using Commons Lang seeing that it maintains the HTML version compliance. I see this BZ has been re-assigned so I will await the resolution for inclusion in downstream. applied the patch to master: dcaae811c774556adeaf22fc983df2661f2f9a38 wrote a unit test for it and fixed a bug detected in patch, applied to master: a41828d54f6ef5cf283e8607d2953a6d9822ad89 Bulk closing of items that are on_qa and in old RHQ releases, which are out for a long time and where the issue has not been re-opened since. |
Created attachment 432847 [details] screenshot Description of problem: Create bundle using 'Recipe' > 'Click to Upload A Recipe File' fails. It gives the error on screen: java.lang.Exception:Failed to parse the bundle Ant script. -> org.apache.tools.ant.BuildException:rhq:input-property doesn't support the nested "deployment-unit" element. -> org.apache.tools.ant.UnsupportedElementException:The type doesn't support the nested "deployment-unit" element. Version-Release number of selected component (if applicable): version: 2.4.0.GA_QA build number: 10852:16ea29a JON tag-jon-release (build #69) How reproducible: Always Steps to Reproduce: 1) Log-in to JON 2) Goto Administration > Content > Bundles 3) Create new bundle (clickon 'New' button of 'Bundles') 4) Select 'Recipe' and click on link 'Click to Upload A Recipe File' 5) Input recipe file location 6) Click on 'Upload' button 6) Click on 'Next' button Actual results: Create bundle using 'Recipe' > 'Click to Upload A Recipe File' failed to create bundle. Expected results: Bundle should be created using 'Recipe' > 'Click to Upload A Recipe File' option. Additional info: JON shows the differ content in recipe free form text rather then input recipe xml. (This observed after step-6) Please refer attached file for recipe xml.