[jira] [Commented] (HADOOP-9637) TestAggregatedLogFormat and TestContainerLogsPage fail on Windows

["Chris Nauroth (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-9637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13685751#comment-13685751 ] 

Chris Nauroth commented on HADOOP-9637:
---------------------------------------

Nice work, Chuan!  Here are a couple of questions:

{code}
        if (Shell.WINDOWS) {
          int i = owner.indexOf('\\');
          if (i != -1)
            this.owner = owner.substring(i + 1);
        }
{code}

If I understand correctly, this is intended to remove the domain portion from a username.  (Am I understanding it correctly?)  Does Windows ever prepend domain to the group name too?  Do we need similar logic for initializing {{this.group}}?

It seems that this logic would treat user DOMAIN1\cnauroth the same as DOMAIN2\cnauroth, when in reality the same username in 2 different domains might not be the same person.  Consumers of fstat would end up seeing "cnauroth" as the owner without being able to differentiate between the 2 different users in 2 different domains.  Is this acceptable?

{code}
  stat_ctor2 = (*env)->GetMethodID(env, stat_clazz, "<init>",
	  "(Ljava/lang/String;Ljava/lang/String;I)V");
  if (!stat_ctor2) {
	  return; // exception has been raised
  }
{code}

Minor nit: use 2-space indentation in the above code.

{code}
  dwRtnCode = GetFinalPathNameByHandle(fileHandle, path, cchPathLen, 0);
  if (dwRtnCode == 0)
  {
    ret = GetLastError();
    goto FindFileOwnerAndPermissionByHandleEnd;
  }
  cchPathLen = dwRtnCode;
  path = (LPWSTR) LocalAlloc(LPTR, cchPathLen * sizeof(WCHAR));
  if (path == NULL)
  {
    ret = GetLastError();
    goto FindFileOwnerAndPermissionByHandleEnd;
  }

  dwRtnCode = GetFinalPathNameByHandle(fileHandle, path, cchPathLen, 0);
  if (dwRtnCode != cchPathLen - 1)
  {
    ret = GetLastError();
    goto FindFileOwnerAndPermissionByHandleEnd;
  }
{code}

I read up on the MSDN docs for {{GetFinalPathNameByHandle}} here:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa364962(v=vs.85).aspx

The first call returns the required buffer length, including null terminator:

{quote}
If the function fails because lpszFilePath is too small to hold the string plus the terminating null character, the return value is the required buffer size, in TCHARs. This value includes the size of the terminating null character.
{quote}

However, the value of {{cchFilePath}} passed to the function in the second call is expected to exclude the null terminator:

{quote}
cchFilePath [in]
The size of lpszFilePath, in TCHARs. This value does not include a NULL termination character.
{quote}

Does this mean that we need to subtract 1 from {{cchPathLen}} earlier, such as when we assign {{cchPathLen = dwRtnCode}}?

                
> TestAggregatedLogFormat and TestContainerLogsPage fail on Windows
> -----------------------------------------------------------------
>
>                 Key: HADOOP-9637
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9637
>             Project: Hadoop Common
>          Issue Type: Bug
>    Affects Versions: 3.0.0, 2.1.0-beta
>            Reporter: Chuan Liu
>            Assignee: Chuan Liu
>         Attachments: HADOOP-9637-trunk.patch
>
>
> TestAggregatedLogFormat.testContainerLogsFileAccess test case fails on Windows. The test case try to simulate a situation where first log file is owned by different user (probably symlink) and second one by the user itself. In this situation, the attempt to try to aggregate the logs should fail with the error message "Owner ... for path ... did not match expected owner ...".
> The check on file owner happens at {{AggregatedLogFormat.write()}} method. The method calls {{SecureIOUtils.openForRead()}} to read the log files before writing out to the OutputStream.
> {{SecureIOUtils.openForRead()}} use {{NativeIO.Posix.getFstat()}} to get the file owner and group. We don't have {{NativeIO.Posix.getFstat()}} implementation on Windows; thus, the failure.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira