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

fix: don't put erroneous ifds back on the stack #17

Merged
merged 1 commit into from
Oct 20, 2024
Merged

Conversation

csarn
Copy link
Contributor

@csarn csarn commented Oct 18, 2024

This avoids an endless loop that happened with an actual image file.
I don't know much about EXIF, so I can't say whether this is the correct way of fixing the problem.
I also can't judge whether the exif data is actually valid in the file, but exiftool and different picture viewers can display the exif data.
Nevertheless, returning Errors from malformed files is ok, an endless loop is not ok.
I tried to attach an the file in question (resized to 1x1), hoping it survives being handled by github.
broken-exif

Review / Feedback is appreciated.

This avoids an endless loop that happened with an actual image file
@mindeng
Copy link
Owner

mindeng commented Oct 19, 2024

Thank you for the PR!

The purpose of putting ifd back is to parse out as many subsequent entries as possible. But since infinite loops can occur in some cases, I think we can make this quick fix first, and see if there is a better way later.

In addition, I suggest putting the image file into testdata/ and adding the corresponding test case to prevent our subsequent modifications from causing the same error.

@mindeng mindeng merged commit 4f7c139 into mindeng:main Oct 20, 2024
1 check passed
@csarn
Copy link
Contributor Author

csarn commented Oct 22, 2024

Thanks for merging! Any tips how to write a test that the code does not enter an infinite loop? The test run never finishing is not a nice outcome. Start it in a background thread and kill it after a second, if not finished?

@mindeng
Copy link
Owner

mindeng commented Oct 23, 2024

Thanks for merging! Any tips how to write a test that the code does not enter an infinite loop? The test run never finishing is not a nice outcome. Start it in a background thread and kill it after a second, if not finished?

Good question! I think what you suggested should be feasible:

  • Parse the image in a separate thread
  • In the main thread, receive a message indicating the parsing is complete via a channel, and if the message is received, the test can end smoothly
  • The main thread can set a timeout of 1 second when receiving the message; if no message is received within that time, an error will be thrown

This way, at least in the absence of errors in the code (which is the majority of cases), the test case can end immediately without waiting for 1 second. A wait time of 1 second will only occur in case of an error, which is acceptable.

@mindeng
Copy link
Owner

mindeng commented Oct 23, 2024

I have added this test case, you can refer to this commit.

@mindeng
Copy link
Owner

mindeng commented Oct 23, 2024

Hi @csarn

Version 2.1.1 has been released. Thank you for your contribution!

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.

2 participants