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 issue#1056 #1061 #1062

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

Conversation

Jay-Ernest
Copy link

@Jay-Ernest Jay-Ernest commented Apr 23, 2022

Description

Reason why leads to the crash

The testOOM.pdf is in a wrong pdf encoding format as it does not end with '%%EOF' which is the correct ending of any pdf file. As there is no '%%EOF', the program will not stop reading so that the memory will soon be used out, which leads to out of memory.

How I solve this problem

To solve this, I added some methods to check whether the file is in pdf format, with header(%PDF-(PDF version)) and end(%%EOF). I read the first 20 bytes to check the header in HEX and read backward from the last line to chech whether it is '%%EOF' in HEX. After doing so, the continuously crash was solved. I can normally open the feature PDF-TO-image without crashing.

Still some problems.

First is that, although the app doesn't crash because of OOM, the code still cannot get rid of the illegal pdf file, which means user can still choose the file. After clicking the transform bottom, the app will crash, but not continously crash.
Second, my code cannot check the format like pdf ending with

%%EOF

that is a redundant enter.

Where I test the app

Using the virtual machine: Pixel 4 API 30 in Android Studio.

Fixes #1061 #1056

Type of change

Just put an x in the [] which are valid.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

  • ./gradlew assembleDebug assembleRelease
  • ./gradlew checkstyle

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@codeclimate
Copy link

codeclimate bot commented Apr 23, 2022

Code Climate has analyzed commit bb785ec and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@codegsaini
Copy link
Contributor

although the app doesn't crash because of OOM, the code still cannot get rid of the illegal pdf file

Can you consider doing some research to solve this issue too.

Second, my code cannot check the format like pdf ending with

Try to find way to do it.

@codegsaini
Copy link
Contributor

codegsaini commented Apr 23, 2022

@Jay-Ernest Pull Request #1057 and #1062 (this) are more or less have similar commit, so please close either pull request and focus on one.
And issue #1048 #1056 and #1061 are pointing out toward same problem, so please try to close those into single PR.

@Jay-Ernest Jay-Ernest mentioned this pull request Apr 24, 2022
9 tasks
@Jay-Ernest
Copy link
Author

Jay-Ernest commented Apr 24, 2022

@codegsaini I agree. And in my point of view, the two issue displays two bugs independently. That is, #1056 points out that we need to check the header but the end will not affect the result actually, while #1061 points out that if not check the end of PDF file(%%EOF) will lead to OutOfMemory.
I will try to focus on it. I am doing my course project which is to fix some issues. As it is the first time to participate in Android development, it may be a little hard for me. Thank you for advice.

@codegsaini
Copy link
Contributor

And in my point of view, the two issue displays two bugs independently. That is, #1056 points out that we need to check the header but the end will not affect the result actually, while #1061 points out that if not check the end of PDF file(%%EOF) will lead to OutOfMemory.

Okay that makes sense :)

As it is the first time to participate in Android development, it may be a little hard for me.

Take your time, your goal should be learning things. Otherwise new issues and their fixes is a lifelong process. Involve yourself in finding the solution and you will learn a lot of things during the process.

Thank you for advice.

You're welcome, keep contributing...

* check whether the file is a real pdf file or not (whether the file has the header of pdf file)
*/
public static boolean isPdfFile(String filePath) {
String header = getFileHeader(filePath);
Copy link
Contributor

@codegsaini codegsaini Apr 24, 2022

Choose a reason for hiding this comment

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

#1057 also contains this function as a fix so what I want to say is that-

PR #1062 = commits of #1057 (getFileHeader() method) + getLastLine() method.

So because #1062 already contains what present in #1057 so just close #1057 and #1062 will solve all those three issues.

Hope you understood.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I will deal with it.

@Jay-Ernest Jay-Ernest changed the title Fix continuously crash Fix issue#1056 #1061 Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The APP continously crashes due to OOM
3 participants