sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 28434: SQOOP-1795: Sqoop2: Retrieve Http post data in plausible manner
Date Tue, 25 Nov 2014 16:52:39 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28434/#review62997
-----------------------------------------------------------


Thank you for jumping in Stanley!


server/src/main/java/org/apache/sqoop/server/RequestContext.java
<https://reviews.apache.org/r/28434/#comment105168>

    Do you think that it would make sense to make this method private, so that users of this
class don't have the option to call the getReader() themselves and have to go through methods
of this class (wrapper) to obtain all they need?



server/src/main/java/org/apache/sqoop/server/RequestContext.java
<https://reviews.apache.org/r/28434/#comment105173>

    I'm wondering whether this method would return the JSON correctly as expected if I would
send POST request as this one:
    
    POST /page?argument=value HTTP/1.0 
    
    {"json":"value"}
    
    E.g. in this case I would expect that the getParametersMap will return Map of size two
and then it will depend on the Map ordering whether we get the correct one or not. Let's perhaps
add unit tests to ensure that we have the behaviour that we're expecting?


- Jarek Cecho


On Nov. 25, 2014, 8:34 a.m., Qian Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28434/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2014, 8:34 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1795
>     https://issues.apache.org/jira/browse/SQOOP-1795
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> 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.
> 
> Expectedly:
> 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. 
> 
> The patch proposes to add `RequestContext.getFirstParameterName()`. so that we can keep
fingers away from `getReader()`.
> 
> 
> Diffs
> -----
> 
>   server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 75a069a227b04e045b76c09e721deb45a35cb9eb

>   server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java d2d080bbe2c2f55feb349089b93da7f41bc13bc6

>   server/src/main/java/org/apache/sqoop/server/RequestContext.java d0963f5bcd09a5f31601f677304165d513cc7de3

> 
> Diff: https://reviews.apache.org/r/28434/diff/
> 
> 
> Testing
> -------
> 
> All passed. (Nits: adding unit tests)
> 
> 
> Thanks,
> 
> Qian Xu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message