Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

6 Win32 function stubs, 8 constants, 1 structure, and 1 helper function #535

Closed
wants to merge 9 commits into from

Conversation

mlfreeman2
Copy link
Contributor

  • Advapi32
    • Added LOGON_WITH_PROFILE constant
    • Added LOGON_NETCREDENTIALS_ONLY constant
    • Added CreateProcessWithLogonW(String, String, String, int, String, String, int, Pointer, String, STARTUPINFO, PROCESS_INFORMATION)
      • No Unit Test - I would have to make it create a user to be 100% able to run a process as another user and that would be a security issue I figure.
  • Crypt32
    • Added CertAddEncodedCertificateToSystemStore(String, Pointer, DWORD)
      • No Unit Test - I doubt anyone would want the security risk of a unit test installing a root certificate.
  • GDI32
    • Added SRCCOPY constant
    • Added BitBlt(HDC, int, int, int, int, HDC, int, int, int)
      • No direct unit test - the test for GDI32Util.getScreenshot() seemed to cover it just fine.
    • Added GDI32Util.getScreenshot(HWND)
      • Added unit test as GDI32UtilTest.testGetScreenshot()
  • Shell32
    • Added SHERB_NOCONFIRMATION constant
    • Added SHERB_NOPROGRESSUI constant
    • Added SHERB_NOSOUND constant
    • Added SEE_MASK_NOCLOSEPROCESS constant
    • Added SHEmptyRecycleBin(HANDLE, String, int)
      • No unit test - no idea how to tell if the recycle bin is empty afterwards
    • Added ShellExecuteEx(SHELLEXECUTEINFO)
      • No unit test - there are a bunch of cases where the hProcess member in SHELLEXECUTEINFO isn't set - not sure how to control for that
  • ShellAPI
    • Added SHELLEXECUTEINFO structure
  • User32
    • Added GetDesktopWindow()
      • Added test as User32Test.testGetDesktopWindow()
  • WinGDI
    • Added HGDI_ERROR
    • Removed superfluous "public" and "public final" from WinGDI

Hopefully this is more manageable and more acceptable than my last way-too-large pull request

	* Added LOGON_WITH_PROFILE constant
	* Added LOGON_NETCREDENTIALS_ONLY constant
	* Added CreateProcessWithLogonW(String, String, String, int, String, String, int, Pointer, String, STARTUPINFO, PROCESS_INFORMATION)
		* No Unit Test - I would have to make it create a user to be 100% able to run a process as another user and that would be a security issue I figure.
* Crypt32
	* Added CertAddEncodedCertificateToSystemStore(String, Pointer, DWORD)
		* No Unit Test - I doubt anyone would want the security risk of a unit test installing a root certificate.
* GDI32
	* Added SRCCOPY constant
	* Added BitBlt(HDC, int, int, int, int, HDC, int, int, int)
		* No direct unit test - the test for GDI32Util.getScreenshot() seemed to cover it just fine.
	* Added GDI32Util.getScreenshot(HWND)
		* Added unit test as GDI32UtilTest.testGetScreenshot()
* Shell32
	* Added SHERB_NOCONFIRMATION constant
	* Added SHERB_NOPROGRESSUI constant
	* Added SHERB_NOSOUND constant
	* Added SEE_MASK_NOCLOSEPROCESS constant
	* Added SHEmptyRecycleBin(HANDLE, String, int)
		* No unit test - no idea how to tell if the recycle bin is empty afterwards
	* Added ShellExecuteEx(SHELLEXECUTEINFO)
		* No unit test - there are a bunch of cases where the hProcess member in SHELLEXECUTEINFO isn't set - not sure how to control for that
* ShellAPI
	* Added SHELLEXECUTEINFO structure
* User32
	* Added GetDesktopWindow()
		* Added test as User32Test.testGetDesktopWindow()
* WinGDI
	* Added HGDI_ERROR
	* Removed superfluous "public" and "public final" from WinGDI
@mlfreeman2
Copy link
Contributor Author

I did try to incorporate all the feedback that @lgoldstein and @dblock gave me on the last (now closed) pull request.

@mlfreeman2
Copy link
Contributor Author

Added negative unit tests for CreateProcessWithLogonW and CertAddEncodedCertificateToSystemStore

I figure those will be secure since they're meant to fail with specific windows error codes.

@mlfreeman2
Copy link
Contributor Author

Got creative and managed to figure out what I think might be reasonable negative tests for the remaining functions in this PR.
In theory every function added in this PR has some sort of test associated with it now.

* For extended error information, call GetLastError.
* @see MSDN {@link http://msdn.microsoft.com/en-us/library/bb736347(v=vs.85).aspx }
*/
boolean CertAddEncodedCertificateToSystemStore(String szCertStoreName, Pointer pbCertEncoded, DWORD cbCertEncoded);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can usually safely use int instead of DWORD here (as well as for return values). I agree that DWORD and int are not entirely equivalent, but for most purposes the sign is not important - e.g., error codes or sizes (as in this case).


// should fail with "the user name or password is incorrect" (error 1326)
assertEquals("GetLastError() should have returned ERROR_LOGON_FAILURE because the username was bogus.", Native.getLastError(), W32Errors.ERROR_LOGON_FAILURE);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverse the order of the arguments to assertEquals - the expected comes first (in this case W32Errors.ERROR_LOGON_FAILURE should be first and getLastError() second).

}

public void testShellExecuteEx() {
File file = new File(System.getProperty("java.io.tmpdir") + System.nanoTime() + ".txt");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment regarding backslash

@lgoldstein
Copy link
Contributor

Seems in order - just add a line in the CHANGES.md file under the Next release / Features section using the same format as the others, and we will be able to merge your pull request.

P.S. You can use the git commit --amend --no-edit option:

  • Modify CHANGES.md
  • git add CHANGES.md
  • git commit --amend --no-edit
  • git push -v --progress -f ...your-fork... ...branch-name:branch-name...

@dblock
Copy link
Member

dblock commented Nov 16, 2015

I merged via db86ae6.

We can iterate on top for anything else that's left. Please try to make small, individual PRs. Thanks for contributing! Thanks for a thorough CR @lgoldstein.

@dblock dblock closed this Nov 16, 2015
@dblock
Copy link
Member

dblock commented Nov 16, 2015

@mlfreeman2 You have been working on JNA master on your fork, you should read http://code.dblock.org/2015/08/31/getting-out-of-your-first-git-mess.html.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants