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

Add plugin ios_siminfo ke Plaso #4931

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

fitrianhikma
Copy link

@fitrianhikma fitrianhikma commented Dec 2, 2024

One line description of pull request

Description:

Related issue (if applicable): fixes #

Notes:

All contributions to Plaso undergo code review.
This makes sure that the code has appropriate test coverage and conforms to the
Plaso style guide.

One of the maintainers will examine your code, and may request changes. Check off the items below in
order, and then a maintainer will review your code.

Checklist:

  • Automated checks (GitHub Actions, AppVeyor) pass
  • No new new dependencies are required or l2tdevtools has been updated
  • Reviewer assigned
  • Test data has a Plaso compatible license

@@ -1795,3 +1795,29 @@ attribute_mappings:
- name: 'recorded_time'
description: 'Recorded Time'
place_holder_event: true

---

Copy link
Member

Choose a reason for hiding this comment

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

please remove the unnecessary white-space

@joachimmetz
Copy link
Member

Thanks for the PR, I'm a bit preoccupied at the moment will try to take a look as soon as time permits

@joachimmetz joachimmetz self-assigned this Mar 3, 2025
@@ -21,3 +21,6 @@
from plaso.parsers.plist_plugins import spotlight_searched_terms
from plaso.parsers.plist_plugins import spotlight_volume
from plaso.parsers.plist_plugins import time_machine

# Impor parser yang baru dibuat
Copy link
Member

Choose a reason for hiding this comment

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

Please stick to English

@joachimmetz
Copy link
Member

@fitrianhikma a couple of questions:

  • what is the origin of the test data file?
  • What does this timestamp represent?
  • can you translate to English?

@@ -233,3 +233,17 @@ short_message:
- 'Message: {text}'
short_source: 'Twitter iOS'
source: 'Twitter iOS Status'
---
#ios_siminfo
Copy link
Member

Choose a reason for hiding this comment

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

this comments adds no value, removing

- 'SIM MDN: {mdn}'
short_source: 'IOS'
source: 'iOS SIM Info'
---
Copy link
Member

Choose a reason for hiding this comment

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

missing newline character

data_type: 'ios:sim:info'
message:
- 'SIM MDN: {mdn}'
- 'SIM Type: {sim_type}'
Copy link
Member

Choose a reason for hiding this comment

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

style does not match yamllint configuration

#Configuration for data type
data_type: 'ios:sim:info'
attribute_mappings:
- name: 'mdn'
Copy link
Member

Choose a reason for hiding this comment

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

same yaml style nits apply here, aslo only timestamp definition is needed

- name: 'label_id'
description: 'ID label untuk SIM'
- name: 'timestamp'
description: 'Timestamp SIM'
Copy link
Member

Choose a reason for hiding this comment

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

What does this timestamp represent?



class IOSSIMInfoEventData(events.EventData):
"""Event data untuk iOS SIM Info."""
Copy link
Member

Choose a reason for hiding this comment

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

attribute definitions missing from docstring

def __init__(self):
"""Inisialisasi event data."""
super(IOSSIMInfoEventData, self).__init__(data_type=self.DATA_TYPE)
self.mdn = None
Copy link
Member

Choose a reason for hiding this comment

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

style guide nit: use 2 space indentation

events = list(storage_writer.GetAttributeContainers('event_data'))

# Memastikan bahwa data di event pertama sesuai dengan yang diharapkan
event = events[0]
Copy link
Member

Choose a reason for hiding this comment

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

Please use expected_event_values to match rest of codebase e.g. see ios_carplay.py for an example

- 'SIM MDN: {mdn}'
short_source: 'IOS'
source: 'iOS SIM Info'
---
Copy link
Member

Choose a reason for hiding this comment

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

remove trailing --- this adds an empty entry

self.eap_aka = None
self.sim_type = None
self.cb_ver = None
self.label_id = None
Copy link
Member

Choose a reason for hiding this comment

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

please take a look at the style guide, abbreviations like id or ver are not desired

@joachimmetz joachimmetz added the pending reporter input Issue is pending input from the reporter label Mar 3, 2025
Copy link

codecov bot commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 88.09524% with 5 lines in your changes missing coverage. Please review.

Project coverage is 85.09%. Comparing base (11259d3) to head (4a2a791).

Files with missing lines Patch % Lines
plaso/parsers/plist_plugins/ios_siminfo.py 88.09% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4931   +/-   ##
=======================================
  Coverage   85.09%   85.09%           
=======================================
  Files         432      433    +1     
  Lines       38792    38834   +42     
=======================================
+ Hits        33009    33046   +37     
- Misses       5783     5788    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joachimmetz
Copy link
Member

You can ignore the failing "test_docs"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending reporter input Issue is pending input from the reporter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants