[
https://issues.apache.org/jira/browse/SQOOP-1795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14224852#comment-14224852
]
Veena Basavaraj edited comment on SQOOP-1795 at 11/25/14 5:31 PM:
------------------------------------------------------------------
[~stanleyxu2005], your proposal on not using getReader() is fine, but we need
to throw an exception or some information when someone uses it.
How do you intend to accomplish that.?
Since Request context is the wrapper that is holding both request and
response! ( which is another bad naming, should be renamed to something that
reflect both are split up to only encapsulate one thing.
{code}
private final HttpServletRequest request;
private final HttpServletResponse response;
{code}
For a reason the HttpServletRequest and HttpServletResponse and not one class
that holds the both.There should have been Response context) or more aptly a
RequestWrapper and ResponseWrapper.
it may hard to enforce it, but try adding a getReader method to the
RequestContext and throw an exception when someone accesses it.
There is no easy way to access the HttpServletRequest directly inside the
various request handlers. The {code}SqoopProtocolServlet{code} still can do it,
so this is one place we cannot guard if some did a getReader from the request
directly. Another place, is if we ever did add filters in future, someone can
easily have access to the request and do a getReader () and we will be
wondering where is the postData :)
Alternatively if we did inheritance that extended HttpServletRequest, we can
override getReader in our code and throw an exception with the right message
(Unsupported) or even better delegate to use the getPostData that we will add.
Second.
{quote}
For raw post data usage, the post data is parsed as the first parameter's key.
So I'd suggest to add RequestContext.getFirstParameterName(). so that we can
keep fingers away from getReader().
{quote}
We can actually call it getPostData()m instead of getFirstParameterName
Third,
Also I have seen many cases using this pattern..the API is not solid
anyways,,,so I am + 1 for changing this to your proposal.
{quote}
In practice, it'd suggest to use query string base pattern, i.e.
jsonObject=JSON. The server can call ctx.getParameterValue("jsonObject") to get
the value without any problem. But we need to change the api.
{quote}
was (Author: vybs):
[~stanleyxu2005], your proposal on not using getReader() is fine, but we need
to throw an exception or some information when someone uses it.
How do you intend to accomplish that.?
Since Request context is the wrapper that is holding both request and
response! ( which is another bad naming, should be renamed to something that
reflect both are split up to only encapsulate one thing.
{code}
private final HttpServletRequest request;
private final HttpServletResponse response;
{code}
For a reason the HttpServletRequest and HttpServletResponse and not one class
that holds the both.There should have been Response context) or more aptly a
RequestWrapper and ResponseWrapper.
it may hard to enforce it, but try adding a getReader method to the
RequestContext and throw an exception when someone accesses it.
There is no easy way to access the HttpServletRequest directly inside the
various request handlers. The {code}SqoopProtocolServlet{code} still can do it,
so this is one place we cannot guard if some did a getReader from the request
directly. Another place, is if we ever did add filters in future, someone can
easily have access to the request and do a getReader () and we will be
wondering where is the postData :)
Alternatively if we did inheritance that extended HttpServletRequest, we can
override getReader in our code and throw an exception with the right message
(Unsupported) or even better delegate to use the getPostData that we will add.
Second.
For raw post data usage, the post data is parsed as the first parameter's key.
So I'd suggest to add RequestContext.getFirstParameterName(). so that we can
keep fingers away from getReader().
We can actually call it getPostData()m instead of getFirstParameterName
Third,
Also I have seen many cases using this pattern..the API is not solid
anyways,,,so I am + 1 for changing this to your proposal.
{quote}
In practice, it'd suggest to use query string base pattern, i.e.
jsonObject=JSON. The server can call ctx.getParameterValue("jsonObject") to get
the value without any problem. But we need to change the api.
{quote}
> Sqoop2: Retrieve Http post data in plausible manner
> ---------------------------------------------------
>
> Key: SQOOP-1795
> URL: https://issues.apache.org/jira/browse/SQOOP-1795
> Project: Sqoop
> Issue Type: Sub-task
> Reporter: Qian Xu
> Assignee: Qian Xu
> Fix For: 1.99.5
>
>
> Sqoop client posts parameters as JSON to server. As it is not query string
> based pattern, {{HttpServletRequest}} is not able to return the JSON using
> {{getParameterValue(...)}}. The current solution is to call {{getReader()}}
> to get raw post data. There is a danger, if the method is NOT called at the
> first place. You do not know, whether the reading position is at the
> beginning. You might get unexpected result without notice. SQOOP-1784 is a
> unfortunate case.
> {code}
> // 2 line unlucky code
> String username = ctx.getUserName(); // The method will change reading
> position of reader internally
> JSONObject postData = (JSONObject)
> JSONValue.parse(ctx.getRequest().getReader()); // No contents read
> // 2 line lucky code
> JSONObject postData = (JSONObject)
> JSONValue.parse(ctx.getRequest().getReader()); // Good
> String username = ctx.getUserName(); // Good
> {code}
> In practice, it'd suggest to use query string base pattern, i.e.
> jsonObject=JSON. The server can call {{ctx.getParameterValue("jsonObject")}}
> to get the value without any problem. But we need to change the api.
> Now it is perfectly clear:
> 1. For query string based pattern, caller should always use
> {{getParameterValue(...)}}.
> 2. For raw post data usage, the post data is parsed as the first parameter's
> key. So I'd suggest to add {{RequestContext.getFirstParameterName()}}. so
> that we can keep fingers away from {{getReader()}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
