Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files with non-standard extension casing don't get picked up for automatic conversion #17

Closed
jason919 opened this issue Jul 7, 2022 · 18 comments
Labels
bug Something isn't working

Comments

@jason919
Copy link

jason919 commented Jul 7, 2022

currently if the user sends an heic image, the web still shows the download sign, but it doesn't either display or download the image,
currently, no browser can actually support heic image yet, so we need a mechanism to automatically convert the heic to JPG

@tagavari
Copy link
Member

tagavari commented Jul 8, 2022

AirMessage Server should automatically convert .heic files to .jpeg for the web and Android clients, whenever they request to download an attachment.

I can confirm this by downloading this HEIC test file, sending it in a conversation, and then downloading that file from AirMessage for web. It gets converted to a .jpeg, and the web browser can display it. Are you finding that this is not the case?

@jason919
Copy link
Author

jason919 commented Jul 8, 2022

not in my case, whenever other people share heic images to me, it shows the download icon, but it doesn't neither download or show image after I click. I attached the screenshots, someone shared 2 heic images to me, then I clicked the first one, it just shows a blank.
1
2

@jason919
Copy link
Author

jason919 commented Jul 8, 2022

I used browser inspector, it shows it tried to access the image link like this: https://web.airmessage.org/28c372fc-7f19-41d7-8ea4-1cc209881efb, so how does it tell it is heic? the interesting thing is it shows http 200 back.

@jason919
Copy link
Author

jason919 commented Jul 8, 2022

can you point me in the code where it does the conversion? Maybe I can fix the bug myself.

@tagavari
Copy link
Member

tagavari commented Jul 8, 2022

Thank you for the details.

FileNormalizationHelper.swift is what controls converting files from Apple's proprietary formats to more common ones.

Another place I would recommend to check is the server logs, which you can find at ~/Library/Application Support/AirMessage/logs/latest.log. Keep the log window open as you download an HEIC file, and you should see a message like this:

2022-07-08 09-16-06 [info] Converting file IMG_0000.heic from HEIC

@jason919
Copy link
Author

jason919 commented Jul 8, 2022

checked the logs, it doesn't have any details. only got this:
2022-07-08 15-33-13 [info] Received new message: attachmentReq / of length 46
2022-07-08 15-33-27 [info] Received new message: attachmentReq / of length 46

@jason919
Copy link
Author

jason919 commented Jul 8, 2022

I read the code, I think the problem is at this line 80 :
else {
return nil
}
It simply checks extension, but Apple may not have the extension for the heic, so the logic can be better with
ext == "heic" || exit == nil do the conversion. A smarter one shall use the image magic tool to do the conversion.

@jason919
Copy link
Author

jason919 commented Jul 8, 2022

if my test server uses connect-open.airmessage.org, what's the link for web client?

@tagavari
Copy link
Member

tagavari commented Jul 9, 2022

I see, that makes sense. Your file isn't being picked up because the file extension is in capitals.

For now, the server should be updated to recognize file extensions in a case-insensitive manner. I've made these changes in this commit.

I agree that at some point this should be done in a more robust manner, perhaps by inspecting the file, or using the mime_type field that Messages provides.

I'm transferring this issue to airmessage-server.

Thank you so much for your help!

@tagavari
Copy link
Member

tagavari commented Jul 9, 2022

I do not have any pre-built site set up for connect-open.airmessage.org, though now that you bring it up it sounds like it might be a good idea. For now, you will have to build it from source.

@tagavari tagavari transferred this issue from airmessage/airmessage-web Jul 9, 2022
@tagavari tagavari added the bug Something isn't working label Jul 9, 2022
@tagavari tagavari changed the title heic support Files with non-standard extension casing don't get picked up for automatic conversion Jul 9, 2022
@jason919
Copy link
Author

jason919 commented Jul 9, 2022

i can build it from source, the config file is pointing to the connect-open.airmessage.org, how do I change it to point to the real server? The key file is also for the test server I believe.

@tagavari
Copy link
Member

tagavari commented Jul 9, 2022

The client secrets for the primary server are not publicly available. I ask that developers please do not connect unofficial clients to AirMessage's primary server.

My apologies for the inconvenience.

@jason919
Copy link
Author

jason919 commented Jul 9, 2022

can you help release a beta for the fix?

@jason919
Copy link
Author

jason919 commented Jul 9, 2022

another thing is, why is it designed to click the image to show? why not just show the images automatically?

@tagavari
Copy link
Member

I'll be able to prepare a release with this fix this week.

Having attachments automatically show up is something I'd love to have. There's no strict reason as to why it's not implemented, though compression or automatic conversions make this a little trickier.

@jason919
Copy link
Author

I'll be able to prepare a release with this fix this week.
Thank you!!

Having attachments automatically show up is something I'd love to have. There's no strict reason as to why it's not implemented, though compression or automatic conversions make this a little trickier.
I don't understand why this is related to "compression or automatic conversions". I thought it is a pure UI issue, the worst case is that you can have a timer to traverse the dom, find those download buttons and click them using JS, without modifying the existing code.

@tagavari
Copy link
Member

The issue with this approach is that it can take up a lot of bandwidth and data if users aren't prepared for it.

AirMessage for Android has this functionality, though it's a fairly simple implementation that downloads small files as you scroll by them.

@jason919
Copy link
Author

I think you worry about it too much. The folks who want to use this App are geeks. Their gadgets and network speed shall be more than enough to handle this tiny traffic.
One thing you can do to reduce your main website traffic is to cache the images on the client side longer, so you can set the image expiration header really long in http header, so web client won't download images every time from server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Resolved bugs
Development

No branches or pull requests

2 participants