-
Notifications
You must be signed in to change notification settings - Fork 238
show link to symlink target on analysis page #1282
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
Conversation
270b8d8 to
24cf135
Compare
maringuu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I am hesitating to merge this as FACT's internals are not really built with support for symlinks in mind as can be seen by the above comments.
Of course, this feature is useful and desirable, but not possible to implement correctly atm.
I'll leave it to you or @dorpvom to merge this.
| for uid in session.execute(query.limit(1)).scalars(): | ||
| return uid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am aware that we, by design, can not get this right, but I'd just like to note that using limit(1) to get a random target file is not necessarily correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, there should only be one result at this point (but there may be so weird edge cases that I haven't thought of)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To elaborate a bit on the underlying idea: at this point we know the root FW of the file and the path that is mentioned in the file type ("link to '/a/b/c'). The idea is to look in the "virtual path" for paths that line up with this. Usually, this should be only one file. But there are two other possibilities: the file is missing from the file system or there are multiple file systems or containers with this path contained in the firmware. In the latter case, there isn't really a right or wrong result, since the file object represents all of these files at once. We could display all link targets, though.
ea4c2b5 to
e255107
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1282 +/- ##
==========================================
- Coverage 92.46% 91.67% -0.80%
==========================================
Files 383 376 -7
Lines 24537 21089 -3448
==========================================
- Hits 22689 19333 -3356
+ Misses 1848 1756 -92 ☔ View full report in Codecov by Sentry. |
e255107 to
adb1e79
Compare
|
rebased on master to fix the bug that let the CI fail |
No description provided.