mate-desktop / atril

A document viewer for MATE
http://www.mate-desktop.org
GNU General Public License v2.0
197 stars 62 forks source link

Trust the file extension for EPUBs #570

Closed raveit65 closed 1 year ago

raveit65 commented 1 year ago

taken from https://github.com/mate-desktop/atril/pull/555

rbuj commented 1 year ago

The example provided is a zip archive. Keep in mind that most documents are compressed archives which can be uncompressed with generic archive/compression tools, however at the beginning of the file there is a file type identifier, which is called a magic number.

The file testatril_mib.zip is a zip archive:

$ file testatril_mib.zip 
testatril_mib.zip: Zip archive data, at least v1.0 to extract, compression method=store```
$ hexdump -C -n 64 testatril_mib.zip
00000000  50 4b 03 04 0a 00 00 00  00 00 26 b7 f6 4c ef e2  |PK........&..L..|
00000010  3e b8 66 46 00 00 66 46  00 00 12 00 00 00 74 65  |>.fF..fF......te|
00000020  73 74 61 74 72 69 6c 5f  6d 69 62 2e 65 70 75 62  |statril_mib.epub|
00000030  50 4b 03 04 14 03 00 00  00 00 eb 01 69 4b 00 00  |PK..........iK..|
00000040

The magic number for epub files is 504B03040A000200 + mimetypeapplication/epub+zip. The mime-type is application/epub+zip.

Sample epub file: epub30-spec.epub

$ $ hexdump -C -n 64 epub30-spec.epub
00000000  50 4b 03 04 0a 00 00 08  00 00 3b 8b 98 46 6f 61  |PK........;..Foa|
00000010  ab 2c 14 00 00 00 14 00  00 00 08 00 00 00 6d 69  |.,............mi|
00000020  6d 65 74 79 70 65 61 70  70 6c 69 63 61 74 69 6f  |metypeapplicatio|
00000030  6e 2f 65 70 75 62 2b 7a  69 70 50 4b 03 04 0a 00  |n/epub+zipPK....|
00000040

Even if the extension is changed, the file type is still preserved

$ mv epub30-spec.epub epub30-spec.zip
$ file epub30-spec.zip 
epub30-spec.zip: EPUB document
raveit65 commented 1 year ago

@rbuj When you have concerns please fix it for yourself. I only picked up a bad PR with other issues.

raveit65 commented 1 year ago

Man, you only have to extract the zip file and you will get an epub file testatril_mib.epub. This was already written in report. Sorry, i 've no idea about your intention of your post :sunglasses:

vkareh commented 1 year ago

I also don't understand @rbuj's response. Are you saying that this PR is unnecessary, or are you confirming that it works, or are you suggesting ways of testing it?

rbuj commented 1 year ago

Ok, I'm going to debug it... the PR is about to disable the file mime type resolution by its content, i.e. use the fast resolution only. The mime type resolution by its content works fine with the epub file I provided after changing the extension to zip.

Captura de pantalla 2022-08-04 a les 19 40 35

Note there is a segfault loading epub files fixed by #568

rbuj commented 1 year ago

testatril_mib.epub is a zip compressed archive which was renamed to testatril_mib.epub.

[robert@riemman Baixades]$ unzip testatril_mib.zip 
Archive:  testatril_mib.zip
 extracting: testatril_mib.epub      
[robert@riemman Baixades]$ file testatril_mib.epub
testatril_mib.epub: Zip archive data, at least v2.0 to extract, compression method=store
[robert@riemman Baixades]$ hexdump -C -n 64 testatril_mib.epub 
00000000  50 4b 03 04 14 03 00 00  00 00 eb 01 69 4b 00 00  |PK..........iK..|
00000010  00 00 00 00 00 00 00 00  00 00 09 00 00 00 4d 45  |..............ME|
00000020  54 41 2d 49 4e 46 2f 50  4b 03 04 14 03 00 00 08  |TA-INF/PK.......|
00000030  00 eb 01 69 4b d3 e2 45  56 93 00 00 00 db 00 00  |...iK..EV.......|
00000040
rbuj commented 1 year ago

EPUBCheck reports several validation errors for the file testatril_mib.epub:

[robert@riemman epubcheck-4.2.6]$ java -jar epubcheck.jar ~/Baixades/testatril_mib.epub 
Validating using EPUB version 2.0.1 rules.
ERROR(PKG-006): /home/robert/Baixades/testatril_mib.epub(-1,-1): Mimetype file entry is missing or is not the first file in the archive.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/003.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/004.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/005.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/006.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/007.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/008.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/009.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/010.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/011.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/012.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/013.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/014.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/015.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/016.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/017.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/018.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/019.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/020.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/021.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/022.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/023.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/024.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/025.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/026.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/027.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/028.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/029.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/030.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/031.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/032.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/033.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/034.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/035.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/036.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/037.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/038.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/039.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/040.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/041.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/042.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/043.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/044.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/045.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/046.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/047.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/048.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/049.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/050.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/051.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/052.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/053.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/054.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/055.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/056.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/057.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/058.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/059.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/060.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/061.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/062.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/063.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/064.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/065.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/066.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/067.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/068.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/069.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/070.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/071.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/072.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/073.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/074.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/075.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/076.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/077.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/078.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/079.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/080.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/081.html" could not be found.
ERROR(RSC-001): /home/robert/Baixades/testatril_mib.epub(-1,-1): File "Ops/notes.html" could not be found.

Check finished with errors
Messages: 0 fatals / 81 errors / 0 warnings / 0 infos

EPUBCheck completed
rbuj commented 1 year ago

@vkareh there is no issue to fix, testatril_mib.epub is a malformed or corrupted epub file.

rbuj commented 1 year ago

Another approach #571

raveit65 commented 1 year ago

I don't mind. Btw. you should really start asking Travis-CI for new credits, because you're the person who do eat all credits. Sorry, i can't review your PRs :sun_with_face:

rbuj commented 1 year ago

I agree, the continuous integration should not be automatically triggered on every pull request.

raveit65 commented 1 year ago

I agree, the continuous integration should not be automatically triggered on every pull request.

Nope, looks like your English is very bad. Everything is fine with current setup. Again in big letters It isn't acceptably that only you won't write a simple mail for asking for new credits Every body in the team can do that. There is no reason for you not to be a team player.

rbuj commented 1 year ago

How many members does the team have? How many credits do they give on each request? How many credits did we have? Am I the only one who hasn't submitted the request for more credits?

raveit65 commented 1 year ago

How many members does the team have?

https://github.com/orgs/mate-desktop/teams

How many credits do they give on each request?

25000

How many credits did we have?

Actual are 19990 available. login at https://app.travis-ci.com/organizations/mate-desktop/plan/usage

Am I the only one who hasn't submitted the request for more credits?

Yes, and actually you eat 25000 credits in six days :-)

rbuj commented 1 year ago
25,000 credits means about 2,500 build minutes on Linux, and some projects take more build minutes than others. Project build time
atril 42m
caja 40m
mate-applets 21m
mate-settings-daemon 20m
mate-desktop 16m
mate-polkit 7m

We are not only talking about the time required to compile the code, plus we are also talking about the time required to perform the static analysis of the code.

An integration test is automatically triggered every time a merge request is sent or updated. What's more, even if the merge request is not carried out, an integration test is performed by sending or updating the development branch before asking for a merge request. That is a problem if you have few credits.

In my opinion, to avoid the loss of credits, the reviewer would have to launch the integration test manually.

raveit65 commented 1 year ago

You only need to write a simple mail.... Try to be a team player.

raveit65 commented 1 year ago

You can discuss 24/7 nonsense with me, i don't review any PR from you until you review your own behavior :P

raveit65 commented 1 year ago

An integration test is automatically triggered every time a merge request is sent or updated.

Every integration to master/stable repo or every new push to PR ( eg. you did a mistake ) needs build time from Travis-CI and triggers a new build.

In the last month i had to ask 3 times for new credits with a simple mail at travis. I don't think MATE need selfish commiters. MATE needs team players. Looks like your profile doesn't match.

rbuj commented 1 year ago

@raveit65 take a look at the csv file.

rbuj commented 1 year ago

I will not send the mail asking for CI credits in the same way that I don't ask for money for contributing in MATE, as I do it in my spare time, without an economic profit.

rbuj commented 1 year ago

Looks like your profile doesn't match.

@raveit65 are you firing me up?

raveit65 commented 1 year ago

I will not send the mail asking for CI credits in the same way that I don't ask for money for contributing in MATE, as I do it in my spare time, without an economic profit.

Thank you for this self-disqualifying comment. I don't see any reason that another team member should do your job.

rbuj commented 1 year ago

I will not send the mail asking for CI credits in the same way that I don't ask for money for contributing in MATE, as I do it in my spare time, without an economic profit.

Thank you for this self-disqualifying comment. I don't see any reason that another team member should do your job.

You see what you want to see. It has been a pleasure contributing to MATE until today.

raveit65 commented 1 year ago

It was always a pleasure to work with you. God bless you.

lukefromdc commented 1 year ago

Closing in favor of https://github.com/mate-desktop/atril/pull/571 which fixes this in a potentially safer way

raveit65 commented 1 year ago

Closing as https://github.com/mate-desktop/atril/pull/571 is merged.