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

Added Advapi32Util accessCheck method to verify file permissions #301

Merged
merged 4 commits into from
Jan 22, 2014
Merged

Conversation

BusyByte
Copy link
Contributor

I had contacted you on the Google Group about adding this functionality to check effective file permissions for the current user.
I've added three tests to verify the positive cases.
I couldn't think of an automated way to test the negative cases so I just manually tested those and deleted the tests for those.
Let me know if you have any questions.
Can you message me back on the Google Group when you have Maven Central updated with a new release with these changes?

Shawn

* @param AccessMask [in, out] A pointer to an access mask.
* @param GenericMapping [in] A pointer to a GENERIC_MAPPING structure specifying a mapping of generic access types to specific and standard access types.
*/
public void MapGenericMask(WinDef.DWORDByReference AccessMask, WinNT.GENERIC_MAPPING GenericMapping);
Copy link
Member

Choose a reason for hiding this comment

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

Import the structures used and change things like WinDef.DWORDByRference to just DWORDByReference.

@dblock
Copy link
Member

dblock commented Jan 21, 2014

This is very good. You need to add lower level tests for AccessCheck and MapGenericMask and an update to CHANGELOG, please.

@BusyByte
Copy link
Contributor Author

AccessCheck is very involved so basically I think the higher level test is the lowest level test you can get for this
If you have any ideas on how to do this let me know. Do you use any mocking frameworks?
I can add a test for MapGenericMask and update the CHANGELOG.

@dblock
Copy link
Member

dblock commented Jan 21, 2014

I am happy with whatever test that calls the API and doesn't implode.

@BusyByte
Copy link
Contributor Author

I believe I've address all your concerns. Let me know if there is anything else to look at.

@BusyByte BusyByte closed this Jan 22, 2014
@BusyByte BusyByte reopened this Jan 22, 2014
@@ -18,6 +18,7 @@ Features
* [#250](https://github.com/twall/jna/pull/250): Added `com.sun.jna.platform.win32.Kernel32.GetPrivateProfileSection`, `GetPrivateProfileSectionNames` and `WritePrivateProfileSection` and corresponding `Kernel32Util` helpers - [@quipsy-karg](https://github.com/quipsy-karg).
* [#287](https://github.com/twall/jna/pull/287): Added `DBTF_MEDIA` and `DBTF_NET` to `com.sun.jna.platform.win32.DBT` - [@daifei4321](https://github.com/daifei4321).
* [#295](https://github.com/twall/jna/pull/295): Added `com.sun.jna.platform.win32.Kernel32.ResetEvent` - [@manithree](https://github.com/manithree).
* [#301](https://github.com/twall/jna/pull/301): Added `com.sun.jna.platform.win32.Advapi32Util.accessCheck` and supporting classes/methods to verify file permissions - [@BusyByte] (https://github.com/BusyByte/jna).
Copy link
Member

Choose a reason for hiding this comment

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

There's an extra space after [@BusyByte] that makes it a non-link. You should also add the mappings that you added as a separate line like above.

@dblock
Copy link
Member

dblock commented Jan 22, 2014

I still don't see a test for AccessCheck. But yes, I see a test for the Util method that eventually calls AccessCheck. Just write something simple that, for example, passes an invalid security descriptor and makes sure it gets back ERROR_INVALID_SECURITY_DESCR. We want a low level mapping test basically that makes sure the function signature is correct.

@BusyByte
Copy link
Contributor Author

I get back invalid handle. Take a look and see if there is anything else I need to do.

assertFalse(status);
assertFalse(result.getValue().booleanValue());

assertEquals("The handle is invalid.", Kernel32Util.formatMessage(W32Errors.HRESULT_FROM_WIN32(Kernel32.INSTANCE.GetLastError())));
Copy link
Member

Choose a reason for hiding this comment

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

Last thing, promise. Just check that Kernel32.INSTANCE.GetLastError() is INVALID_HANDLE or something like that. This would fail on a non-US-English system.

@BusyByte
Copy link
Contributor Author

Alright let me know how it looks now.

@dblock
Copy link
Member

dblock commented Jan 22, 2014

Great, merging.

dblock added a commit that referenced this pull request Jan 22, 2014
Added Advapi32Util accessCheck method to verify file permissions
@dblock dblock merged commit 2c0d097 into java-native-access:master Jan 22, 2014
mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024
Bumps logback-classic from 1.1.7 to 1.2.0.

---
updated-dependencies:
- dependency-name: ch.qos.logback:logback-classic
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

2 participants