Fix NSInvalidArgumentException when querying connected_displays
#8628
Conversation
|
Nice, thank you.
I'm guessing you have a use case here. What would be best for you?
Not that I can think of. Generally we try to do some manual testing against a few different configurations and then make sure it at least returns some sort of sane result in the automated testing/on CI. |
I don't have a specific use case for the rotation value, I'm just looking to query these tables without That said, I think that having the degree rotation ( |
|
abadaf7 should resolve the failing test, by returning 82: [----------] 1 test from connectedDisplays
82: [ RUN ] connectedDisplays.test_sanity
82: 2025-06-18 03:16:20.743 system_profiler[8992:43550] Device PreExisted [00000001000002f4] Apple Paravirtual device
82: /Users/runner/work/osquery/osquery/workspace/src/tests/integration/tables/helper.cpp:167: Failure
82: Value of: validate_value_using_flags(value, flags)
82: Actual: false
82: Expected: true
82: Standard validator of the column "rotation" with value "" failed
82: Row: {ambient_brightness_enabled: "-1", connection_type: "spdisplays_internal", display_id: "1800000", display_type: "", main: "1", manufactured_week: "26", manufactured_year: "2020", mirror: "0", name: "Unknown Display", online: "1", pixels: "1920 x 1080", product_id: "0", resolution: "1920 x 1080 @ 30.00Hz", rotation: "", serial_number: "1", vendor_id: "1006"}
82:
82: [ FAILED ] connectedDisplays.test_sanity (527 ms)
82: [----------] 1 test from connectedDisplays (527 ms total)5f0b8ad updates the docs for the |
| } else { | ||
| r["rotation"] = INTEGER(0); | ||
| // rotation is not supported | ||
| r["rotation"] = INTEGER(-1); | ||
| } |
There was a problem hiding this comment.
This is arguably a breaking change as rotation used to be 0 for not supported and now it is -1. 🤔 However I don't think these tables (and specifically this column) are widely used as osqueryd crashes if this table is queried while a display is rotated.
Alternatively we could just continue to return 1 for rotation supported and 0 for not supported to avoid breaking the current behavior.
There was a problem hiding this comment.
My hunch is that this is fine to change
|
I did some testing with the build from this PR and it's working as intended. 🎉 🚀
|
This PR resolves an exception when querying
connected_displaysif one or more displays are rotated.The content of
spdisplays_rotationis"spdisplays_supported"when rotation is supported but the display is not rotated (i.e., 0 degree rotation) and either90,180, or270when the display is rotated. An example of both cases is available here.🐛 The bug
When querying
connected_displaysif any display is rotated auncaught exception 'NSInvalidArgumentException', reason: '-[__NSCFNumber isEqualToString:]: unrecognized selector sent to instanceis raised because of theisEqualToStringcheck here.The documentation for the
rotationfield says it should be the rotation of the monitor but it's currently implemented as1when rotation is supported and0when it isn't supported.The implementation proposed here changes that to be
0for supported but not rotated, 90/180/270 if rotated, otherwise null. The table documentation should also be updated if this moves forward.💭 Questions
rotationbe the degree rotation of the monitor, or should it indicate whether rotation is (or is not) supported?📚 References
connected_displaystable on macOS #7946video_infotable for macOS #7486 (comment)