Bug 615978

Summary: Create bundle using 'Recipe' > 'Click to Upload A Recipe File' fails
Product: [Other] RHQ Project Reporter: Rajan Timaniya <rtimaniy>
Component: DocumentationAssignee: John Mazzitelli <mazz>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: medium Docs Contact:
Priority: medium    
Version: 3.0.0, 3.0.1CC: 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 06:02:39 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 577210, 735475, 782579, 806214    
Attachments:
Description Flags
screenshot
none
recipe xml
none
server log
none
Screenshot on IE
none
Modified recipe file
none
Propsed patch to resolve this issue
none
Propsed patch without Commons Lang dependency none

Description Rajan Timaniya 2010-07-19 08:21:22 EDT
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.
Comment 1 Rajan Timaniya 2010-07-19 08:21:54 EDT
Created attachment 432848 [details]
recipe xml
Comment 2 Rajan Timaniya 2010-07-19 08:24:05 EDT
Created attachment 432849 [details]
server log
Comment 3 John Mazzitelli 2010-07-19 10:22:08 EDT
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.
Comment 4 Charles Crouch 2010-07-19 10:29:50 EDT
*** Bug 615980 has been marked as a duplicate of this bug. ***
Comment 5 Sunil Kondkar 2010-07-20 08:45:18 EDT
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
Comment 6 Sunil Kondkar 2010-07-20 08:46:20 EDT
Created attachment 433151 [details]
Screenshot on IE
Comment 7 Sunil Kondkar 2010-07-20 08:46:52 EDT
Created attachment 433152 [details]
Modified recipe file
Comment 8 Charles Crouch 2010-07-20 08:50:11 EDT
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.
Comment 9 John Mazzitelli 2010-07-20 09:27:10 EDT
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.
Comment 10 John Mazzitelli 2010-07-20 14:18:54 EDT
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 {
Comment 11 Charles Crouch 2010-07-20 15:31:07 EDT
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
Comment 12 Deon Ballard 2010-08-02 11:58:56 EDT
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
Comment 13 Corey Welton 2010-08-18 15:38:53 EDT
QA verified doc change
Comment 14 Larry O'Leary 2012-03-06 18:28:33 EST
Re-opened as this issue appears to happen on Firefox and other browsers and is not limited to Windows IE.
Comment 15 Larry O'Leary 2012-03-27 22:05:53 EDT
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.
Comment 16 Larry O'Leary 2012-03-27 22:15:44 EDT
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.
Comment 17 John Mazzitelli 2012-03-28 09:15:53 EDT
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.
Comment 18 John Mazzitelli 2012-03-28 09:21:02 EDT
(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.
Comment 19 Larry O'Leary 2012-03-28 09:31:15 EDT
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.
Comment 20 John Mazzitelli 2012-03-28 09:44:19 EDT
(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("&lt;");
                break;
            case '>':
                result.append("&gt;");
                break;
            case '\"':
                result.append("&quot;");
                break;
            case '\'':
                result.append("&#039;");
                break;
            case '\\':
                result.append("&#092;");
                break;
            case '&':
                result.append("&amp;");
                break;
            case '|':
                result.append("&#124;");
                break;
            case ',':
                result.append("&#44;");
                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)
Comment 21 Larry O'Leary 2012-03-29 17:47:41 EDT
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.
Comment 22 John Mazzitelli 2012-04-11 14:47:18 EDT
applied the patch to master:

   dcaae811c774556adeaf22fc983df2661f2f9a38

wrote a unit test for it and fixed a bug detected in patch, applied to master:

   a41828d54f6ef5cf283e8607d2953a6d9822ad89
Comment 23 Heiko W. Rupp 2013-09-01 06:02:39 EDT
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.