Repository: drill
Updated Branches:
  refs/heads/master 7a900b71f -> c7c8ffd6f


DRILL-5766: Fix XSS vulnerabilities in Drill

1. Bumped up freemarker version to 2.3.26-incubating.
2. Indicated default output format in Freemarker configuration (HTML).
3. Fixed Web UI bugs listed in DRILL-5346, DRILL-5341, DRILL-5339, DRILL-5338.

closes #935


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/c7c8ffd6
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/c7c8ffd6
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/c7c8ffd6

Branch: refs/heads/master
Commit: c7c8ffd6f5e1b8f4677f9e95c317482d929a1bb5
Parents: 7a900b7
Author: Arina Ielchiieva <arina.yelchiy...@gmail.com>
Authored: Thu Sep 7 15:30:39 2017 +0300
Committer: Arina Ielchiieva <arina.yelchiy...@gmail.com>
Committed: Tue Sep 12 12:53:53 2017 +0300

----------------------------------------------------------------------
 .../drill/exec/server/rest/DrillRestServer.java |  43 ++++-
 .../drill/exec/server/rest/WebServer.java       |   2 +-
 .../src/main/resources/rest/logs/log.ftl        |   2 +-
 .../src/main/resources/rest/options.ftl         |   3 -
 .../src/main/resources/rest/profile/list.ftl    | 135 ++++---------
 .../src/main/resources/rest/profile/profile.ftl | 187 +++++++++++++------
 .../src/main/resources/rest/query/result.ftl    |   4 +-
 pom.xml                                         |   2 +-
 8 files changed, 207 insertions(+), 171 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
index bd01fea..238f5ca 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
@@ -22,6 +22,13 @@ import 
com.fasterxml.jackson.jaxrs.base.JsonMappingExceptionMapper;
 import com.fasterxml.jackson.jaxrs.base.JsonParseExceptionMapper;
 import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider;
 import com.google.common.base.Strings;
+import freemarker.cache.ClassTemplateLoader;
+import freemarker.cache.FileTemplateLoader;
+import freemarker.cache.MultiTemplateLoader;
+import freemarker.cache.TemplateLoader;
+import freemarker.cache.WebappTemplateLoader;
+import freemarker.core.HTMLOutputFormat;
+import freemarker.template.Configuration;
 import org.apache.drill.common.config.DrillConfig;
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.memory.BufferAllocator;
@@ -48,17 +55,22 @@ import 
org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature;
 import org.glassfish.jersey.server.mvc.freemarker.FreemarkerMvcFeature;
 
 import javax.inject.Inject;
+import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
+import java.io.File;
+import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.security.Principal;
+import java.util.ArrayList;
+import java.util.List;
 
 public class DrillRestServer extends ResourceConfig {
   static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillRestServer.class);
 
-  public DrillRestServer(final WorkManager workManager) {
+  public DrillRestServer(final WorkManager workManager, final ServletContext 
servletContext) {
     register(DrillRoot.class);
     register(StatusResources.class);
     register(StorageResources.class);
@@ -67,10 +79,14 @@ public class DrillRestServer extends ResourceConfig {
     register(MetricsResources.class);
     register(ThreadsResources.class);
     register(LogsResources.class);
+
+    property(FreemarkerMvcFeature.TEMPLATE_OBJECT_FACTORY, 
getFreemarkerConfiguration(servletContext));
     register(FreemarkerMvcFeature.class);
+
     register(MultiPartFeature.class);
     property(ServerProperties.METAINF_SERVICES_LOOKUP_DISABLE, true);
 
+
     final boolean isAuthEnabled =
         
workManager.getContext().getConfig().getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED);
 
@@ -112,6 +128,31 @@ public class DrillRestServer extends ResourceConfig {
     });
   }
 
+  /**
+   * Creates freemarker configuration settings,
+   * default output format to trigger auto-escaping policy
+   * and template loaders.
+   *
+   * @param servletContext servlet context
+   * @return freemarker configuration settings
+   */
+  private Configuration getFreemarkerConfiguration(ServletContext 
servletContext) {
+    Configuration configuration = new 
Configuration(Configuration.VERSION_2_3_26);
+    configuration.setOutputFormat(HTMLOutputFormat.INSTANCE);
+
+    List<TemplateLoader> loaders = new ArrayList<>();
+    loaders.add(new WebappTemplateLoader(servletContext));
+    loaders.add(new ClassTemplateLoader(DrillRestServer.class, "/"));
+    try {
+      loaders.add(new FileTemplateLoader(new File("/")));
+    } catch (IOException e) {
+      logger.error("Could not set up file template loader.", e);
+    }
+    configuration.setTemplateLoader(new 
MultiTemplateLoader(loaders.toArray(new TemplateLoader[loaders.size()])));
+    return configuration;
+  }
+
+
   public static class AuthWebUserConnectionProvider implements 
Factory<WebUserConnection> {
 
     @Inject

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
----------------------------------------------------------------------
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
index 3fc95cd..8818349 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
@@ -171,7 +171,7 @@ public class WebServer implements AutoCloseable {
     servletContextHandler.setErrorHandler(errorHandler);
     servletContextHandler.setContextPath("/");
 
-    final ServletHolder servletHolder = new ServletHolder(new 
ServletContainer(new DrillRestServer(workManager)));
+    final ServletHolder servletHolder = new ServletHolder(new 
ServletContainer(new DrillRestServer(workManager, 
servletContextHandler.getServletContext())));
     servletHolder.setInitOrder(1);
     servletContextHandler.addServlet(servletHolder, "/*");
 

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/logs/log.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/logs/log.ftl 
b/exec/java-exec/src/main/resources/rest/logs/log.ftl
index f5386bd..4f37beb 100644
--- a/exec/java-exec/src/main/resources/rest/logs/log.ftl
+++ b/exec/java-exec/src/main/resources/rest/logs/log.ftl
@@ -24,7 +24,7 @@
     <#if (model.getLines()?size > 0)>
     <pre>
         <#list model.getLines() as line>
-${line?html}
+${line}
         </#list>
      </pre>
     <#else>

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/options.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/options.ftl 
b/exec/java-exec/src/main/resources/rest/options.ftl
index 7ba1250..e280e81 100644
--- a/exec/java-exec/src/main/resources/rest/options.ftl
+++ b/exec/java-exec/src/main/resources/rest/options.ftl
@@ -18,9 +18,6 @@
   <div class="page-header">
   </div>
   <h4>System options</h4>
-  <div align="right">
-    <a 
href="http://drill.apache.org/docs/planning-and-execution-options/";>Documentation</a>
-  </div>
   <div class="table-responsive">
     <table class="table table-hover">
       <tbody>

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/profile/list.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/profile/list.ftl 
b/exec/java-exec/src/main/resources/rest/profile/list.ftl
index 1fcffb6..aba0033 100644
--- a/exec/java-exec/src/main/resources/rest/profile/list.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/list.ftl
@@ -28,55 +28,7 @@
   </#if>
   <#if (model.getRunningQueries()?size > 0) >
     <h3>Running Queries</h3>
-    <div class="table-responsive">
-      <table class="table table-hover">
-        <thead>
-           <td>Time</td>
-           <!-- <td>Query Id</td> -->
-           <td>User</td>
-           <td>Query</td>
-           <td>State</td>
-           <td>Elapsed</td>
-           <td>Foreman</td>
-        </thead>
-        <tbody>
-          <#list model.getRunningQueries() as query>
-          <tr>
-            <td>${query.getTime()}</td>
-            <!--
-            <td>
-              <a href="/profiles/${query.getQueryId()}">
-                <div style="height:100%;width:100%">
-                  ${query.getQueryId()}
-                </div>
-              </a>
-            </td>
-            -->
-            <td>
-              <a href="/profiles/${query.getQueryId()}">
-              <div 
style="height:100%;width:100%;white-space:pre-line">${query.getUser()}</div>
-              </a>
-            </td> 
-            <td>
-              <a href="/profiles/${query.getQueryId()}">
-              <div 
style="height:100%;width:100%;white-space:pre-line">${query.getQuery()}</div>
-              </a>
-            </td> 
-            <td>
-              <div style="height:100%;width:100%">${query.getState()}</div>
-            <td>
-              <div style="height:100%;width:100%">${query.getDuration()}</div>
-            <td>
-                <div style="height:100%;width:100%">
-                  ${query.getForeman()}
-                </div>
-            </td>
-            
-          </tr>
-          </#list>
-        </tbody>
-      </table>
-    </div>
+    <@list_queries queries=model.getRunningQueries()/>
     <div class="page-header">
     </div>
   <#else>
@@ -86,57 +38,40 @@
     </div>
   </#if>
   <h3>Completed Queries</h3>
-  <div class="table-responsive">
-    <table class="table table-hover">
-      <thead>
-         <td>Time</td>
-         <td>User</td>
-         <!-- <td>Query Id</td> -->
-         <td>Query</td>
-         <td>State</td>
-         <td>Duration</td>
-         <td>Foreman</td>
-      </thead>
-      <tbody>
-        <#list model.getFinishedQueries() as query>
-        <tr>
-          <td>${query.getTime()}</td>
-          <!--
-          <td>
-            <a href="/profiles/${query.getQueryId()}">
-              <div style="height:100%;width:100%">
-                ${query.getQueryId()}
-              </div>
-            </a>
-          </td>
-          -->
-          <td>
-            <a href="/profiles/${query.getQueryId()}">
-            <div 
style="height:100%;width:100%;white-space:pre-line">${query.getUser()}</div>
-            </a>
-          </td> 
-          
-          <td>
-            <a href="/profiles/${query.getQueryId()}">
-              <div 
style="height:100%;width:100%;white-space:pre-line">${query.getQuery()}</div>
-            </a>
-          </td>      
-          <td>
-              <div style="height:100%;width:100%">${query.getState()}</div>
-          </td>
-          <td>
-              <div style="height:100%;width:100%">${query.getDuration()}</div>
-          </td>
-          <td>
-              <div style="height:100%;width:100%">
-                ${query.getForeman()}
-              </div>
-          </td>
-        </tr>
-        </#list>
-      </tbody>
-    </table>
-  </div>
+  <@list_queries queries=model.getFinishedQueries()/>
+</#macro>
+
+<#macro list_queries queries>
+    <div class="table-responsive">
+        <table class="table table-hover">
+            <thead>
+            <tr>
+                <th>Time</th>
+                <th>User</th>
+                <th>Query</th>
+                <th>State</th>
+                <th>Duration</th>
+                <th>Foreman</th>
+            </tr>
+            </thead>
+            <tbody>
+            <#list queries as query>
+            <tr>
+                <td>${query.getTime()}</td>
+                <td>${query.getUser()}</td>
+                <td>
+                    <a href="/profiles/${query.getQueryId()}">
+                        <div 
style="height:100%;width:100%;white-space:pre-line">${query.getQuery()}</div>
+                    </a>
+                </td>
+                <td>${query.getState()}</td>
+                <td>${query.getDuration()}</td>
+                <td>${query.getForeman()}</td>
+            </tr>
+            </#list>
+            </tbody>
+        </table>
+    </div>
 </#macro>
 
 <@page_html/>
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/profile/profile.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/profile/profile.ftl 
b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
index 95d7d56..5bb7be8 100644
--- a/exec/java-exec/src/main/resources/rest/profile/profile.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
@@ -17,7 +17,7 @@
 <script>
     var globalconfig = {
         "queryid" : "${model.getQueryId()}",
-        "operators" : ${model.getOperatorsJSON()}
+        "operators" : ${model.getOperatorsJSON()?no_esc}
     };
 </script>
 </#macro>
@@ -32,7 +32,9 @@
     <li><a href="#query-physical" role="tab" data-toggle="tab">Physical 
Plan</a></li>
     <li><a href="#query-visual" role="tab" data-toggle="tab">Visualized 
Plan</a></li>
     <li><a href="#query-edit" role="tab" data-toggle="tab">Edit Query</a></li>
-    <#if model.hasError() ><li><a href="#query-error" role="tab" 
data-toggle="tab">Error</a></li></#if>
+    <#if model.hasError()>
+        <li><a href="#query-error" role="tab" data-toggle="tab">Error</a></li>
+    </#if>
   </ul>
   <div id="query-content" class="tab-content">
     <div id="query-query" class="tab-pane">
@@ -45,71 +47,133 @@
       <svg id="query-visual-canvas" class="center-block"></svg>
     </div>
     <div id="query-edit" class="tab-pane">
-      <form role="form" action="/query" method="POST">
-        <div class="form-group">
-          <textarea class="form-control" id="query" name="query" 
style="font-family: Courier;">${model.getProfile().query}</textarea>
-        </div>
-        <div class="form-group">
-          <div class="radio-inline">
-            <label>
-              <input type="radio" name="queryType" id="sql" value="SQL" 
checked>
-              SQL
-            </label>
-          </div>
-          <div class="radio-inline">
-            <label>
-              <input type="radio" name="queryType" id="physical" 
value="PHYSICAL">
-              PHYSICAL
-            </label>
-          </div>
-          <div class="radio-inline">
-            <label>
-              <input type="radio" name="queryType" id="logical" 
value="LOGICAL">
-              LOGICAL
-            </label>
+      <p>
+        <form role="form" action="/query" method="POST">
+          <div class="form-group">
+            <textarea class="form-control" id="query" name="query" 
style="font-family: Courier;">${model.getProfile().query}</textarea>
           </div>
-        </div>
-        <button type="submit" class="btn btn-default">Re-run query</button>
-      </form>
+          <div class="form-group">
+            <div class="radio-inline">
+              <label>
+                <input type="radio" name="queryType" id="sql" value="SQL" 
checked>
+                    SQL
+              </label>
+            </div>
+            <div class="radio-inline">
+              <label>
+                 <input type="radio" name="queryType" id="physical" 
value="PHYSICAL">
+                     PHYSICAL
+              </label>
+            </div>
+            <div class="radio-inline">
+              <label>
+                <input type="radio" name="queryType" id="logical" 
value="LOGICAL">
+                    LOGICAL
+              </label>
+            </div>
+            </div>
+            <button type="submit" class="btn btn-default">Re-run query</button>
+          </form>
+      </p>
+      <p>
       <form action="/profiles/cancel/${model.queryId}" method="GET">
-        <button type="link" class="btn btn-default">Cancel query</button>
+        <div class="form-group">
+          <button type="submit" class="btn btn-default">Cancel query</button>
+        </div>
       </form>
+        </p>
     </div>
-    <#if model.hasError() >
-    <div id="query-error" class="tab-pane">
-      <p>
-      <pre>
-      ${model.getProfile().error?trim}
-      </pre>
-      </p>
-      <p>Failure node: ${model.getProfile().errorNode}</p>
-      <p>Error ID: ${model.getProfile().errorId}</p>
+    <#if model.hasError()>
+      <div id="query-error" class="tab-pane fade">
+        <p><pre>${model.getProfile().error?trim}</pre></p>
+        <p>Failure node: ${model.getProfile().errorNode}</p>
+        <p>Error ID: ${model.getProfile().errorId}</p>
 
-        <h3 class="panel-title">
-          <a data-toggle="collapse" href="#error-verbose">
-            Verbose Error Message...
-          </a>
-        </h3>
-      <div id="error-verbose" class="panel-collapse collapse">
-        <div class="panel-body">
-        <p></p><p></p>
-          <pre>
-            ${model.getProfile().verboseError?trim}
-          </pre>
+        <div class="page-header"></div>
+        <h3>Verbose Error Message</h3>
+        <div class="panel panel-default">
+          <div class="panel-heading">
+            <h4 class="panel-title">
+              <a data-toggle="collapse" href="#error-message-overview">
+                 Overview
+              </a>
+            </h4>
+          </div>
+          <div id="error-message-overview" class="panel-collapse collapse">
+            <div class="panel-body">
+              <pre>${model.getProfile().verboseError?trim}</pre>
+            </div>
+          </div>
         </div>
-    </div>
+      </div>
     </#if>
+
   </div>
 
   <div class="page-header"></div>
   <h3>Query Profile</h3>
-  <p>STATE: ${model.getProfile().getState().name()}</p>
-  <p>FOREMAN: ${model.getProfile().getForeman().getAddress()}</p>
-  <p>TOTAL FRAGMENTS: ${model.getProfile().getTotalFragments()}</p>
-  <p>DURATION: ${model.getProfileDuration()}</p>
-  <p style="text-indent:5em;">PLANNING: ${model.getPlanningDuration()}</p>
-  <p style="text-indent:5em;">QUEUED: ${model.getQueuedDuration()}</p>
-  <p style="text-indent:5em;">EXECUTION: ${model.getExecutionDuration()}</p>
+  <div class="panel-group" id="query-profile-accordion">
+    <div class="panel panel-default">
+      <div class="panel-heading">
+        <h4 class="panel-title">
+          <a data-toggle="collapse" href="#query-profile-overview">
+              Overview
+          </a>
+        </h4>
+      </div>
+      <div id="query-profile-overview" class="panel-collapse collapse in">
+        <div class="panel-body">
+          <table class="table table-bordered">
+            <thead>
+            <tr>
+                <th>State</th>
+                <th>Foreman</th>
+                <th>Total Fragments</th>
+            </tr>
+            </thead>
+            <tbody>
+              <tr>
+                  <td>${model.getProfile().getState().name()}</td>
+                  <td>${model.getProfile().getForeman().getAddress()}</td>
+                  <td>${model.getProfile().getTotalFragments()}</td>
+              </tr>
+            </tbody>
+          </table>
+        </div>
+      </div>
+    </div>
+    <div class="panel panel-default">
+      <div class="panel-heading">
+        <h4 class="panel-title">
+          <a data-toggle="collapse" href="#query-profile-duration">
+             Duration
+          </a>
+        </h4>
+      </div>
+      <div id="query-profile-duration" class="panel-collapse collapse">
+        <div class="panel-body">
+          <table class="table table-bordered">
+            <thead>
+              <tr>
+                <th>Planning</th>
+                <th>Queued</th>
+                <th>Execution</th>
+                <th>Total</th>
+              </tr>
+            </thead>
+            <tbody>
+              <tr>
+                <td>${model.getPlanningDuration()}</td>
+                <td>${model.getQueuedDuration()}</td>
+                <td>${model.getExecutionDuration()}</td>
+                <td>${model.getProfileDuration()}</td>
+              </tr>
+            </tbody>
+          </table>
+        </div>
+      </div>
+    </div>
+  </div>
 
   <#assign options = model.getOptions()>
   <#if (options?keys?size > 0)>
@@ -162,8 +226,7 @@
       </div>
       <div id="fragment-overview" class="panel-collapse collapse">
         <div class="panel-body">
-          <svg id="fragment-overview-canvas" class="center-block"></svg>
-          ${model.getFragmentsOverview()}
+          ${model.getFragmentsOverview()?no_esc}
         </div>
       </div>
     </div>
@@ -178,7 +241,7 @@
       </div>
       <div id="${frag.getId()}" class="panel-collapse collapse">
         <div class="panel-body">
-          ${frag.getContent()}
+          ${frag.getContent()?no_esc}
         </div>
       </div>
     </div>
@@ -199,7 +262,7 @@
       </div>
       <div id="operator-overview" class="panel-collapse collapse">
         <div class="panel-body">
-          ${model.getOperatorsOverview()}
+          ${model.getOperatorsOverview()?no_esc}
         </div>
       </div>
     </div>
@@ -215,7 +278,7 @@
       </div>
       <div id="${op.getId()}" class="panel-collapse collapse">
         <div class="panel-body">
-          ${op.getContent()}
+          ${op.getContent()?no_esc}
         </div>
         <div class="panel panel-info">
           <div class="panel-heading">
@@ -227,7 +290,7 @@
           </div>
           <div id="${op.getId()}-metrics" class="panel-collapse collapse">
             <div class="panel-body" style="display:block;overflow-x:auto">
-              ${op.getMetricsTable()}
+              ${op.getMetricsTable()?no_esc}
             </div>
           </div>
         </div>

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/exec/java-exec/src/main/resources/rest/query/result.ftl
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/resources/rest/query/result.ftl 
b/exec/java-exec/src/main/resources/rest/query/result.ftl
index 8382e4f..3e1f49f 100644
--- a/exec/java-exec/src/main/resources/rest/query/result.ftl
+++ b/exec/java-exec/src/main/resources/rest/query/result.ftl
@@ -31,7 +31,7 @@
       <thead>
         <tr>
           <#list model.getColumns() as value>
-          <th>${value?html}</th>
+          <th>${value}</th>
           </#list>
         </tr>
       </thead>
@@ -39,7 +39,7 @@
       <#list model.getRows() as record>
         <tr>
           <#list record as value>
-          <td>${value!"null"?html}</td>
+          <td>${value!"null"}</td>
           </#list>
         </tr>
       </#list>

http://git-wip-us.apache.org/repos/asf/drill/blob/c7c8ffd6/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 6bd5b66..a843960 100644
--- a/pom.xml
+++ b/pom.xml
@@ -52,7 +52,7 @@
     <hadoop.version>2.7.1</hadoop.version>
     <hbase.version>1.1.3</hbase.version>
     <fmpp.version>0.9.15</fmpp.version>
-    <freemarker.version>2.3.21</freemarker.version>
+    <freemarker.version>2.3.26-incubating</freemarker.version>
   </properties>
 
   <scm>

Reply via email to