Skip to content

Conversation

@jgajek
Copy link

@jgajek jgajek commented Oct 26, 2016

Fixes parser bug described in jesparza#61

@jbremer
Copy link

jbremer commented Oct 27, 2016

Would this be a good time to setup unit testing for peepdf? :-)
Do you have a 'minimal' pdf for this by any chance?

@jgajek
Copy link
Author

jgajek commented Oct 27, 2016

Well, it's not 'minimal', but here is a real PDF that exhibits this issue:

http://www.cci.health.wa.gov.au/docs/BB-1-Overview.pdf

@jbremer
Copy link

jbremer commented Oct 27, 2016

Sweet! I'll take it.

jbremer pushed a commit that referenced this pull request Oct 27, 2016
Also known upstream as jesparza#61.
@jbremer jbremer merged commit fb4c484 into hatching:master Oct 27, 2016
@jbremer
Copy link

jbremer commented Oct 27, 2016

Merged and added a unit test for it ;-) Interestingly enough getting 21% code coverage with just a single unit test, not bad.

@jbremer
Copy link

jbremer commented Oct 27, 2016

And last but not least, bumped to 0.3.3.

@jgajek
Copy link
Author

jgajek commented Oct 27, 2016

Thanks for your work in packaging it :)

For the unit test, perhaps it would be better to use manualAnalysis=True? Unless you intentionally want to kick off automatic JS emulation.

@jgajek jgajek deleted the jbremer branch October 27, 2016 16:32
@jbremer
Copy link

jbremer commented Oct 27, 2016

I guess that works, too. I'm just wondering though, does it really matter? And if so, should we not do the same in cuckoo?

@jgajek
Copy link
Author

jgajek commented Oct 27, 2016

Yes, I think we should do the same in Cuckoo, at least by default. Perhaps
have an advanced option to turn on JS emulation -- it has the potential to
really slow down the analysis or go into an infinite loop.

On Thu, Oct 27, 2016 at 2:19 PM, Jurriaan Bremer notifications@github.com
wrote:

I guess that works, too. I'm just wondering though, does it really matter?
And if so, should we not do the same in cuckoo?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AHJezjVdvHjR73rEFjGZdlmsZkdSNDXkks5q4Os8gaJpZM4KhttE
.

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