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

Do not increment loop counter. #1165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MancunianSam
Copy link
Collaborator

fixes #1140

DROID is getting stuck in an infinite loop trying to match the right fragments for this signature

The steps are something like:

  1. It finds matches for 7 of the 8 right fragments.
  2. It adds these to a matched list
  3. It doesn't find a match for the 8th fragment
  4. It then searches each of the fragments in the matched list to either side of the previous matches.
  5. Fragments 1-7 match and so stay in the matched list
  6. The last matched fragment position is set to 7
  7. The loop checking each fragment increments the fragment position to 8
  8. It doesn't find a match for the 8th fragment
  9. Repeat steps 3 - 9

The problem I think is step 6. It shouldn't be changing the loop counter.

The issue is that I'm not 100% sure if by doing this, we'll be skipping
some fragment checks which will cause a different format to not be
matched at some point in the future.
I don't think so though because the main loop still iterates through
every fragment.

fixes #1140

DROID is getting stuck in an infinite loop trying to match the right fragments for [this signature](https://github.com/nationalarchives/pronom-signatures/blob/develop/signatures/fmt/134.json#L132)

The steps are something like:

1. It finds matches for 7 of the 8 right fragments.
2. It adds these to a matched list
3. It doesn't find a match for the 8th fragment
4. It then searches each of the fragments in the matched list to either side of the previous matches.
5. Fragments 1-7 match and so stay in the matched list
6. The last matched fragment position is set to 7
7. The loop checking each fragment increments the fragment position to 8
8. It doesn't find a match for the 8th fragment
9. Repeat steps 3 - 9

The problem I think is step 6. It shouldn't be changing the loop counter.

The issue is that I'm not 100% sure if by doing this, we'll be skipping
some fragment checks which will cause a different format to not be
matched at some point in the future.
I don't think so though because the main loop still iterates through
every fragment.
Copy link
Collaborator

@sparkhi sparkhi left a comment

Choose a reason for hiding this comment

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

Its quite a core change, I think we will need quite a bit of testing with variety of files

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.

TIFF file(s) causing DROID to hang
2 participants