Check when a HRESULT is a Win32 code and fetch it#9
Open
zeroSteiner wants to merge 1 commit into
Open
Conversation
5 tasks
There was a problem hiding this comment.
Pull request overview
Adds Win32-aware HRESULT lookup so callers can resolve FACILITY_WIN32 HRESULTs (e.g., 0x80070547) to the underlying Win32 error code, and makes it easier to require submodules directly.
Changes:
- Ensure
windows_error/win32andwindows_error/nt_statuscan be required standalone by requiring the basewindows_errordefinitions. - Add
Facility.find_by_h_resultand harden facility constant iteration to only considerFacilityCodeinstances. - Teach
HResult.find_by_retvalto detectFACILITY_WIN32and delegate toWin32.find_by_retval(retval & 0xffff).
Impact Analysis:
- Blast radius: medium; affects consumers of
WindowsError::HResult.find_by_retvaland anyone requiringwindows_error/h_result, plus standalone require paths forwindows_error/win32andwindows_error/nt_status. - Data and contract effects:
HResult.find_by_retvalmay now return Win32WindowsError::ErrorCodeinstances (notHResultCode) forFACILITY_WIN32HRESULTs, which changes the effective return-type contract for that method. - Rollback and test focus: rollback is straightforward (revert), but validation should focus on
FACILITY_WIN32mapping cases (e.g.,0x80070547 -> ERROR_CANT_ACCESS_DOMAIN_INFO) and confirming non-WIN32 HRESULT lookups remain unchanged.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/windows_error/win32.rb | Require base library so Win32 constants can be loaded via standalone require. |
| lib/windows_error/nt_status.rb | Require base library so NTSTATUS constants can be loaded via standalone require. |
| lib/windows_error/h_result/facility.rb | Add find_by_h_result and skip non-FacilityCode constants during lookup. |
| lib/windows_error/h_result.rb | Require Win32 and map FACILITY_WIN32 HRESULTs to Win32 error codes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -15,6 +17,11 @@ module HResult | |||
| # @return [Array<HResultCode>] all Win32 ErrorCodes that matched | |||
Comment on lines
+21
to
+23
| if Facility.find_by_h_result(retval) == Facility::FACILITY_WIN32.value | ||
| return Win32.find_by_retval(retval & 0xffff) | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes an error that was privately reported to me where Metasploit's
icpr_certmodule was not fetching an error code and then crashing with a stack trace because the HRESULT definition didn't exist. The code in question was 0x80070547 which should map to the win32 ERROR_CANT_ACCESS_DOMAIN_INFO code.There's also some extra require statements, so consumers can just
require 'windows_error/h_result'.