| <<<Back 1 day (to 2020/09/24) | Fwd 1 day (to 2020/09/26)>>> | 20200925 |
ator | Robin_Watts_: fredross-perry: both those commits are on the master branch | 10:59.03 |
sebras | seems to me like mupdfdk might need to merge master onto its respective branches? | 11:08.31 |
| or even better, merge to a single branch and use that for all platforms. | 11:11.32 |
Robin_Watts_ | sebras: I believe the plan is to converge as best we can, yes. | 11:37.21 |
| aot, sebras: I'm just looking at bug 702857 | 11:48.04 |
| ator: ^ | 11:48.11 |
ator | Robin_Watts_: there's still time if you want to get a fix for that in :) | 11:49.41 |
Robin_Watts_ | ator: That's what I'm trying :) | 11:54.02 |
sebras | "Caused by: java.lang.IllegalStateException: Cannot lock execution history cache" I hate gradle. :-P | 11:55.57 |
| ./gradlew --status && ./gradlew --stop && ../gradlew --status resolved it. | 11:57.43 |
Robin_Watts_ | ator, sebras: Urm... pixmap.c line 83... | 11:59.43 |
| n? n? Surely h ? | 11:59.48 |
sebras | Robin_Watts_: it was n from the beginning: dcdb116ea172b2954d42304238f32183263ddfed | 12:03.18 |
Robin_Watts_ | yeah, but it was checking a different thing then. | 12:04.01 |
| (I don't entirely understand it the check it was doing, but the malloc_array was checking for overflow for us.) | 12:04.39 |
sebras | INT_MAX / pix->n must be the maximum number of pixels an image can be wide. | 12:05.39 |
Robin_Watts_ | yes. | 12:06.07 |
sebras | why zeniko's original patch tested pix->w + pix->n - 1 is less clear to me. | 12:06.09 |
| the next commit that changed it was d0b78f4166a1503ce522944002b3aab035724cd9 | 12:06.31 |
Robin_Watts_ | I have a fix. | 12:06.38 |
| It doesn't SEGV now, but we leak a lot. Let me try to see why. | 12:06.49 |
sebras | zeniko seems to have been tryign to test pix->w > INT_MAX / pix->n | 12:07.47 |
Robin_Watts_ | We want to that that w * b <= INT_MAX | 12:08.37 |
malc_ | tell him to do or not do there is no tryi_GN_ | 12:08.48 |
Robin_Watts_ | We want to test that w * n <= INT_MAX | 12:08.55 |
| or rather to fail if w * n > INT_MAX | 12:09.16 |
| which you'd think would be w > INT_MAX/n , but we're working in ints, so the right hand side rounds down. | 12:09.38 |
| hence testing w - 1 > INT_MAX/n seems reasonable to me. | 12:09.49 |
| actually, rounding down makes it a safer test anyway. | 12:10.24 |
| so I don't get the -1 either. | 12:10.28 |
sebras | git grep '>.*/' | grep MAX | 12:10.42 |
| that reveals a number of locations that do similar tests. | 12:10.50 |
Robin_Watts_ | I can't see any that do the -1 thing? | 12:16.11 |
sebras | nope, but along the lines of w > INT_MAX / n | 12:17.06 |
| that's what I meant. | 12:17.09 |
| and w >= INT_MAX / n is not correct. | 12:18.31 |
| cgdae: welcome back! :) | 12:19.03 |
Robin_Watts_ | sebras: Right. I agree. | 12:19.21 |
cgdae | sebras: :-) | 12:20.44 |
Robin_Watts_ | Hmm. | 12:23.21 |
| pixmap->stride is actually a ptrdiff_t. | 12:23.33 |
| So we should be able to cope with larger sizes. | 12:23.42 |
| But I'd need to change a prototype from taking an int to taking a ptrdiff_t. | 12:24.02 |
sebras | it is _now_, but was it _then_? | 12:24.03 |
| maybe it should be w > SIZE_MAX / n | 12:24.12 |
Robin_Watts_ | ptrdiff_t is signed, SIZE_MAX isn't. | 12:24.30 |
| At the moment we calculate it using an int, because of passing int strode. | 12:24.43 |
| int stride. | 12:24.48 |
| If I change that to a ptrdiff_t, that'll have knock on effects for the bindings, I bet. | 12:25.12 |
| Let's ignore that for now. | 12:25.17 |
| 2 commits on robin/master then. | 12:26.37 |
sebras | ator: -mini and -viewer build fine with an updated jni/libmupdf | 12:26.41 |
Robin_Watts_ | And now I'll look at bug 702937 | 12:28.16 |
ator | sebras: sebras/fix-java2 LGTM | 12:29.34 |
sebras | pushed. | 12:29.38 |
| ator: -mini:sebras/master and -viewer:sebras/master prepared. I think they're correct, but maybe you want an 1.18.0-rc1 or something, I don't know. | 12:31.20 |
| I need to go grab food 30 min ago. | 12:31.38 |
Robin_Watts_ | is in a similar position. | 12:32.10 |
malc_ | "api: Optional use of Tesseract to use OCR to extract text." the wording here could be improved methinks... | 12:32.15 |
ator | sebras: we're doing a 1.18.0-rc1 tag yes | 12:33.08 |
sebras | then my commits are shit. | 12:34.29 |
Robin_Watts_ | ok, part 1 of that bug is solved by the pixmap overflow commit. | 12:36.11 |
| I'll look at part2 after lunch. | 12:36.17 |
| bbs | 12:36.20 |
sebras | ator: we got a security bug for jbig2dec today. I've been looking at it, but I don't have a fix for it yet. | 12:36.46 |
| but then again, there are probably three more assigned to me. | 12:36.59 |
| and a heap of oss-fuzz issues I haven't tended to. | 12:37.16 |
| I will not be able to resolve any of these before the release. | 12:39.05 |
Robin_Watts_ | sebras: THat's the commit I'm looking at. | 13:43.30 |
| THe first part is fixed with the pixmap overflow commit. | 13:43.40 |
| And so it the second part. | 13:45.45 |
| SO if we can get the 2 commits in on robin/master, we can close that bug too. | 13:45.58 |
ator | Robin_Watts_: both LGTM | 13:48.29 |
sebras | Robin_Watts_: I meant 702937, not 702857 | 13:55.11 |
Robin_Watts_ | ator: Ta | 14:02.44 |
| sebras: Yes. 702937 is fixed by the fix for 702857. | 14:03.12 |
sebras | then I give up for the week. | 14:04.33 |
| there is no point in continuing. | 14:05.14 |
Robin_Watts_ | Hmm. I hope I didn't offend him. (I mean it's friday night where he is, so quitting is entirely reasonable...( | 14:06.46 |
| So that leaves us with 2 security bugs. 700204 is a jbig2 one, and 701297 is a problem with our repair stuff that I can't see how to solve easily. | 14:07.53 |
| ator: https://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=e5818fa57947e0f3d6146687dd3628bccc60acbf | 14:21.31 |
ator | if (pdf_annot_type(ctx, annot) == PDF_ANNOT_FILE_ATTACHMENT) could we abstract that into a function pdf_should_print_attachment() maybe? | 14:24.15 |
Robin_Watts_ | ator: Could do. | 14:27.22 |
| Crap. I've just had a file die with division by zero in the overflow commit. | 14:27.45 |
| ator: I think I should pull the top commit out and put in a fixed one. | 14:35.55 |
ator | go for it | 14:36.18 |
Robin_Watts_ | ator: Ok, 2 updated commits, clustering now. | 14:46.47 |
ator | both look fine to me | 15:12.20 |
Robin_Watts_ | ator: Pushed | 15:18.07 |
| ator: 1 more on robin/master. | 16:19.32 |
| I'm going to run through the open mupdf bugs and close the trivial ones I can. | 16:19.48 |
fredross-perry | Robin_Watts_: ator: I’ll just create new branches, then. | 16:23.44 |
Robin_Watts_ | If those are on casper, presumably they are tags? | 16:24.05 |
| s/casper/master/ | 16:24.16 |
fredross-perry | presumably. I think it's just | 16:32.12 |
| 1. check out the numbered commit I'm at (as origin master) | 16:32.12 |
| 2. cherry-pick the one fix | 16:32.13 |
| 3. git push origin master:android-ice (or some such) | 16:32.14 |
Robin_Watts_ | Urm... | 16:34.55 |
| That presupposes you're on master. | 16:35.03 |
| and I suspect step 3 will fail, because the remote won't know what sort of thing android-ice is supposed to be. | 16:35.38 |
| git tag android-ice | 16:35.46 |
| git push origin android-ice | 16:35.56 |
| would be safer. | 16:36.00 |
| (or git branch android-ice, git checkout android-ice, git push origin android-ice, if you want to use branches) | 16:36.29 |
fredross-perry | Thanks, I do think I want branches. Btw I do ‘git push repo branch:new-branch’ to create new branches all the time. What’s not safe? | 16:40.25 |
| I have git version 2.13.5 | 16:41.05 |
Robin_Watts_ | IME, git push repo branch:new-branch gives an error from the server | 16:57.53 |
| unless new-branch is already a known thing on the server. | 16:58.04 |
| If it works for you, then no problem. | 16:58.18 |
| Well, I have no idea about Bug 702506. It's failing to write to stdout. | 17:05.00 |
fredross-perry | “gives an error from the server” - maybe it’s repo-specific? | 17:24.17 |
| <<<Back 1 day (to 2020/09/24) | Forward 1 day (to 2020/09/26)>>> | |