| | <<<Back 1 day (to 2018/02/06) | 20180207 |
| sebras | tor8: there's a single commit on sebras/master up for review | 11:48.20 |
| | but I'd like to talk a bit about the ones on sebras/wip too. | 11:48.36 |
| | btw, one thing I'm worried a bit about is that the different locations involving marking objects to resolve recursion might interfere with each other. | 11:49.43 |
| tor8 | I think the only way we could end up in that case is if one traversal happens at the same time as another | 11:51.41 |
| | which should only be possible if it happens in the repair step which is triggered while traversing something | 11:52.06 |
| paulgardiner | Could someone please review a tiny change on my master? - a fix for an uninitialised variable warning. | 11:52.20 |
| sebras | paulgardiner: you haven't pushed it to paul/master, right? | 11:53.08 |
| paulgardiner | I thought I just did. Will just look | 11:53.28 |
| tor8 | paulgardiner: stupid compiler... that variable can't be uninitialized! are you using gcc perchance? | 11:53.32 |
| | I know gcc gets royally confused in the presence of setjmp | 11:53.46 |
| | anyway, patch LGTM | 11:53.56 |
| paulgardiner | iOS build | 11:53.57 |
| | Ta | 11:54.04 |
| tor8 | huh, I thought iOS used clang | 11:54.07 |
| paulgardiner | Something's moaning :-) | 11:54.26 |
| sebras | LGTM2 | 11:55.04 |
| paulgardiner | I wondered if it might be confused by the rethrow call, but presumably that just looks like an ordinary function call, so still c = EOF looks to be reached | 11:55.47 |
| | ... so I don't understand why it thinks it's uninitialised either | 11:56.22 |
| sebras | paulgardiner: it probably can't tell what fz_push_try() would return and what the error code in fz_catch() would be, and so thinks it can arrive at the if check below fz_catch() without having passed through either fz_try() or fz_catch() | 11:57.41 |
| paulgardiner | Ah yes. That makes sense | 11:58.11 |
| sebras | tor8: so LGTM or no LGTM on the sebras/master one? | 12:00.10 |
| | seems like I recently fixed a number of those oss-fuzz thingies without having tested them all. I guess I need to go back an confirm that I just haven't papered over something. :-/ | 12:00.55 |
| tor8 | sebras: yes. the one on sebras/master LGTM. | 12:01.55 |
| sebras | wee! :) | 12:02.35 |
| | tor8: in "gl: Render separations using equivalent process colors." and "Add separations to pixmap rendering functions." I change the API so that we are able to render separations in mupdf-gl | 12:13.14 |
| | there's a patch to the same effect for mupdf-x11. | 12:13.23 |
| | I ran into a bug that was separations related and discovered that mutool and mupdf didn't draw these two the same way by default. | 12:13.56 |
| tor8 | I saw, but am not convinced. That engages the full overprint simulation and other overhead, which is maybe not what we want for a viewer (only for printing to a CMYK printer and proofing) | 12:14.02 |
| sebras | tor8: how about making it something we can enable by pressing a button..? | 12:14.38 |
| tor8 | sebras: if you use the -O option to mutool draw (control spot/overprint rendering) can you get the renders to match? | 12:15.10 |
| sebras | 'o' for prooooofing? (I assume p will eventually be for print) | 12:15.12 |
| tor8 | p is for password | 12:15.24 |
| | and P is for parallel | 12:15.30 |
| sebras | if I disable overprint simulation I think it would match yes (haven't tried yet) | 12:15.36 |
| | I mean interactive key, not command-line argument. | 12:15.54 |
| | meant. | 12:15.58 |
| | also the overprint simulation is incorrect in several places. | 12:16.41 |
| | renderingwise, but that is probably a known issue. | 12:16.52 |
| tor8 | sebras: that's unknown to me (and really more in robin and mvrhel's area) | 12:17.23 |
| sebras | a later patch on sebras/wip addresses where we index the alpha incorrectly and so fixes the mutool ASAN complaint. | 12:17.30 |
| | tor8: I know, but r o b i n is busy doing other things so I have avoided bothering him | 12:18.13 |
| Robin_Watts | sebras: how is the overprint simulation wrong ? | 12:18.33 |
| Robin_Watts | has mostly dug himself out of the pit of despair he was in monday/yesterday. | 12:19.08 |
| sebras | Robin_Watts: there are buffer overflow things where the alpha indexing is incorrect (I've fixed those). | 12:19.19 |
| | Robin_Watts: but what I'm referring to here is the rendering of Bug691425.pdf as compared to ghostscript. | 12:19.45 |
| | Robin_Watts: there is a box of yellow drawn around certain graphical elements. and the boundaries of the boxes are clearly visible. | 12:20.17 |
| | in gs they are not. | 12:20.20 |
| tor8 | sebras: I should probably turn off IRC so I can focus on my actual high priority task from last meeting instead of putting out fires everywhere >.< | 12:20.28 |
| sebras | I can fix that. | 12:21.09 |
| Robin_Watts | sebras: For the logs: Those boxes are due to antialiasing, not spot rendering. | 12:30.45 |
| tor8 | Robin_Watts: paulgardiner: fixes for the encryption saving bug on tor/master (the first two commits) | 16:56.37 |
| Robin_Watts | tor8: I'll look in a sec/ | 16:57.00 |
| tor8 | sebras (for the logs): proper fix for bug 698991 on tor/master. | 16:57.11 |
| Robin_Watts | Where did we land on the user/owner password thing? | 16:57.12 |
| | This one: http://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=b6c6eb9327679cbad74a52697e3252141f1c29d6 | 16:57.37 |
| tor8 | I have no strong preference, but it feels *wrong* somehow to refuse to open a document that we *can* decrypt just because the password is missing | 16:58.08 |
| Robin_Watts | tor8: Well, we *can* do anything we like to a document if we have the user password, irrespective of the owner password. | 16:59.41 |
| | but the spec says don't! | 16:59.52 |
| tor8 | just don't ask me to start respecting the permissions ;) | 17:00.11 |
| Robin_Watts | All of this is purely convention (PDF security is shite like that) | 17:00.16 |
| | tor8: That's exactly what this is. | 17:00.22 |
| tor8 | we provide hooks to the clients to look at permissions...enforcing them would be DRMish and immoral. | 17:00.28 |
| Robin_Watts | Ok, but how is Cody supposed to respect the permissions in this case? | 17:01.00 |
| | both your commits lgtm. | 17:01.35 |
| tor8 | Robin_Watts: so in this case, we've got a file that we can only open with the (initial default blank "" password) as the owner password | 17:02.08 |
| Robin_Watts | tor8: We have a file with ""= owner, and "userpass"=user. | 17:02.36 |
| tor8 | would ""=owner and ""=user be okay? | 17:03.12 |
| Robin_Watts | So cody does fz_needs_password, and because the owner password works as "" he gets told not to prompt for a passowrd. | 17:03.26 |
| tor8 | yes, because the initial 'default' password worked | 17:03.54 |
| | I'm thinking whether the 'correct' fix is to disallow blank owner passwords in pdf_authenticate_owner_password | 17:04.57 |
| Robin_Watts | tor8: Right, so he can't implement "correct behaviour" on top of what we are giving him. | 17:05.03 |
| | I think a blank owner password is fine, only if we have no user password (or a blank user password) | 17:05.21 |
| | which is basically what I coded, I believe. | 17:05.41 |
| tor8 | doc->crypt->u == NULL is always false (crypt.u is an array) | 17:06.19 |
| | hm, we run the same password through both functions to see which ones match | 17:07.02 |
| | so what we want is a check that "owner password may only be blank if the user password is also blank" | 17:10.01 |
| | so if (strlen(password) == 0 && auth == 4) fail. | 17:10.19 |
| | auth==4 means owner password matched but not user (if both match, auth will be 6) | 17:10.46 |
| Robin_Watts | Yes. | 17:10.51 |
| | Urm... what if there IS no user password for a document? | 17:12.14 |
| tor8 | 'no' password is equivalent to a password of "" | 17:12.44 |
| | AFAIK | 17:12.58 |
| | Robin_Watts: first commit on tor/wip is one formulation I could live with | 17:13.54 |
| kens | I thought that (if a document is encrypted) then no user password is equivalen tto the 'default' password | 17:14.04 |
| | Which is essentially the same as "" of course | 17:14.26 |
| Robin_Watts | We calculate a crypted password from the password given, and compare it with the value of crypt->u. | 17:14.33 |
| | Suppose there is no crypt->u given in the file. | 17:14.53 |
| | we end up with crypt->u being all zeros. | 17:15.05 |
| | I don't believe that the crypted empty string is all zeros. | 17:15.23 |
| tor8 | Robin_Watts: then we won't be able to authenticate any user password | 17:15.42 |
| Robin_Watts | tor8: Right, but they might be able to authenticate an owner passowrd. | 17:16.10 |
| tor8 | Robin_Watts: both the /O and /U entries are required | 17:16.13 |
| Robin_Watts | which *could* be "" | 17:16.19 |
| | tor8: According to the spec :) | 17:16.26 |
| tor8 | Robin_Watts: that *is* what you just said we should respect ;) | 17:16.38 |
| kens | Are you sure that /U is "" when the user password is "" | 17:17.16 |
| Robin_Watts | kens: I'm pretty sure it's not. | 17:17.32 |
| kens | Yeah that's what I mean I'm almost certain its not | 17:17.43 |
| Robin_Watts | tor8: I said we should respect the spirit of the spec. | 17:17.52 |
| tor8 | /U is a 32-byte string, regardless of the user password length. | 17:17.57 |
| kens | will shut up, I'm not clued up enough on this any more | 17:18.08 |
| Robin_Watts | We should allow people to successfully implement the DRMy nature of the spec if that's what they want to do, on all manner of PDF files, whether they exactly conform to the spec or not. | 17:18.45 |
| tor8 | Robin_Watts: if the file is effed up enough to be *missing* the required user password, but have a working blank owner password, screw 'em | 17:18.59 |
| Robin_Watts | tor8: That's only 1 step further on than the example we have now. | 17:19.41 |
| tor8 | Robin_Watts: thing is, we don't really know if the /U entry is missing or invalid | 17:20.09 |
| | we could throw an exception if it's missing | 17:20.16 |
| | but then if we have a working owner password (that isn't just blank and subject to the issue at hand) we'd fail to open that file | 17:20.50 |
| Robin_Watts | tor8: I'd accept the U entry being all zeros as counting as being "missing". | 17:21.06 |
| tor8 | as it stands, if it's missing (which it really shouldn't) we'll not be able to authenticate | 17:21.19 |
| | and that's probably okay, because if it is missing, we won't be able to decrypt using it either | 17:21.38 |
| | as I understand it, every encrypted PDF file always has both a user and owner password. they may be the same or different. | 17:23.44 |
| | and they may be blank if you just want protection. | 17:23.55 |
| | s/protection/permissions/ | 17:24.04 |
| Robin_Watts | http://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=316baae59e9009e790180460f4bf4499222a3405 | 17:24.20 |
| | Ignore that. | 17:24.29 |
| tor8 | was going to ask what that memcmp function pointer is doing there :) | 17:24.50 |
| | Robin_Watts: and yes, that's essentially the same as my commit mentioned above. | 17:25.19 |
| Robin_Watts | http://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=1b2dae6a35a964731c5eee03293da84da25c7b02 | 17:25.24 |
| | tor8: Oh, I couldn't find your commit. | 17:25.33 |
| tor8 | but with an improved message, so LGTM | 17:25.39 |
| Robin_Watts | Ta. | 17:25.43 |
| tor8 | it was on tor/wip f51a5fb85 pdf: Only allow blank owner password if the user password is also blank. | 17:25.52 |
| Robin_Watts | Has anyone built the windows build recently? :/ | 17:27.08 |
| | paulgardiner's pdf_check_signature stuff has broken in. | 17:29.03 |
| | tests_private/pdf/sumatra/1635_-_empty_owner_password_leads_to_no_password_prompt.pdf now gives an error. I'll have to check whether it should or not. | 18:20.45 |
| paulgardiner | Robin_Watts: I was mainly using the Windows build when doing the signature support reorganisation. Possibly I could have accidentally made the debug build require the presence of openssl | 18:31.39 |
| Robin_Watts | paulgardiner: A simple checkout is failing to build complaining that pdf_check_signature is not defined, and is needed. | 18:32.20 |
| | from pdfapp_onmouse, and from verify_signature | 18:32.47 |
| paulgardiner | Does mupdf => pkcs7 => pkcs7-check.c have a red circlle? It shouldn't have, but if it does that would explain it. | 18:34.41 |
| | Which configuration? I may have tested only Debug and DebugOpenssl | 18:35.54 |
| Robin_Watts | Debug Win32. | 18:36.04 |
| | It does no have a red circle. | 18:36.08 |
| | Ah. It's mupdf-curl that is failing. | 18:36.56 |
| | And mutool. | 18:38.11 |
| | Were you only building mupdf itself ? | 18:38.21 |
| | I suspect you should have put the pkcs7 stuff in libpkcs7, and then made mupdf/mutool/mupdf-curl all depend on it. | 18:39.11 |
| | mupdf-curl failing is not the end of the world. mutool failing is bad. | 18:39.41 |
| paulgardiner | Yeah that would be it. | 18:51.16 |
| | Forward 1 day (to 2018/02/08)>>> | |
Log of #mupdf at irc.freenode.net.