|
anroid lomarf
|
#
?
Mar 18, 2023 15:41
|
|
|
|
| # ? Mar 29, 2023 10:32 |
|
|
it used to work when they wrote the app. the os changed the behavior of the api they were using now ideally you’d have an automated test for it, and then hey, maybe you catch the regression in the os api. but i can understand why they didn’t check it again manually
|
|
#
?
Mar 18, 2023 15:42
|
|
|
an os like android by a company like google should have unit tests covering every single minuscule loving thing and that something like this would slip through is just absolutely bonkers
|
|
#
?
Mar 18, 2023 15:44
|
|
|
mystes posted:I don't understand if you're saying they're not rewriting the entire new length of the file (in which case that sounds hard but I guess it's clearly a performance optimization to have to write fewer bytes then) or if you're saying that they're writing the whole new length which is shorter than the original length (in which case you're saying exactly the same thing I'm saying) i'm not sure, that's basically what i was asking. it seems like the first one to me
|
|
#
?
Mar 18, 2023 15:48
|
|
|
mystes posted:I don't understand if you're saying they're not rewriting the entire new length of the file (in which case that sounds hard but I guess it's clearly a performance optimization to have to write fewer bytes then) or if you're saying that they're writing the whole new length which is shorter than the original length (in which case you're saying exactly the same thing I'm saying) they open the file for writing. this is supposed to truncate any existing content in the file, so that writes just append to a now-empty file. and that is how it worked on android for a long time, including when they wrote this app. but android broke that api in a release a couple years ago, so now the file is not truncated, and the writes just overwrite whatever content is at the start of the file. that content includes header information describing the supposed layout of the file, so if you process the file normally, the file won’t claim to have any meaningful data past a certain offset. as a result, clients reading the file normally will just ignore all the old data at the end. but that data is still there, and if you take some reasonable guesses at how to interpret it, you can extract meaningful images
|
|
#
?
Mar 18, 2023 15:57
|
|
|
rjmccall posted:they open the file for writing. this is supposed to truncate any existing content in the file, so that writes just append to a now-empty file. and that is how it worked on android for a long time, including when they wrote this app. but android broke that api in a release a couple years ago, so now the file is not truncated, and the writes just overwrite whatever content is at the start of the file. that content includes header information describing the supposed layout of the file, so if you process the file normally, the file won’t claim to have any meaningful data past a certain offset. as a result, clients reading the file normally will just ignore all the old data at the end. but that data is still there, and if you take some reasonable guesses at how to interpret it, you can extract meaningful images
|
|
#
?
Mar 18, 2023 16:05
|
|
|
now i'm curious, someone with an affected device: does it write multiple IEND chunks? there should only be one from libpng pre:Critical chunks (must appear in this order, except PLTE is optional):
Name Multiple Ordering constraints
OK?
IHDR No Must be first
PLTE No Before IDAT
IDAT Yes Multiple IDATs must be consecutive
IEND No Must be last
|
|
#
?
Mar 18, 2023 16:07
|
|
|
rjmccall posted:fwiw in c “w” is specified to truncate, so i don’t know what the gently caress this dude was thinking just to in-line it, this was the original code that was replaced with the new modes: code:the bug IDs in the commit don’t open for me but I wonder…
|
|
#
?
Mar 18, 2023 16:09
|
|
|
hobbesmaster posted:they changed the flag behavior from matching the C fopen to matching the POSIX open function? yeah, it's the sort of change that an idiot would think was elegant, it stops recognizing specific strings and instead does a sort of lovely parse. it adds O_TRUNC if the string contains 't' at least once, and since "w" does not, it no longer truncates
|
|
#
?
Mar 18, 2023 16:24
|
|
|
i've been on anroid 9 because i'm an idiot who doesn't update but, once again, somehow it ends up saving my rear end it is intensely funny to me though because you saw this poo poo a ton in the old days
|
|
#
?
Mar 18, 2023 19:21
|
|
|
distortion park posted:loving hell glad i never use phone images this way, any interesting itw redactions been found? given how many platforms make a newer compressed version it should be minimal impact for all shared images.. surely..
|
|
#
?
Mar 18, 2023 19:26
|
|
|
according to the write-up discord doesn't recompress the image or strip the extra data, so there's probably a ton of leaky images kicking around on there
|
|
#
?
Mar 18, 2023 19:31
|
|
|
repiv posted:according to the write-up discord doesn't recompress the image or strip the extra data, so there's probably a ton of leaky images kicking around on there ....
|
|
#
?
Mar 18, 2023 19:41
|
|
|
discord not re-encoding is an incredible choice for a public platform. just amazing thinking
|
|
#
?
Mar 18, 2023 19:51
|
|
|
i'm actually quite angry right now, and not enjoying this knowledge at all
|
|
#
?
Mar 18, 2023 19:52
|
|
|
its extremely funny but we are the butt of the joke
|
|
#
?
Mar 18, 2023 19:53
|
|
|
I don't have a pixel phone anyway but I'm so paranoid about failing to strip exif data properly and somehow accidentally leaving in the gps data that I don't think I ever directly upload images I've taken with my cellphone without editing them on my computer or something
|
|
#
?
Mar 18, 2023 20:24
|
|
|
Subjunctive posted:discord not re-encoding is an incredible choice for a public platform. just amazing thinking failures all around. anroid open() calls writing in append mode by default lmfao
|
|
#
?
Mar 18, 2023 20:34
|
|
|
I usually just upload on imgur because I'm under the impression that it strips exif data. Also, I dunno if I ever used the cropping tool on my pixel because it's such a headache to use. Much easier to put it on a laptop and gimp it from there.
|
|
#
?
Mar 18, 2023 20:34
|
|
|
spankmeister posted:failures all around. anroid open() calls writing in append mode by default lmfao I thought it was overwrite-but-not-truncate?
|
|
#
?
Mar 18, 2023 20:49
|
|
|
Subjunctive posted:discord not re-encoding is an incredible choice for a public platform. just amazing thinking up until relatively recently, it also did not strip hevc or heif metadata. this left gps location data in them.
|
|
#
?
Mar 18, 2023 20:52
|
|
|
Subjunctive posted:I thought it was overwrite-but-not-truncate? yeah if it was append mode a lot more things would break, like in this case nobody would be able to see the changes they did in the crop program after saving
|
|
#
?
Mar 18, 2023 20:52
|
|
|
sb hermit posted:I usually just upload on imgur because I'm under the impression that it strips exif data. imgur does strip metadata, but that's not the same thing as re-encoding the file itself PNGs don't have a designated container, just basically a header, image data, and then and ending (see my previous post for the general structure). basically any metadata further than that is extraneous to a viewer, anything past the IEND chunk can be ignored and usually is. that's what google was doing the problem is i'm not sure what happens when you have multiple IDATs in a file uploaded to imgur after an IEND. it might truncate there and it might not. this is what i was asking about earlier
|
|
#
?
Mar 18, 2023 21:19
|
|
|
this is just making sure that the rest of the image is still there in case you need it again later
|
|
#
?
Mar 18, 2023 21:22
|
|
|
Progressive JPEG posted:this is just making sure that the rest of the image is still there in case you need it again later i really don't think it was intentional since IHDR is overwritten
|
|
#
?
Mar 18, 2023 21:24
|
|
|
Beeftweeter posted:i really don't think it was intentional since IHDR is overwritten
|
|
#
?
Mar 18, 2023 21:26
|
|
|
to be clear it’s ParcelFileDescriptor.open() in the ndk, and only if you don’t construct the flags to pass yourself and use the helper function to construct them from a string. definitely not at the posix layer i don’t know the android stack well enough to know the breadth of impact of that. i would guess that android’s implementations of the standard java.util i/o apis do not funnel through this behind the scenes, or at least don’t usually construct flags from string. (i don’t even know to what extent android reimplements that stuff vs. using a standard implementation that talks to the posix layer directly.) so probably this only affects whatever slice of android code is working with these apis directly? which is more likely to be first-party stuff like the google apps
|
|
#
?
Mar 18, 2023 21:36
|
|
|
fwiw i just looked at distorton park's mystes example and there is only one IEND and a "normal" IHDR so it seems imgur at least does re-encapsulate them. acropalypse also says "no additional image data" which tracks with what i see
|
|
#
?
Mar 18, 2023 21:37
|
|
|
it does seem a little weird to me that they changed the default write mode without so much as an fyi
|
|
#
?
Mar 18, 2023 22:28
|
|
|
ah there’s some valuable context in this bug there are indeed a bunch of apis in the ndk that open files using this api with the mode string “w”, notably all the ContentResolver apis for opening output streams. so all of that poo poo got broken also someone pointed out the test contains MODE_CREATE twice in the test for “w”, so i’m guessing some combination of copy-and-paste bugs from the two blocks of tests, plus a heavy measure of not thinking too much
|
|
#
?
Mar 18, 2023 22:44
|
|
|
Subjunctive posted:I thought it was overwrite-but-not-truncate? you're absolutely right I was mistaken. in my defense i was full of khachapuri and chacha when posting and am still dealing with the after effects now
|
|
#
?
Mar 19, 2023 03:10
|
|
|
redleader posted:it does seem a little weird to me that they changed the default write mode without so much as an fyi yeah what’s the background of this google employee that did this. it outright seems intentional.
|
|
#
?
Mar 19, 2023 05:37
|
|
|
Celexi posted:yeah what’s the background of this google employee that did this. it outright seems intentional. extreme git blame
|
|
#
?
Mar 19, 2023 06:32
|
|
|
honestly seems to me like a mistake that wasn't caught in pr
|
|
#
?
Mar 19, 2023 07:38
|
|
|
the whole patch is a slowly unrolling catastrophe, but yeah, i don’t think there was any ill intent, it’s just carelessness 1. the rewrite is poorly conceived to begin with. valid mode strings are enumerated and should be treated that way. the new implementation allows all sorts of stupid stuff to go through without complaint, like “wr” as a synonym for “w” or “retain” as a synonym for “rta” which is nonsensical anyway. but someone was offended by the minor conceptual duplication, i guess 2. the original code was not subtle about treating “w” as a synonym for “wt”, maybe take half a second to understand the behavior of the function before you go barging in 3. the new test obviously would not have passed on the old implementation, that is an important part of testing when you’re refactoring without any intent to change behavior 4. the new test shows a lot of signs of sloppy copy-and-paste, like there’s a repeated mask in the test for this exact case 5. it’s fine to do unit testing of individual apis like this, but there should definitely be at least some functional testing of this api and the apis that wrap it. those tests don’t have to run on every build if you’re worried about overhead, just treat it like conformance testing and run that job every few weeks
|
|
#
?
Mar 19, 2023 08:08
|
|
|
it's obviously an op by a three letter agency
|
|
#
?
Mar 19, 2023 09:17
|
|
|
redleader posted:it's obviously an op by a three letter agency IRS really want to know if you’re cropping any income out of your selfies
|
|
#
?
Mar 19, 2023 09:25
|
|
|
image recovery service 
|
|
#
?
Mar 19, 2023 09:38
|
|
|
redleader posted:it's obviously an op by a three letter agency the Open Source Foundation?
|
|
#
?
Mar 19, 2023 10:44
|
|
|
|
| # ? Mar 29, 2023 10:32 |
|
|
I worked in a datacentre years ago. One of the junior guys got sent to escort a contractor to collect all the kit from a client who had cancelled their rack. An hour later we got a call from another client to check on their stuff as it was offline. He'd led the contractor to the wrong rack, who had then gone on to pack up the wrong company's stuff and drive off.
|
|
#
?
Mar 19, 2023 11:25
|
|
