Github user benchristel commented on a diff in the pull request: https://github.com/apache/incubator-hawq/pull/1379#discussion_r200797423 --- Diff: pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/servlet/SecurityServletFilter.java --- @@ -64,21 +75,28 @@ public void init(FilterConfig filterConfig) throws ServletException { * @param chain filter chain */ @Override - public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { + public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) + throws IOException, ServletException { if (SecureLogin.isUserImpersonationEnabled()) { // retrieve user header and make sure header is present and is not empty - final String user = ((HttpServletRequest) request).getHeader(USER_HEADER); - if (user == null) { - throw new IllegalArgumentException(MISSING_HEADER_ERROR); - } else if (user.trim().isEmpty()) { - throw new IllegalArgumentException(EMPTY_HEADER_ERROR); + final String user = getHeaderValue(request, USER_HEADER); + String transactionId = getHeaderValue(request, TRANSACTION_ID_HEADER); --- End diff -- We are not passing `required=true` to `getHeaderValue()` hereâis it really valid for `transactionId` to be null? If `transactionId` can be null, that means we're going to create `SegmentTransactionId`s with nothing but a segment ID in them, and we'll potentially get the wrong UGI from the cache if another request with no `transactionId` was recently served.
---