Skip to content

Conversation

@qkaiser
Copy link
Contributor

@qkaiser qkaiser commented Feb 2, 2023

ubi-reader scripts creates an output directory named after the input file basename within the extraction directory.

We don't think these directories are required and writing directly to the extraction directory saves some space.

This is what it looks like before:

$ ubireader_extract_images ~/unblob/tests/integration/filesystem/ubi/ubi/__input__/fruits.ubi
$ find ubifs-root 
ubifs-root
ubifs-root/fruits.ubi
ubifs-root/fruits.ubi/img-1180426539_vol-apple.ubifs
ubifs-root/fruits.ubi/img-1180426539_vol-data.ubifs

This is what it looks like with our fix:

$ ubireader_extract_images ~/unblob/tests/integration/filesystem/ubi/ubi/__input__/fruits.ubi
$  find ubifs-root 
ubifs-root
ubifs-root/img-1180426539_vol-apple.ubifs
ubifs-root/img-1180426539_vol-data.ubifs

We initially applied this change in our fork of ubi-reader to make the output of unblob cleaner (see onekey-sec/unblob#345). I understand this is kind of an opinionated API change but our team would love to see it land upstream so that we can publish unblob to PyPi (see onekey-sec/unblob#501).

@kissgyorgy
Copy link

kissgyorgy commented Feb 2, 2023

As @jrspruitt commented:

The other patch, about the extract path, I don't want to include as this actually serves a purpose for me.

What about keeping the original logic if not specified? but use the specified one if specified. Maybe even a new option "--basename-in-outpath" or something?

@jrspruitt
Copy link
Contributor

I failed to notice that this did not change ubireader_extract_files. I totally agree the directory is not needed. My only issue is that if you work on more than one UBI image, perhaps to compare contents, the UBIFS file names can be ambiguous. So what about something like this, prefixing the image file name, with the UBI filename?

ubifs-root/fruits.ubi_img-1180426539_vol-apple.ubifs
ubifs-root/fruits.ubi_img-1180426539_vol-data.ubifs

@qkaiser
Copy link
Contributor Author

qkaiser commented Feb 3, 2023

I'll talk with the unblob dev team, I think we can live without this change applied to ubi-reader. We'll just need to adapt our integration tests for UBI. Will get back to you.

ubireader_extract_images creates an output directory named after the input file
basename _within_ the output path.

We introduce the --no-basename-subdir option for users who don't want
this extra directory to be created.
@qkaiser
Copy link
Contributor Author

qkaiser commented Feb 3, 2023

@jrspruitt I modified the initial pull request to only touch ubireader_extract_images. The default behavior stays the same, but if users don't want the extra directory they can use the --no-basename-subdir flag.

We can change the option name / add a short name if required. What do you think ?

@jrspruitt
Copy link
Contributor

Hi @qkaiser,

You said in your other post you didn't need this. I'm actually in the middle of a big UI update, that would essentially just wipe out this code when it's finally done as the code behind it is completely changing #63 . I can add this at that point, if it's not crucial now.

I am more inclined to just skip the directory creation all together, for extract_images and utils_info, and just make the file name include the UBI image name. Is there an issue with just putting it in the file name instead of creating a directory?

-Jason

qkaiser added a commit to onekey-sec/unblob that referenced this pull request Feb 6, 2023
We cannot use git hosted dependencies if we want to publish unblob to
PyPi. We therefore moved from our fork of ubi-reader to the official
package of ubi-reader.

Our fork was synced with upstream except for one UI change we did to get
rid of superfluous directories created by ubireader_extract_images.
Since the maintainer of ubi-reader does not want to include it upstream,
we moved that logic into the UBIExtractor class.

See onekey-sec/ubi_reader#67 for more details.
@qkaiser
Copy link
Contributor Author

qkaiser commented Feb 6, 2023

That's ok, we've moved our custom logic to the UBI handler (onekey-sec/unblob#504). Closing this PR.

@qkaiser qkaiser closed this Feb 6, 2023
qkaiser added a commit to onekey-sec/unblob that referenced this pull request Feb 6, 2023
We cannot use git hosted dependencies if we want to publish unblob to
PyPi. We therefore moved from our fork of ubi-reader to the official
package of ubi-reader.

Our fork was synced with upstream except for one UI change we did to get
rid of superfluous directories created by ubireader_extract_images.
Since the maintainer of ubi-reader does not want to include it upstream,
we moved that logic into the UBIExtractor class.

See onekey-sec/ubi_reader#67 for more details.
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.

3 participants