You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Zsombor Gegesy <zs...@apache.org> on 2018/03/11 20:58:47 UTC

Review Request 66024: RANGER-1991 - fix

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

Review request for ranger.


Bugs: RANGER-1991
    https://issues.apache.org/jira/browse/RANGER-1991


Repository: ranger


Description
-------

There is a code path, where a null is passed as a HttpServletRequest - to trigger 'searching with empty filter'.
A simple fix is to introduce a getServicePolicies(serviceName,SearchFilter) method, so the HttpServletRequest is not needed anymore.


Diffs
-----

  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 229863e7 


Diff: https://reviews.apache.org/r/66024/diff/1/


Testing
-------

Tested locally


Thanks,

Zsombor Gegesy


Re: Review Request 66024: RANGER-2016 - fix the code cleanup

Posted by Qiang Zhang <zh...@zte.com.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66024/#review199942
-----------------------------------------------------------


Ship it!




Ship It!

- Qiang Zhang


On March 24, 2018, 12:18 p.m., Zsombor Gegesy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66024/
> -----------------------------------------------------------
> 
> (Updated March 24, 2018, 12:18 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-2016
>     https://issues.apache.org/jira/browse/RANGER-2016
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> There is a code path, where a null is passed as a HttpServletRequest - to trigger 'searching with empty filter'.
> A simple fix is to introduce a getServicePolicies(serviceName,SearchFilter) method on ServiceREST class, so the HttpServletRequest is not needed anymore, and this method can be called freely.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java c2783dcf6 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3642252f1 
> 
> 
> Diff: https://reviews.apache.org/r/66024/diff/3/
> 
> 
> Testing
> -------
> 
> Tested locally
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>


Re: Review Request 66024: RANGER-2016 - fix the code cleanup

Posted by Zsombor Gegesy <zs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66024/
-----------------------------------------------------------

(Updated March 24, 2018, 12:18 p.m.)


Review request for ranger.


Changes
-------

rebased changes


Bugs: RANGER-2016
    https://issues.apache.org/jira/browse/RANGER-2016


Repository: ranger


Description
-------

There is a code path, where a null is passed as a HttpServletRequest - to trigger 'searching with empty filter'.
A simple fix is to introduce a getServicePolicies(serviceName,SearchFilter) method on ServiceREST class, so the HttpServletRequest is not needed anymore, and this method can be called freely.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java c2783dcf6 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 3642252f1 


Diff: https://reviews.apache.org/r/66024/diff/3/

Changes: https://reviews.apache.org/r/66024/diff/2-3/


Testing
-------

Tested locally


Thanks,

Zsombor Gegesy


Re: Review Request 66024: RANGER-2016 - fix the code cleanup

Posted by Velmurugan Periasamy <vp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66024/#review199919
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java
Line 40 (original), 41 (patched)
<https://reviews.apache.org/r/66024/#comment280429>

    $ git apply --check -v ~/Downloads/RANGER-2016\ (1).patch
    Checking patch security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java...
    Hunk #1 succeeded at 33 (offset 1 line).
    error: while searching for:
    	final static Logger logger = Logger.getLogger(RangerSearchUtil.class);
    
    	public SearchFilter getSearchFilter(HttpServletRequest request, List<SortField> sortFields) {
    		if (request == null) {
    			return null;
    		}
    		SearchFilter ret = new SearchFilter();
    
    		if (MapUtils.isEmpty(request.getParameterMap())) {
    
    This patch fails to apply
    
    error: patch failed: security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java:41
    error: security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java: patch does not apply
    Checking patch security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java...
    Hunk #1 succeeded at 2089 (offset 21 lines).
    Hunk #2 succeeded at 2303 (offset 21 lines).
    Hunk #3 succeeded at 2533 (offset 22 lines).
    Hunk #4 succeeded at 2555 (offset 22 lines).
    Hunk #5 succeeded at 2575 (offset 22 lines).
    Hunk #6 succeeded at 2586 (offset 22 lines).


- Velmurugan Periasamy


On March 15, 2018, 8:12 a.m., Zsombor Gegesy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66024/
> -----------------------------------------------------------
> 
> (Updated March 15, 2018, 8:12 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-2016
>     https://issues.apache.org/jira/browse/RANGER-2016
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> There is a code path, where a null is passed as a HttpServletRequest - to trigger 'searching with empty filter'.
> A simple fix is to introduce a getServicePolicies(serviceName,SearchFilter) method on ServiceREST class, so the HttpServletRequest is not needed anymore, and this method can be called freely.
> 
> 
> Diffs
> -----
> 
>   security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java 86b4e4309 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 229863e74 
> 
> 
> Diff: https://reviews.apache.org/r/66024/diff/2/
> 
> 
> Testing
> -------
> 
> Tested locally
> 
> 
> Thanks,
> 
> Zsombor Gegesy
> 
>


Re: Review Request 66024: RANGER-2016 - fix the code cleanup

Posted by Zsombor Gegesy <zs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66024/
-----------------------------------------------------------

(Updated March 15, 2018, 8:12 a.m.)


Review request for ranger.


Summary (updated)
-----------------

RANGER-2016 - fix the code cleanup


Bugs: RANGER-2016
    https://issues.apache.org/jira/browse/RANGER-2016


Repository: ranger


Description (updated)
-------

There is a code path, where a null is passed as a HttpServletRequest - to trigger 'searching with empty filter'.
A simple fix is to introduce a getServicePolicies(serviceName,SearchFilter) method on ServiceREST class, so the HttpServletRequest is not needed anymore, and this method can be called freely.


Diffs (updated)
-----

  security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java 86b4e4309 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 229863e74 


Diff: https://reviews.apache.org/r/66024/diff/2/

Changes: https://reviews.apache.org/r/66024/diff/1-2/


Testing
-------

Tested locally


Thanks,

Zsombor Gegesy