luocooong commented on a change in pull request #2225:
URL: https://github.com/apache/drill/pull/2225#discussion_r642077253
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServerConstants.java
##########
@@ -45,4 +45,7 @@ private WebServerConstants() {}
// Name of the CSRF protection token attribute
public static final String CSRF_TOKEN = "csrfToken";
+
+ // Key of the profile data to view
+ public static final String PROFILE_DATA = "profile_data";
Review comment:
Good define.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebUtils.java
##########
@@ -55,7 +55,10 @@
public static String getCsrfTokenFromHttpRequest(HttpServletRequest request)
{
// No need to create a session if not present (i.e. if a user is logged in)
HttpSession session = request.getSession(false);
- return session == null ? "" : (String)
session.getAttribute(WebServerConstants.CSRF_TOKEN);
+ String res = session == null ? "" :
+ (String) session.getAttribute(WebServerConstants.CSRF_TOKEN);
+ // In case session is created when authentication is disabled
+ return res == null ? "" : res;
Review comment:
What happens without these changes? Is it possible to revert these if no
effect for anything?
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
##########
@@ -373,9 +379,18 @@ private QueryProfile getQueryProfile(String queryId) {
@GET
@Path("/profiles/{queryid}.json")
@Produces(MediaType.APPLICATION_JSON)
- public String getProfileJSON(@PathParam("queryid") String queryId) {
+ public String getProfileJSON(@PathParam("queryid") String queryId,
+ @Context HttpServletRequest req) {
try {
- return new
String(work.getContext().getProfileStoreContext().getProfileStoreConfig().getSerializer().serialize(getQueryProfile(queryId)));
+ HttpSession session = req.getSession(false);
+ if (session == null || session.getAttribute(PROFILE_DATA) == null) {
Review comment:
I think this session is not null here, because the `Cookie: JSESSIONID`
is not null at this time. I recommend that :
```java
if(req.getSession().getAttribute(PROFILE_DATA) == null) { }
```
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
##########
@@ -395,6 +410,33 @@ public Viewable getProfile(@PathParam("queryid") String
queryId){
}
}
+ @POST
+ @Path("/profiles/view")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.TEXT_HTML)
+ public Viewable viewProfile(@FormDataParam(PROFILE_DATA) String content,
+ @Context HttpServletRequest req){
+ try {
+ HttpSession session = req.getSession(true);
+ if (session.isNew()) {
+ session.setMaxInactiveInterval(work.getContext().getConfig()
+ .getInt(ExecConstants.HTTP_SESSION_MAX_IDLE_SECS));
+ }
+ session.setAttribute(PROFILE_DATA, content);
Review comment:
As the above said, I recommend that :
```java
req.getSession().setAttribute(PROFILE_DATA, content)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
